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

[WIP] [SPIKE] Replace header with GOV.UK One Login header #4915

Closed
wants to merge 10 commits into from

Conversation

domoscargin
Copy link
Contributor

@domoscargin domoscargin commented Apr 2, 2024

This provides a base to work off.

Copy link

github-actions bot commented Apr 2, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 119.08 KiB
dist/govuk-frontend-development.min.js 44.45 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 91.06 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 85.61 KiB
packages/govuk-frontend/dist/govuk/all.mjs 4.17 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 119.07 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 44.44 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 81.34 KiB 42.4 KiB
accordion.mjs 22.71 KiB 12.85 KiB
button.mjs 5.98 KiB 2.69 KiB
character-count.mjs 22.4 KiB 9.92 KiB
checkboxes.mjs 5.83 KiB 2.83 KiB
error-summary.mjs 7.89 KiB 3.46 KiB
exit-this-page.mjs 17.1 KiB 9.26 KiB
header.mjs 8.13 KiB 4.81 KiB
notification-banner.mjs 6.26 KiB 2.62 KiB
password-input.mjs 15.15 KiB 7.25 KiB
radios.mjs 4.83 KiB 2.38 KiB
skip-link.mjs 4.39 KiB 2.18 KiB
tabs.mjs 10.13 KiB 6.11 KiB

View stats and visualisations on the review app


Action run for 6a97262

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@selfthinker
Copy link

selfthinker commented Apr 12, 2024

I have started to look at the accessibility of the kitchen sink header. I haven't done any testing yet but have looked through the code. (I will do the testing either later today or on Wednesday and then add the results here.)

As this is not production code yet, none of the below needs to necessarily be fixed before the working group review. But it would be good to make them aware of them.

Language switcher

  • The language switcher should be a button (or have a role=button with the corresponding JavaScript).
  • The language switcher doesn't say what it is and what it does. Its label is currently just "English" (or "Current language: English" for screen reader users, plus that it's expandable). I'd say that doesn't describe "topic or purpose" well enough. We should think about adding something that says it's about the language (icon and/or words), especially for users with cognitive impairments. The fact that it's expandable is probably conveyed well enough with the chevron and aria-expanded. It would be good to confirm with user research.
  • I wonder if it might be worth calling the language switcher area out as its own landmark. But I'm not sure about that.

