-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VACMS-19449]Situation updates static banner #32872
Conversation
6e62f26
to
1c653f5
Compare
a918dd5
to
8b884d6
Compare
a980648
to
006bfef
Compare
6f0ba61
to
e8d4fde
Compare
e8d4fde
to
dc956b7
Compare
0af17fd
to
ab5d089
Compare
95e1bdf
to
615be39
Compare
615be39
to
d059c72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice use of the feature toggles in the tests.
Also, thanks for putting in the memory updates for watch. I thought I did, but seemed to have missed it somewhere in my changes. |
src/applications/static-pages/situation-updates-banner/createSituationUpdatesBanner.jsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
cda840d
17d12e7
to
cda840d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1 question about common practice/pattern
Are you removing, renaming or moving a folder in this PR?
Did you change site-wide styles, platform utilities or other infrastructure?
Summary
banner_use_alternative_banners
Related issue(s)
Testing done
Scre
What areas of the site does it impact?
This is interacting with a header level feature so it has the potential for sitewide impact
Acceptance criteria
Quality Assurance & Testing
Error Handling
Authentication