Skip to content
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 support for displaying a banner across all of GOV.UK #947

Merged
merged 4 commits into from
Mar 14, 2017

Conversation

boffbowsh
Copy link
Contributor

@boffbowsh boffbowsh commented Mar 13, 2017

Allows for a partial to be deployed based on app/views/root/_promo_banner_example.html.erb. This should include the global-bar javascript module in order to enable view count tracking. The banner won't display alongside the survey banner or the emergency publishing banner.

app/views/root/_promo_banner.html.erb should remain empty if the banner isn't in use.

Example:

screen shot 2017-03-13 at 16 55 49

timblair and others added 4 commits March 13, 2017 16:18
- Previous banner (Register to Vote) did not display on /register-to-vote as users did not need to be directed to the page that they were already on.
- This commit removes that functionality.
This allows us to deploy the logic without deploying the copy until
it’s needed.

Override the controller’s view paths to stub the template. This is
borrowed from rspec-rails’ `stub_template` method
https://github.com/rspec/rspec-rails/blob/e8054a1cd03044f725030fe8315952
cf3799a395/lib/rspec/rails/example/view_example_group.rb#L86-L88
@boffbowsh boffbowsh requested review from timblair and fofr March 13, 2017 17:21
count = count + 1;
GOVUK.setCookie(GLOBAL_BAR_SEEN_COOKIE, count, {days: 84});

if (count == 2) {
Copy link
Contributor

@fofr fofr Mar 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we auto-dismiss after 2/1 view rather than 3? Is the banner informative rather than a call to action?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be configured based on the need for the banner. Currently people are happy with 3 as a default

@@ -128,6 +128,7 @@
init: function() {
var activeSurvey = userSurveys.getActiveSurvey(userSurveys.defaultSurvey, userSurveys.smallSurveys);
if (userSurveys.isSurveyToBeDisplayed(activeSurvey)) {
$('#global-bar').hide(); // Hide global bar if one is showing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently have some fairly aggressive surveys running (ie every 2 pageviews). We should avoiding running these types of survey during a banner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we've checked on that 👍

Copy link
Contributor

@fofr fofr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor comments, otherwise 👍

@boffbowsh boffbowsh merged commit bb2e0b0 into master Mar 14, 2017
@boffbowsh boffbowsh deleted the global-banner branch March 14, 2017 09:17
@fofr fofr mentioned this pull request May 23, 2017
fofr added a commit that referenced this pull request May 24, 2017
Remove global bar JS, styles, templates and tests

* Global bar used for EU referendum was brought back for Article 50
notification – #947
* It was partially removed in #1003
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants