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 banner support #1583

Merged
merged 11 commits into from
Dec 5, 2018
Merged

Add banner support #1583

merged 11 commits into from
Dec 5, 2018

Conversation

steventux
Copy link
Contributor

@steventux steventux commented Dec 4, 2018

https://trello.com/c/ItwW03vg/48-build-a-generic-banner-based-on-an-older-banner-pattern

This PR resurrects previous banner code as the rules and design pattern works well for general site wide announcements which are not suitable for emergency banner content.

This can be merged as the banner is not enabled: https://github.com/alphagov/static/pull/1583/files#diff-e17896925164fa4471101aca24b30733R2

screenshot from 2018-12-05 14-24-03

This is based on code from #769

Future iterations should address:

  • Converting global-bar styles to BEM naming convention
  • Addressing inline minified JS

Steve Laing added 5 commits December 3, 2018 12:00
Steve Laing added 2 commits December 4, 2018 12:45
Move the banner up to sit directly below the GOV.UK header and hide
the blue global-header-bar element. This element will display when
the banner is dismissed or automatically hidden.
This commit moves the banner activation js and markup into one location
and adds the ability to easily edit the content of the banner.
The banner can be shown or hidden based on a boolean.
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Have noted a few frontend tidying things, shout if you want any help.

app/assets/stylesheets/helpers/_header.scss Outdated Show resolved Hide resolved
spec/javascripts/global-bar-class-toggle.spec.js Outdated Show resolved Hide resolved
app/assets/stylesheets/helpers/_header.scss Show resolved Hide resolved
app/assets/stylesheets/helpers/_header.scss Outdated Show resolved Hide resolved
@steventux steventux changed the title [Do not merge] Add banner support Add banner support Dec 5, 2018
@steventux
Copy link
Contributor Author

@andysellick are you OK with review replies/updates?

We've evaluated a couple of different design ideas and decided
on a slightly different colour for the global banner.
@andysellick
Copy link
Contributor

@steventux all but one, I've added a comment.

@andysellick
Copy link
Contributor

@steventux all looks good now.

@steventux steventux merged commit f62738d into master Dec 5, 2018
@steventux steventux deleted the add-banner-support branch December 5, 2018 18:00
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