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

Use devolved nations component in place of important metadata component on publication page #2220

Conversation

jon-kirwan
Copy link
Contributor

@jon-kirwan jon-kirwan commented Sep 9, 2021

What

https://trello.com/c/WVrZ35zt/793-dev-implementation-of-devolved-nations-task

Replace uses of 'Metadata block component' for new 'Devolved Nations component' for linking to guidance for other nations.

This change takes effect on the following page types -

When no advice is available the component is not rendered e.g. https://www.gov.uk/government/publications/foi-release-technology-and-outserviced-contracts

Why

  • The list of countries are being read out without providing any context that individual guidance is available
  • This task is to implement a fix to aid screen-reader users

Visual changes

Before After

Anything else

@bevanloon bevanloon temporarily deployed to government-f-use-devolv-qs0syw September 9, 2021 15:08 Inactive
@jon-kirwan jon-kirwan force-pushed the Use-devolved-nations-component-in-place-of-important-metadata-component-on-publication-page branch from d9c136e to 5124724 Compare September 10, 2021 12:06
@bevanloon bevanloon temporarily deployed to government-f-use-devolv-qs0syw September 10, 2021 12:07 Inactive
@bevanloon bevanloon temporarily deployed to government-f-use-devolv-qs0syw September 10, 2021 12:31 Inactive
@bevanloon bevanloon temporarily deployed to government-f-use-devolv-qs0syw September 10, 2021 13:22 Inactive
@jon-kirwan jon-kirwan force-pushed the Use-devolved-nations-component-in-place-of-important-metadata-component-on-publication-page branch from 3463491 to 1bbbdb9 Compare September 13, 2021 08:33
@bevanloon bevanloon temporarily deployed to government-f-use-devolv-qs0syw September 13, 2021 08:34 Inactive
@jon-kirwan jon-kirwan marked this pull request as ready for review September 13, 2021 08:49
@jon-kirwan jon-kirwan requested a review from maxgds September 13, 2021 09:47
@maxgds
Copy link
Contributor

maxgds commented Sep 14, 2021

Can you explain to me how you identified the formats that are using the important metadata for rendering devolved nations info? I can see that it's used across a number of formats and while I can see some are clearly using it for a different purpose (eg the Fatality notices: https://www.gov.uk/government/fatalities/sergeant-eddie-collins-killed-in-iraq) I can't see how/where it's used on the likes of document collections: https://www.gov.uk/government/collections/severn-tidal-power-feasibility-study-conclusions

I'm intrigued by the "temp" commit that seems to be doing the sort of stuff I'd expect to be handled by dependabot PRs. I think I saw a reference to it needing to be deleted at one point on this PR?

Finally (for now!) a couple of minor points on the PR detail -

  1. I don't think the metadata component itself is especially inaccessible, it was more that the way we were using it for this particular data didn't work well as the links were unhelpfully labelled.
  2. We should link our PRs back to the Trello card so it's easy to trace further info. Sometimes putting the trello card url on the PR auto-links it for us and saves us that job, but the magic that drives that hasn't been very reliable recently.

@jon-kirwan
Copy link
Contributor Author

jon-kirwan commented Sep 14, 2021

Can you explain to me how you identified the formats that are using the important metadata for rendering devolved nations info? I can see that it's used across a number of formats and while I can see some are clearly using it for a different purpose (eg the Fatality notices: https://www.gov.uk/government/fatalities/sergeant-eddie-collins-killed-in-iraq) I can't see how/where it's used on the likes of document collections: https://www.gov.uk/government/collections/severn-tidal-power-feasibility-study-conclusions

Yep, so I've gone about this in two ways -

@jon-kirwan jon-kirwan force-pushed the Use-devolved-nations-component-in-place-of-important-metadata-component-on-publication-page branch from 1bbbdb9 to 437565e Compare September 14, 2021 17:10
@govuk-ci govuk-ci had a problem deploying to government-f-use-devolv-qs0syw September 14, 2021 17:10 Failure
@jon-kirwan
Copy link
Contributor Author

I'm intrigued by the "temp" commit that seems to be doing the sort of stuff I'd expect to be handled by dependabot PRs. I think I saw a reference to it needing to be deleted at one point on this PR?

Ah I'd added this as a temporary way to test the new component (prior to the dependabot merge) and forgot to go back and remove it once the version had been bumped! I've now removed the extra commit.

@jon-kirwan
Copy link
Contributor Author

  1. I don't think the metadata component itself is especially inaccessible, it was more that the way we were using it for this particular data didn't work well as the links were unhelpfully labelled.

I've deleted from the 'Why' section - 'Currently, this "component" isn't accessible to screen-reader users.' so it's hopefully now clearer that the fail is related to the hyperlink context rather than the actual component itself.

  1. We should link our PRs back to the Trello card so it's easy to trace further info. Sometimes putting the trello card url on the PR auto-links it for us and saves us that job, but the magic that drives that hasn't been very reliable recently.

Trello link now included in the description.

@maxgds
Copy link
Contributor

maxgds commented Sep 15, 2021

Yep, so I've gone about this in two ways -

Great, that makes sense and I can't see any gaps after looking through the list. If it turns out there are it'll be easy enough to update further template types.

@maxgds
Copy link
Contributor

maxgds commented Sep 15, 2021

I notice you've added a json lockfile in the root that wasn't there before. I assume that's an error, can you remove it (or justify its presence)?

@jon-kirwan jon-kirwan force-pushed the Use-devolved-nations-component-in-place-of-important-metadata-component-on-publication-page branch from 437565e to 6ff3812 Compare September 15, 2021 08:36
@govuk-ci govuk-ci had a problem deploying to government-f-use-devolv-qs0syw September 15, 2021 08:37 Failure
@jon-kirwan jon-kirwan force-pushed the Use-devolved-nations-component-in-place-of-important-metadata-component-on-publication-page branch from 6ff3812 to 64fa451 Compare September 15, 2021 08:53
@govuk-ci govuk-ci temporarily deployed to government-f-use-devolv-qs0syw September 15, 2021 08:54 Inactive
@jon-kirwan
Copy link
Contributor Author

I notice you've added a json lockfile in the root that wasn't there before. I assume that's an error, can you remove it (or justify its presence)?

Apologies that's a mistake - i've now removed it.

@jon-kirwan jon-kirwan force-pushed the Use-devolved-nations-component-in-place-of-important-metadata-component-on-publication-page branch from 64fa451 to acabc7f Compare September 15, 2021 19:23
@govuk-ci govuk-ci temporarily deployed to government-f-use-devolv-qs0syw September 15, 2021 19:23 Inactive
@maxgds maxgds merged commit fa5f29b into main Sep 16, 2021
@maxgds maxgds deleted the Use-devolved-nations-component-in-place-of-important-metadata-component-on-publication-page branch September 16, 2021 09:41
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.

4 participants