Main navigation

  • The active item in the main navigation should have an aria-current attribute. If it should be set to 'page' or just 'true' depends on if the active item is active because we're on that exact page or if we're on a child page that is under that parent page. (Do we know that already? I suspect we'll give guidance for both cases?) Although that could also be solved by adding a visually hidden "Current page: " or similar before the item.
  • Something not necessary as I seem to be the only person on this planet doing this, but I like to add strong around an active item (just in the code, not visually) just so that people who use userstyles (or don't use styles at all, like Lynx users) also get the benefit of visually understanding that it's an active item.
  • The label of the landmark is currently just "Menu". From what I understand this will pretty much always be the main menu? I would make the label more specific then, so "Main menu" or "Service menu".

One Login navigation

  • In the mobile view or when zoomed in, the dropdown is very far away from the trigger. This is likely to get missed by some magnification users.
  • The mobile One Login trigger doesn't make it very clear what it is either. The label adds the word "menu" for screen reader users. But for visual users it looks nearly the same as the desktop version. The visual chevron might make it clear enough. That's maybe something to explore in user research as well.

@querkmachine
Copy link
Member

querkmachine commented Apr 12, 2024

My thoughts. Notwithstanding any further changes, just rather how it ended up as it is.

Language switcher

  • The language switcher should be a button (or have a role=button with the corresponding JavaScript).

Agreed. This was just a corner that was cut for the purposes of the spike.

My thinking is that this will probably have to fallback to being a link for no-JS scenarios anyway, as the dropdown menu just won't work in that situation, similar to how GOV.UK's mega-menu works; but doing that also requires the service to provide a dedicated page to fall back to, which I feel very iffy about generally.

  • The language switcher doesn't say what it is and what it does. Its label is currently just "English" (or "Current language: English" for screen reader users, plus that it's expandable). I'd say that doesn't describe "topic or purpose" well enough. We should think about adding something that says it's about the language (icon and/or words), especially for users with cognitive impairments. The fact that it's expandable is probably conveyed well enough with the chevron and aria-expanded. It would be good to confirm with user research.

Also agree. It seems commonplace for language switcher interfaces (including virtually all of them on GOV.UK and other public service websites) to display the currently in-use language (or alternate language, in the case of toggles) rather than using explicit labelling like "Select language", so I tried to stay within that kind of wheelhouse.

I considered making the screen reader text be something like "Change language. Current language: English" but was worried about that being too verbose.

I also considered using iconography to make it more visually distinct, but Frontend's general 'phobia' of using icons, plus there being (in my experience) no universally accepted icon representing languages or translations meant I didn't pursue it.

  • I wonder if it might be worth calling the language switcher area out as its own landmark. But I'm not sure about that.

Can do! I assume this is a use case for role="region" (probably needing an aria-label too, as we have no distinct heading to reference?)

Main navigation

  • The active item in the main navigation should have an aria-current attribute. If it should be set to 'page' or just 'true' depends on if the active item is active because we're on that exact page or if we're on a child page that is under that parent page. (Do we know that already? I suspect we'll give guidance for both cases?) Although that could also be solved by adding a visually hidden "Current page: " or similar before the item.

I omitted adding aria-current as I was unsure whether it could be used for child pages of the navigation item.

If using true is allowed for that situation, I can default to that, though I'm not sure how easy it will be to know whether it should be true or page. It will probably have to be up to authors to decide which one is the correct one to use for the current situation.

  • Something not necessary as I seem to be the only person on this planet doing this, but I like to add strong around an active item (just in the code, not visually) just so that people who use userstyles (or don't use styles at all, like Lynx users) also get the benefit of visually understanding that it's an active item.

We can probably do that without too much faff, sure.

  • The label of the landmark is currently just "Menu". From what I understand this will pretty much always be the main menu? I would make the label more specific then, so "Main menu" or "Service menu".

This is carried over from the existing header component, which just uses "Menu". I guess if we also have the One Login menu we'd want to differentiate it from that?

I wonder if the average GOV.UK user would recognise the word 'service' in that context, given the single-website perception that GOV.UK tries to give off.

One Login navigation

  • In the mobile view or when zoomed in, the dropdown is very far away from the trigger. This is likely to get missed by some magnification users.

This is carried over from the One Login header, though this is also true of the present Design System header component. Having the toggle on the right and menu items on the left seems a common enough pattern across responsive websites that I'm curious if it actually does present an issue to magnification users or is at worst an annoyance.

I'm a little hesitant to change anything with that, if just that I'm not sure what 'better' would look like in this case (Move the menu dropdown to below the logo? Make the navigation items right-aligned? Use visuals to connect them together somehow?)

  • The mobile One Login trigger doesn't make it very clear what it is either. The label adds the word "menu" for screen reader users. But for visual users it looks nearly the same as the desktop version. The visual chevron might make it clear enough. That's maybe something to explore in user research as well.

Also carried over from the One Login header, and also not sure what might be an improvement here. There is unfortunately not a lot of free space here on mobile screens, so any visible label longer than "One Login" isn't guaranteed to fit on those narrow viewports.

Unrelatedly, I find it very weird that One Login opted to place the chevron to the left of the label here, when these are pretty much universally on the right otherwise. I'd like to know what the reasoning behind that was.

- Wrap active links in `strong` tags so that there is a non-CSS way of indicating the currently active link.
- Add `aria-current="true"` to active links. No way of setting this to `page` currently, though. Does not apply to links-as-dropdowns.
- Add ability to add attributes to parent list items. Use this to add `role="region"` and `aria-label` to language dropdown examples.
- Update the screen reader text for the language dropdown toggle.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4915 April 12, 2024 14:11 Inactive
@stevenjmesser
Copy link

Header kitchen sink made me burst out laughing, bravo 👏👏👏

@querkmachine
Copy link
Member

Closing this spike as it's now served its purpose. We'll be iterating upon this concept in a new branch.

@selfthinker
Copy link

Sorry, I know this is over 3 weeks old. I just wanted to comment on one particular thing. And doing it here (even though the PR is closed) seems to fit better than doing it elsewhere without the context.

I omitted adding aria-current as I was unsure whether it could be used for child pages of the navigation item.

If using true is allowed for that situation, I can default to that, though I'm not sure how easy it will be to know whether it should be true or page. It will probably have to be up to authors to decide which one is the correct one to use for the current situation.

I'm actually not too sure about the use of aria-current myself. The ARIA spec definitely doesn't make any distinction between being on the current page or within sub pages of the current page. I assume when aria-current was created that this was not thought about.

That means the main question is what users expect when they encounter it. I tried to search for an answer to that (3 weeks ago) and couldn't find anything. I will try to dig a bit deeper.

But my assumption is that having something that is sort of the ARIA equivalent of that visual highlighting is better than having no equivalent, even if that might not be 100% correct because the actual equivalent is missing from the ARIA spec.

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.

None yet

5 participants