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

Remove default font styles from cookie banner Sass #2432

Merged
merged 2 commits into from
Nov 24, 2021

Conversation

vanitabarrett
Copy link
Contributor

@vanitabarrett vanitabarrett commented Nov 16, 2021

Closes #2358

What

Remove the default font styles (setting the font and font-size) from the cookie banner Sass.

Why

With the default font styles present, it's very difficult to spot if you forget to add the relevant govuk-body styles to any html you pass into the component. We had missed this ourselves when we added the cookie banner to the design system.

Screenshots

Before change - cookie banner with html, without govuk-body classes

Screenshot 2021-11-16 at 15 13 59

After change - cookie banner with html, without govuk-body classes

Screenshot 2021-11-16 at 13 02 32

After change - cookie banner with html, with the correct govuk-body classes

Screenshot 2021-11-16 at 13 02 53

After change - in the Design System

This work was prompted by when we forgot to add our own styling classes to the cookie banner on the design system site, which resulted in slightly incorrect text spacing. This is what the design system cookie banner would now look like if we forgot to add the relevant classes:

Screenshot 2021-11-16 at 13 58 39

Details of breaking change

This is a breaking change because any users who are relying on the default styles will see unstyled text in their cookie banner when they update. Users will need to add the correct classes to style the text - this will probably involve adding the govuk-body class to any HTML paragraph tags they're passing through to the component.

  • which users are affected: users who are using the cookie banner component and passing their own html into the cookie banner component
  • the change that’s been made: we've removed the default font styles from the cookie banner component
  • changes users will have to make: users will need to make sure that any HTML they pass into the cookie banner component has the appropriate classes/styles (e.g: govuk-body)
  • impact of users not making those changes: cookie banners will appear with unstyled text

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 16, 2021 13:59 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 16, 2021 15:32 Inactive
@vanitabarrett vanitabarrett marked this pull request as ready for review November 16, 2021 15:36
@vanitabarrett vanitabarrett requested a review from a team November 18, 2021 09:43
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks @vanitabarrett 👍

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@vanitabarrett vanitabarrett force-pushed the make-missing-cookie-banner-classes-clearer branch from b684dbe to cba080d Compare November 24, 2021 10:03
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 24, 2021 10:03 Inactive
@vanitabarrett vanitabarrett force-pushed the make-missing-cookie-banner-classes-clearer branch from cba080d to 15a3812 Compare November 24, 2021 10:04
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 24, 2021 10:04 Inactive
Removes the default font styles (setting the font and font-size) from
the cookie banner Sass.

This work was prompted by alphagov/govuk-design-system#1886

With the default font styles present, it's very difficult to spot if you forget
to add the relevant `govuk-body` styles to any `html` you pass into the component.
We had missed this ourselves when we added the cookie banner to the design system.

This change should only affect users who are passing `html` to the cookie banner.
@vanitabarrett vanitabarrett force-pushed the make-missing-cookie-banner-classes-clearer branch from 15a3812 to b0fb211 Compare November 24, 2021 10:05
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 24, 2021 10:05 Inactive
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@vanitabarrett vanitabarrett force-pushed the make-missing-cookie-banner-classes-clearer branch from b0fb211 to 973b462 Compare November 24, 2021 11:18
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 24, 2021 11:18 Inactive
CHANGELOG.md Outdated Show resolved Hide resolved
@vanitabarrett vanitabarrett force-pushed the make-missing-cookie-banner-classes-clearer branch from 973b462 to 2d24d6b Compare November 24, 2021 11:37
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 24, 2021 11:38 Inactive
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@EoinShaughnessy EoinShaughnessy left a comment

Choose a reason for hiding this comment

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

Just suggesting a last tiny change, otherwise content is golden. :)

@vanitabarrett vanitabarrett force-pushed the make-missing-cookie-banner-classes-clearer branch from 2d24d6b to 66ea6e3 Compare November 24, 2021 13:27
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2432 November 24, 2021 13:27 Inactive
@vanitabarrett vanitabarrett merged commit 0dcf65e into main Nov 24, 2021
@vanitabarrett vanitabarrett deleted the make-missing-cookie-banner-classes-clearer branch November 24, 2021 13:30
@vanitabarrett vanitabarrett mentioned this pull request Dec 15, 2021
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.

Difficult to notice when cookie banner HTML classes are missing
4 participants