-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add a banner to most of GOV.UK for the EU Referendum #769
Conversation
Respects 'hide_banner_notification', so will not be shown on the GOV.UK homepage, which can add it's own banner content, as it does for the emergency publishing system
To prevent the global bar from flashing or shifting the page up and down, we want to check whether to show the bar as soon as possible, and set a class if we want to show it. * Include the full source in the test * Run the test spec against source and the minified version Ideally we would have the full source minified on the fly and injected into the template.
The bar will never show for users without JS
* Don’t show on any paths beginning with /register-to-vote or /done Testing this is a little tricky. Usually you’d create an intermediate method for grabbing the pathname so that you can mock it. This doesn’t work for the minified approach we’ve taken. Instead pass in a fake window object for those specific test scopes.
Multiple bars will clash.
Make it clearer that this is the JS for toggling the HTML class – responsible for hiding and showing it, rather than any interactions related to the bar itself.
* Move from `after_header` to just before the main content block * Add dismissible link which sets seen count to 999
Category: “Global bar” Track when bar is shown, manually dismissed and automatically dismissed
Improve accessibility of the “Hide message” toggle.
Some apps haven’t set `strong` to be bold, eg collections. On these pages the bar doesn’t display correctly.
(12 weeks)
The dismiss function isn’t working, so the bar would always show on every pageview. Use conditional tags to remove the logic to show it.
} | ||
} | ||
|
||
function viewCount() { |
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.
retrieve/get/readViewCount
are ever so slightly more descriptive names, but don't bother updating unless you also think it's worth to.
Looks good to me functionally, found a missing semicolon I didn't bother to mention because I think there isn't anything we need to do before this goes out. 👍 |
* Delete inline class toggle JS and spec * Delete global bar markup and module for dismissing/storing view count * Remove styles We don’t intend to use this again soon. Reverts #769
There are two parts to this:
location.pathname
can be mockedRules about the bar: