-
Notifications
You must be signed in to change notification settings - Fork 20
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
Upgrade to govuk frontend 5.1 #4041
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
May 31, 2024 11:00
c017184
to
1bbeb47
Compare
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
June 4, 2024 09:04
c3dc0e1
to
81dcab5
Compare
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
June 6, 2024 14:04
81dcab5
to
02d2ef7
Compare
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
June 14, 2024 13:07
02d2ef7
to
7f286c4
Compare
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
June 14, 2024 14:14
7f286c4
to
7960266
Compare
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
June 17, 2024 08:47
e868873
to
6defcdf
Compare
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
June 19, 2024 07:14
23408e9
to
bcc574e
Compare
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
June 24, 2024 08:23
bcc574e
to
30d6d60
Compare
kevindew
reviewed
Jun 27, 2024
app/views/govuk_publishing_components/components/_layout_for_admin.html.erb
Outdated
Show resolved
Hide resolved
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
July 5, 2024 13:12
29e8b07
to
39f1732
Compare
andysellick
reviewed
Jul 5, 2024
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.
Good clear commit descriptions, changes seem comprehensive and understandable. Just have a few minor comments, but nothing show stopping. Didn't take as long to review as thought - happy to do a second pass if you want. Good job 👏
app/views/govuk_publishing_components/components/layout_for_public/_header_logo.html.erb
Outdated
Show resolved
Hide resolved
app/assets/stylesheets/govuk_publishing_components/components/_warning-text.scss
Show resolved
Hide resolved
app/assets/javascripts/govuk_publishing_components/analytics-ga4/ga4-ecommerce-tracker.js
Show resolved
Hide resolved
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
July 9, 2024 10:18
39f1732
to
e717f1c
Compare
In govuk-frontend v5 the asset path has changed from: `govuk-frontend/govuk/` to `govuk-frontend/govuk/dist` so we have to change references to the old path.
In the govuk-frontend v5, the .js of UMD files has been changed to use the "*.bundle.js" suffix. This means we have to update references to those UMD files in the JS of the components.
As of govuk-frontend v5, the details component does not use JS and the .js file has been removed from the package. Remove the `data-module="govuk-details"` attribute See 'Remove references to the JavaScript for the Details component' from https://github.com/alphagov/govuk-frontend/releases/tag/v5.0.0
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour` mixin. Continuing to use it produces warnings for each instance of it in the CSS. It can be safely removed as alphagov/govuk-frontend#3576 notes that `it is non-operational and the parameter is only maintained to prevent compilation errors`.
govuk-frontend v5 introduces some changes to the tag component. The one that has the most impact, is that the tag is now designed not to use ALL CAPS text anymore. Instead it is to be titlecase. In addition there are new background colours introduced which means that the styling for some tags in govuk_publishing_components has to be changed to maintain sufficient contrast. Tests have been updated to anticipate titlecase tag text instead of ALL CAPS text.
In govuk-frontend v5, the new link styles are default and this feature flag has been removed.
In govuk-frontend v5, the `govuk-warning-text__assistive` has been removed. Instead `govuk-visually-hidden` is to be used. Tests that look for `govuk-warning-text__assistive` have been updated.
This is a recommend change in v5.1.0 of govuk-frontend - https://github.com/alphagov/govuk-frontend/releases/tag/v5.1.0
Add new config to state if application is loading ES6 components in 'es6-components.js'. If set to true `layout_for_public` and `layout_for_admin` will include a script tag with type `module` that points to `es6-components.js`. If this config is set to true and there is no `es6-components.js` then an error will occur.
In govuk-frontend v5, the names of the app icons have changed and need to be updated. Also added the new `favicon.svg` which is new for v5.
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text. - Updated the crown logo SVG in required components - Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134 -Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage - Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
Copied polyfills from v4 of govuk-frontend to allow them to still be used where required. References to the polyfill website have been removed, the map files have also been removed as these are not required and also contained references to the polyfill website.
In govuk-frontend v5.0.0, JS polyfills have been removed so therefore we need to update all references use the polyfills copied from govuk-frontend v4 where required.
- Use ES6 for loop in accordion component
In v5.1.0 `.govuk-checkboxes` now uses flexbox, this caused a rendering issue when using nested checkboxes. Nested checkboxes are not included as part of the design system and the `.govuk-checkboxes--nested` CSS class only exists in the publishing components gem. Updating the width to 100% we fix the alignment issue.
In V5 of the design system, checkboxes use flexbox, `display: flex`. In the option select component, visibility of the checkbox is set using either `display: none` or `display: block`. Setting the value to `block` caused rendering issues when focusing a checkbox item. Setting the value to `flex` aligns with the approach used in the design system and fixes the rendering issue.
The focus style of the super navigation header logo was not rendering correctly in the super navigation header menu, but it was styled correct in the layout-header component. To fix this, the line height is set to 1 in `.gem-c-layout-super-navigation-header__header-logo`, this matches the approach used for `.govuk-header` in the layout-header from the design system.
- Added support for custom filename when using es6 components - Added tests to check the correct file is used when using either a custom filename or using the default filename - Documented the new input option for custom es6 component file names - Moved the new es6-components js file to the head section of template
MartinJJones
force-pushed
the
upgrade-to-govuk-frontend-5.1
branch
from
July 15, 2024 08:31
70e84de
to
0f3cedc
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Make changes in govuk_publishing_components to allow us to update to govuk-frontend v5.1.0.
Trello
Asset Changes
assets/images/govuk-apple-touch-icon.png
assets/images/govuk-apple-touch-icon-152x152.png
assets/images/govuk-apple-touch-icon-167x167.png
assets/images/govuk-apple-touch-icon-180x180.png
assets/images/govuk-mask-icon.svg
assets/images/govuk-icon-180.png
assets/images/govuk-icon-mask.svg
layout_super_navigation_header
Javascript Changes
details.js
from govuk-frontend as thedetails
component no longer uses Javascriptoption-select.js
to ensure checkboxes render as intended when focusedStylesheet Changes
$legacy
from thegovuk-colour
mixin (this has been removed from govuk-frontend)$govuk-new-link-styles
feature flag (new link styles now default, feature flag removed)Component specific changes
tag
component nowtitleize
the tag text to adopt the new tag designheader_logo
andphase_banner
componentslayout_header
andphase_banner
gem-c-environment-tag
to ensure contrast (default tag colour is now black).warning_text
component no longer uses thegovuk-warning-text__assistive
classgovuk-warning-text__assistive
withgovuk-visually-hidden
as specified in the release notesConfig changes
use_es6_components
layout_for_public
will try to load the ES6 (govuk-frontend) using components from the filees6-components.js
Update the "Install and use this gem" docs to include further JS guidance
Polyfills copied from GOV.UK Frontend v4
In GOV.UK Frontend v5 polyfills have been removed, see “Verify your code does not rely on polyfills we have now removed” in the release notes for 5.0.0.
The polyfills originally provided by GOV.UK Frontend v4 have been copied to the publishing-components gem. This will allow you to still use the existing polyfills where required.
What I Haven't Changed
This PR is for things that have to change. To keep the PR size as manageable as possible (sorry, it is still very long) there are pieces of work that I think should be separate PRs:
Does not use the new manifest.json file provided in v5 of govuk-frontend
This PR does not implement the new manifest.json file provided in v5 of govuk-frontend. The main reason for this is that the icon names included in the manifest.json are hardcoded and do not have the correct file name once fingerprinted.
Taking
favicon.svg
as an example, the filename for this in the assets folder would befavicon-[SHA].svg
, but the reference in manifest.json would still befavicon.svg
This would result in the error below:
There is a GitHub issue logged for this in govuk-frontend - alphagov/govuk-frontend#5024
We would likely need a custom solution to be able to create our own manifest.json file in the future.
In this PR we mostly keep things the same as before for our icons, and implement the changes below:
favicon.svg
iconicongovuk-icon-192.png
andicongovuk-icon-512.png
, further details below:Component guide does not support Grade X browsers
Opening the component guide in browsers that don't support
type="module"
will result in some ungraceful erroring. This is because in this PR there isn't a way to individually load the JS that contains ES6 code. In frontend rendering applications I have established a pattern to follow to do this and it could be implemented in them gem also (but in a separate PR)Why
There are several breaking changes in the latest version of gouk-frontend which requires us to update govuk_publishing_components. Please refer to the full release notes for v5.0.0 and 5.1.0. The changes that are specifically being addressed here are:
Visual Differences
Percy: https://percy.io/55ba5e1a/govuk_publishing_components/builds/34630046?utm_campaign=55ba5e1a&utm_content=govuk_publishing_components&utm_source=github_status_public
Layout Header
The reason for the visual change here is that
.gem-c-header__logotype
was causing the line-height to increase, now this class is removed, the total height of the layout header has decreased from 72.66px to exactly 70px.Before
After
Public Layout
Percy is also showing a visual change in public layout wherever the layout header component is used, this is to be expected following the change mentioned above.
Phase Banner
The phase banner will now use the new design included in v5 of the govuk-frontend.
Form inputs
The visual changes shown in Percy for forms inputs are expected and come from changes made in v5.1.0 of govuk-frontend:
There is a slight alignment bug with the left border when conditional radio or checkboxes are used, this was logged as an issue and fixed in govuk-frontend - alphagov/govuk-frontend#5069