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

[DO NOT MERGE] Refactor organisation styles and upgrade to govuk-frontend v5.7.1 #4321

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

AshGDS
Copy link
Contributor

@AshGDS AshGDS commented Oct 21, 2024

What / Why

  • Removes generic .brand__color class, which was used to give organisations custom link colours from govuk-frontend. However we keep a namespaced .brand__color style for prime-ministers-office-10-downing-street, so that links will remain black on the no 10 page only.
  • Also removes the brand option from the share_links component, and replaces it with black_icons and black_links. These are needed because the default brand styles are now going to be a black icon with our standard blue links, but the No 10 page needs black links as well, hence needing both options. Only collections used the brand variant of this component, so this shouldn't break any other pages. Using these new options in collections is simple: Update share links branding collections#3802
  • The two above points will introduce minor tech debt in the sense that collections will be passing brand: to the share links component when that is no longer a variant, and organisation components will have a brand__colour class on them which isn't doing anything now that they all use the default blue colour. These two things may be a backendy thing to resolve, as the brand options in collections seem quite tightly coupled to the backend/models of the org pages.
  • This also updates the DSIT class override we have, instead of removing it. We need this because the class was renamed in govuk-frontend from department-for-science-innovation-and-technology to department-for-science-innovation-technology ( - the and was dropped.) Our whitehall model still uses the old class name, and therefore renders the page using the old class name, so we need to keep this temporary class in for now. Once we've updated whitehall model to use the new class name, it will grab the styles directly from govuk-frontend allowing us to remove the class in our gem.

Testing

Visual Changes

Before

image image

After

image image

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 11:28 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 11:32 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 11:39 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 12:52 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 13:04 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 13:12 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 13:15 Inactive
@AshGDS AshGDS changed the title Fix links oct 24 Refactor organisation styles Oct 21, 2024
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 13:18 Inactive
@AshGDS AshGDS changed the title Refactor organisation styles [DO NOT MERGE] Refactor organisation styles Oct 21, 2024
@AshGDS AshGDS changed the title [DO NOT MERGE] Refactor organisation styles [DO NOT MERGE] Refactor organisation styles and upgrade to govuk-frontend v5.7.1 Oct 21, 2024
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 13:43 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 14:28 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 14:30 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 14:58 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4321 October 21, 2024 14:59 Inactive
@AshGDS AshGDS self-assigned this Oct 21, 2024
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.

Looks pretty comprehensive, just a few comments.

docs/component_branding.md Outdated Show resolved Hide resolved
docs/component_branding.md Show resolved Hide resolved
@AshGDS
Copy link
Contributor Author

AshGDS commented Oct 22, 2024

Thanks @andysellick - should be ready for re-review 👍

@maxgds
Copy link
Contributor

maxgds commented Oct 22, 2024

In terms of the two things outlined in your PR description:
Could you raise a ticket on the tech debt board for Web to get the collections update done so we stop passing around redundant brand names?
Similarly, could you ask the Whitehall team about their plans to update the DSIT name and ask them to inform us when it's done so we can remove the temporary fix.

@AshGDS
Copy link
Contributor Author

AshGDS commented Oct 22, 2024

Thanks both 👍 @maxgds will do. Last time we did the Whitehall change ourselves, should we do it ourselves again just to reduce dependency on other teams? alphagov/whitehall#9379

@maxgds
Copy link
Contributor

maxgds commented Oct 22, 2024

Thanks both 👍 @maxgds will do. Last time we did the Whitehall change ourselves, should we do it ourselves again just to reduce dependency on other teams? alphagov/whitehall#9379

That looks a lot easier than I expected it to - yes we should be proactive if we have capacity!

Share links need to have a black icon with blue links, but the no 10 page needs to retain black links.
govuk-frontend removed the 'and' from their class. We will need to use this temporary mapping class, then change the name in Whitehall, and then remove this class.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants