-
Notifications
You must be signed in to change notification settings - Fork 7
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
105 update to latest govuk frontend version #117
base: master
Are you sure you want to change the base?
Conversation
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.
Not fully tested everything, but leaving the comments I have for now so they're not lost (as I'm not around tomorrow)
Spotted an issue with accordions (though not cross checked against main yet to see if it's pre-existing)
- Only the 'show' is clickable, if you click on the title of the accordion it doesn't open, though for GDS component it seems like clicking anywhere around the accordion title should also open it? https://design-system.service.gov.uk/components/accordion/
inst/www/images/govuk-crest.svg
Outdated
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.
Do we use this anywhere? Looks like the footer crest, though seems odd we didn't already have a version of it? Do we need to point the footer function this this?
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.
Given we just copy this in now (and we are full rem instead of norem), would it make sense to change to leaving the name as the same as the one from the GOV.UK ZIP? I think then it's just a case of updating the attach function to import this sheet as a dependency?
@@ -1,8 +1,7 @@ | |||
@charset "UTF-8"; | |||
|
|||
:root { | |||
--govuk-frontend-version: "5.4.0"; | |||
font-size: 16px; |
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.
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.
Overview of changes
Update to 5.7.1 frontend - adding @cjrace as a reviewer as you've had experience updating this before too!
Everything looks ok apart from value boxes - tested this in master and still looks odd! I've raised #116 for Andy to revisit as this looks like a separate issue.
Removed some old manual changes from the css_changes.md file as no longer relevant
PR Checklist
devtools::check()
Reviewer instructions
Run devtools::load_all
Run run_example()
Check that all css changes look ok and functionality still works