DFE-Digital / schools-experience

The Department for Education's Get Schools Experience Service which allows you to find out more about teaching by visiting schools.
https://dfe-digital.github.io/schools-experience/
MIT License
9 stars 4 forks source link

Implement GOV.UK Frontend v5.0 on Get school experience and Manage school experience #3109

Closed ekumachidi closed 3 months ago

ekumachidi commented 3 months ago

Trello card

https://trello.com/c/Z0TJAkll/

Context

GDS have announced that they have released a new version of GOV.‌UK Frontend v5.0.0 (breaking release) We need to upgrade Get school experience / Manage school experience to GOV.UK Frontend v5.0.0

Changes proposed in this pull request

Guidance to review

MylesJarvis commented 3 months ago

I'll add comments as and when I find something if that's ok. I believe the Service Name needs to be bolded (and a larger heading), as does the GOV.UK element: image

Guidance on this from the design system: https://design-system.service.gov.uk/styles/page-template/

We should also capitalise the word 'beta'.

MylesJarvis commented 3 months ago

On the candidate search page, for example this search URL, we're seeing some spillover on the filter option.

image

These should be aligned next to the check boxes, even with long text lengths, for example this in current implementation:

image

MylesJarvis commented 3 months ago

In the 'withdrawn requests' page, we have an issue with contrast on the 'unviewed' request. This won't pass accessibility requirements for contrast, so needs to be fixed. We also ned to ensure it mirrors the implementation where we have capitalised first letter, lower case rest for the tag.

Current prod: image

vs Review app:

image

MylesJarvis commented 3 months ago

This appears to be an existing issue (or was perhaps introduced by other version bumps), but our tables are spilling over when there is a large amount of content and the user has a small screen. We'll need to make sure these are responsive to screen size. @gemmadallmandfe thoughts on whether we need to amend the accessibility statement with this in mind?

MylesJarvis commented 3 months ago

Another tag related issue in the Manage dates section of the service (/schools/placement_dates)

We need to ensure that contrast is sufficient in this 'Open' Tag.

image

Update: The closed tag also has low contrast that needs fixing.

image
MylesJarvis commented 3 months ago

@ekumachidi there's a checklist of actions to take for the frontend upgrade here for v5.0.0. I've pulled this from the release notes.

I think it's worth going through this to double check everything, for example the one below:

Update references to the govuk-warning-textassistive class from the HTML for the Warning Text component For the Warning Text component, we've removed the govuk-warning-textassistive class and its styles from GOV.UK Frontend. This class is unnecessary, as it duplicates the functionality of the govuk-visually-hidden class already present in Frontend.

If you're not using Nunjucks macros, update your warning text HTML to replace the govuk-warning-text__assistive class with the govuk-visually-hidden class.

This change was introduced in https://github.com/alphagov/govuk-frontend/pull/3569.

We need to change govuk-warning-text__assistive -> govuk-visually-hidden. A quick search on the codebase finds 11 of these references.

MylesJarvis commented 3 months ago

Looking at the release notes more, we also have this issue:

  • Remove references to the JavaScript for the Details component The Details component no longer uses JavaScript, and is no longer polyfilled in older browsers.

If you are importing the JavaScript for this component individually, remove any references to it.

If you are not using our Nunjucks macros, remove the data-module="govuk-details" attribute from all <details> elements.

We've styled the details component so content does not look 'broken' in browsers that do not support it. If your service supports these browsers, you will need to add your own polyfills.

This change was introduced in:

https://github.com/alphagov/govuk-frontend/pull/3766. https://github.com/alphagov/govuk-frontend/pull/3758

We have 2 references to data-module="govuk-details" in the codebase, specifically at /app/views/schools/availability_preferences/edit.html.erb.

ekumachidi commented 3 months ago

This appears to be an existing issue (or was perhaps introduced by other version bumps), but our tables are spilling over when there is a large amount of content and the user has a small screen. We'll need to make sure these are responsive to screen size. @gemmadallmandfe thoughts on whether we need to amend the accessibility statement with this in mind?

get-school-experience-review-pr-3109 test teacherservices cloud_candidates_dashboard(iPhone XR)

@MylesJarvis I could not replicate this, please, can you confirm it still happens for you

MylesJarvis commented 3 months ago

This appears to be an existing issue (or was perhaps introduced by other version bumps), but our tables are spilling over when there is a large amount of content and the user has a small screen. We'll need to make sure these are responsive to screen size. @gemmadallmandfe thoughts on whether we need to amend the accessibility statement with this in mind? get-school-experience-review-pr-3109 test teacherservices cloud_candidates_dashboard(iPhone XR)

@MylesJarvis I could not replicate this, please, can you confirm it still happens for you

Thanks @ekumachidi, I've re-tested this and it appears to be only when I use the 'inspect' function on Chrome, not normal usage. I think we can remove this one from the to-do list!

MylesJarvis commented 3 months ago

@ekumachidi just pushed a small change around capitalisations on the labels, which I hadn't flagged previously. Everything else looks good to me so I'll give this a tick once it's deployed and I've given it a final once over.

github-actions[bot] commented 3 months ago

Review app deployed to https://get-school-experience-review-pr-3109.test.teacherservices.cloud :white_check_mark: DfE sign in route obtained: https://get-school-experience-review-pr-2.test.teacherservices.cloud

ekumachidi commented 3 months ago

I've had a check through against the release notes LGTM 👍

Only final question is around removing the aria-labelledby in the accordion functionality. Can you let me know where you saw that @ekumachidi? It doesn't seem to have changed the code that is output, so just curious.

Thank you @MylesJarvis I got that from the release google sheets checklist

MylesJarvis commented 3 months ago

Thanks @ekumachidi, I missed that one in the 5.1.0 release notes, apologies. All good to go 👍