alphagov / govuk-design-system

One place for service teams to find styles, components and patterns for designing government services.
https://www.gov.uk/design-system
MIT License
474 stars 226 forks source link

Examine and resolve accessibility findings #3811

Open CharlotteDowns opened 1 month ago

CharlotteDowns commented 1 month ago

What

Examine and resolve accessibility findings from accessibility specialist report

Why

To ensure the Navigation component is accessible for as many users as possible.

Who needs to be involved

Developers, Designers (to help with decisions)

Who needs to review

Accessibility specialist

Done when

querkmachine commented 1 month ago

I have a PR for the breadcrumb labelling open here: https://github.com/alphagov/govuk-frontend/pull/4995

CharlotteDowns commented 1 month ago

Designer suggestions / decisions

Unlabelled breadcrumbs confusing

We could advise in guidance not to use Navigation and Breadcrumbs together, due to known issues like this.

Position of phase banner makes service name less clear

The phase banner being between the logo and the service name makes it harder to understand what the service name is. It previously would have read (simplified): "GOV.UK - Manage company information - Alpha This is a new service – your feedback will help us to improve it." Now it reads (simplified): "GOV.UK - Alpha This is a new service – your feedback will help us to improve it. - Manage company information". The service name seems "lost" at a later point. I have made a few screenshots with CSS disabled (and the landmarks showing) to hopefully make the issue more visible.

Is it possible to put the phase banner (full width) between the service name and the navigation, so it could read: "GOV.UK - Manage company information - Alpha This is a new service – your feedback will help us to improve it - Navigation"

A similar approach to this mock up of an organisation switcher, which is taking space between the service name and navigation.

Example of an organisation switcher between the service name and navigation items

Navigation difficult to discover when magnified

The navigation is currently right-aligned when it comes after the service name (in desktop view). That will make it harder for magnification users to discover. The horizontal lines help a bit. But I'd suggest thinking about left-aligning them instead.

I agree with this. There doesn't seem to be a single solution without trade offs.

If we left align the navigation (which will occur if the Navigation takes a new line, for example if the service name is long or there is a phase banner ) the vertical space of the header will increase, pushing page content down. If a service wishes to include additional service specific components, for example, a search box and account links this will be extended even more.

We are prioritising the behaviour on smaller screens where the Navigation is left aligned.

patrickpatrickpatrick commented 1 month ago

Designer suggestions

Unlabelled breadcrumbs confusing

We could advise in guidance not to use Navigation and Breadcrumbs together, due to known issues like this.

Position of phase banner makes service name less clear

The phase banner being between the logo and the service name makes it harder to understand what the service name is. It previously would have read (simplified): "GOV.UK - Manage company information - Alpha This is a new service – your feedback will help us to improve it." Now it reads (simplified): "GOV.UK - Alpha This is a new service – your feedback will help us to improve it. - Manage company information". The service name seems "lost" at a later point. I have made a few screenshots with CSS disabled (and the landmarks showing) to hopefully make the issue more visible.

Is it possible to put the phase banner (full width) between the service name and the navigation, so it could read: "GOV.UK - Manage company information - Alpha This is a new service – your feedback will help us to improve it - Navigation"

A similar approach to this mock up of an organisation switcher, which is taking space between the service name and navigation. Example of an organisation switcher between the service name and navigation items

(more thoughts to follow)

for the proposed "Position of phase banner makes service name less clear" fix, would this only be the case if there was a phase banner present or would it only be the case if a phase banner was present? I mocked up what this could look like:

Screenshot 2024-05-22 at 15 31 45

We would need to move the phase banner into the component itself for this to work. Either we could have a 'slot' that could pass any component into the 'slot' where the phase banner would be or we could take phase banner parameters as input so that only a phase banner could be put in the slot.

selfthinker commented 1 month ago

Is it possible to put the phase banner (full width) between the service name and the navigation

That wouldn't solve the issue when what's in between is other things in the global header, like the One Login menu.

CharlotteDowns commented 1 month ago

@selfthinker Is the phase banner issue still unsolved if we use option H. Generic landmark for the whole service header in this google document and suggested the phase banner live within this landmark?

selfthinker commented 1 month ago

@selfthinker Is the phase banner issue still unsolved if we use option H. Generic landmark for the whole service header in this google document and suggested the phase banner live within this landmark?

@CharlotteDowns, that particular accessibility issue with the phase banner would be solved by option H, using a generic landmark for the service header. It might still be visually or structurally confusing to have the phase banner between global and service header, but that's then a potential usability, not accessibility issue.

CharlotteDowns commented 1 month ago

With the help of @patrickpatrickpatrick I've been looking at putting the phase banner between the service name and the navigation in a hope that this, role="region" and aria-label="Service information and navigation" resolve the issue 'Position of phase banner makes service name less clear'

Here is a screenshot of an example where the phase banner is between the service name and the service navigation. an example where the phase banner is between the service name and the service navigation where the service header div element has a role=region and an aria-label=service information and navigation

Will this change in landmarks also help resolve 'NVDA in Chrome behaving weird'?

CharlotteDowns commented 1 month ago

I just also found this accessibility concern Review the use of aria-label on a <ul> element in the site navigation menu about sub navigation, will this be relevant in our work (incl. guidance)

selfthinker commented 1 month ago

I've been looking at putting the phase banner between the service name and the navigation in a hope that this, role="region" and aria-label="Service information and navigation" resolve the issue 'Position of phase banner makes service name less clear'

That generally looks good to me.

I would probably choose to use <section> instead of <div role="region"> as a section has the implicit role of 'region'. It's irrelevant for accessibility which of the two you use, but it would be more consistent with how we approach roles elsewhere (header, footer, etc).

Will this change in landmarks also help resolve 'NVDA in Chrome behaving weird'?

I doubt it. I hadn't looked further into it, so have no idea why that happens. My hunch was that it has maybe something to do with flexbox. But we cannot know more until we investigate the issue.

selfthinker commented 1 month ago

I just also found this accessibility concern Review the use of aria-label on a <ul> element in the site navigation menu about sub navigation, will this be relevant in our work (incl. guidance)

That seems to be quite complicated. From what I can tell that is pretty unconventional code. It's used for a (button) link to open and close a sub menu.

I don't think we use any such behaviour in the new component, so I don't think it is relevant. But it would be good if someone who is currently working on the navigation could confirm that. If you have access to the latest code (sorry, I couldn't find it), you can simply check if aria-label is used anywhere on a ul to know for sure.

CharlotteDowns commented 4 weeks ago

We can suggest the phase banner comes after the navigation but still included in the

'Service information and navigation'.

cc: @querkmachine @domoscargin for next cycle??

CharlotteDowns commented 3 weeks ago

Further to the discussion on Phase banner placement. We have come to the conclusion that the Phase banner should come after the service header and navigation component, ideally wrapped in a <section></section> that is labelled as 'Service information and navigation' so it is clear it relates to the service.

Here is a sketch plan for the order of things that could appear in both the GOV.UK header component and the Service header navigation component.

A plan for adding the phase banner to follow the service information and navigation content, which will order directly after the navigation

The possible objections to the approach have been that it could be deemed that a section of the website is at a certain phase rather than the service entirely. However, we wish to justify this by acknowledging that the Phase banner will persist across all pages of the service in this location and that the content within the component mentions it is service-wide.

This is a new service. Help us improve it...

selfthinker commented 2 weeks ago

NVDA in Chrome behaving weird

When using NVDA in Chrome it behaves a bit weird. It doesn't read the service name when arrowing. (It does read it when tabbing.) It also reads all the navigation items in one go, meaning I cannot navigate them individually via arrowing. (Again, it reads them when tabbing.) That is only the case for the desktop view, not for the mobile (or zoomed in) view.

I wonder if this has something to do with flexbox? It's possible it's a bug in NVDA (or Chrome?) rather than something wrong in our code. NVDA is working as expected in Firefox, and JAWS is working as expected in Chrome in this regard.

I looked into this. It turns out that some of this was fixed in Chrome version 125, so was present until version 124. I wrote back then that this didn't happen with NVDA and Firefox. But now that I re-tested it I found that's not true, it is also happening in Firefox. (I also re-tested with JAWS, which was working fine in both browsers.)

Due to the recent changes in the code, the service name is now read out, but all the navigation items are still read in one go up until version 124.

This problem appears whenever list items are set to display: inline-block. And I found it could be fixed with setting whatever is inside (so, links in our case) to display: block. If you think it's worth fixing, it would be good to check doing that doesn't have any unintended consequences.

An NVDA bug report suggests that this is intended behaviour. While I get their point that any elements in an inline-block look like they are in a paragraph and should therefore behave like text in a paragraph, I don't think it makes sense in this case. Most screen reader users will be used to a navigation reading differently, because the code for navigations is usually done differently. When it's done with float, for example, this might look the same but reads each item separately.

querkmachine commented 2 weeks ago

I made the change suggested above, and although it resolved the issue of NVDA reading all of the links out in one go, it introduced a new problem of NVDA announcing the empty text node between each nav item. This required users of arrow navigation to press twice in order to move from one link to the next.

Service information and navigation  region    link    Apply for a juggling license 
Menu  navigation landmark    list  with 4 items  link    Navigation item 1 
blank
link    current  Navigation item 2   
blank
link    Navigation item 3 
blank
link    Navigation item 4 

I've instead changed the navigation list to use a flexbox. This removes the empty text nodes, and also seems to prevent the navigation items from being read as a single unit.

@selfthinker Quickly devtesting this, I've not found any obvious issues created by making the list a flexbox. Do you think it's worth retesting the component due to this change?

selfthinker commented 1 week ago

Navigation difficult to discover when magnified

The navigation is currently right-aligned when it comes after the service name (in desktop view). That will make it harder for magnification users to discover. The horizontal lines help a bit. But I'd suggest thinking about left-aligning them instead.

I agree with this. There doesn't seem to be a single solution without trade offs.

Can I just check because this has been ticked off in the "Done when" list, it's done because we decided not to fix it due to the trade offs, not because we fixed it?

If we left align the navigation (which will occur if the Navigation takes a new line, for example if the service name is long or there is a phase banner ) the vertical space of the header will increase, pushing page content down. If a service wishes to include additional service specific components, for example, a search box and account links this will be extended even more.

Wouldn't the page content also be pushed down the same way when the service navigation is right-aligned? Why does the alignment make a difference in this situation? It should take up the same amount of space.

I agree that if there are too many trade offs, it's not worth fixing. But I just want to make sure I understand those trade offs.

selfthinker commented 1 week ago

@selfthinker Quickly devtesting this, I've not found any obvious issues created by making the list a flexbox. Do you think it's worth retesting the component due to this change?

@querkmachine, I have retested everything yesterday and couldn't find any issue with this change. Thanks for finding a better way to fix it. 👍

querkmachine commented 1 week ago

Can I just check because this has been ticked off in the "Done when" list, it's done because we decided not to fix it due to the trade offs, not because we fixed it?

@selfthinker Looking at the issue history, this item was checked off a few weeks ago. I don't recall us coming to any specific decision on it and it's not in the decision log. It could be that it was checked off in error?

selfthinker commented 1 week ago

I don't recall us coming to any specific decision on it and it's not in the decision log.

Good point about it missing from the decision log. I've just added it.

selfthinker commented 1 week ago

There are two other things I noticed while re-testing the component.

querkmachine commented 1 week ago
  • The label for the service navigation only says "Menu". That doesn't really "describe topic or purpose" (to use WCAG speak). When screen readers read the landmark, they usually add "navigation" to it, so it ends up saying "menu navigation" which is basically saying the same thing twice. I would give the navigation a more descriptive label and not say "menu" at all. Maybe "Service"? (I thought I had written this down before but couldn't find a reference to it.)

I think this is because we're moving the navigation features from the GOV.UK Header to the Service Header (or we don't expect both to be used at the same time). Because the GOV.UK Header toggle uses the label "Menu" that has thus been moved to the Service Header unaltered. "Menu" was the wording suggested by the 2023 DAC audit, it was previously "Show or hide menu".

  • The label for the service header is "Service information and navigation". Although I think that is fine, I wonder if it is a bit too long and also unnecessarily duplicating the "navigation" bit that is inside that landmark.

Yeah, it felt a bit lengthy to me too when I was devtesting. There isn't a guarantee of navigation being present either (i.e. a Service Header that only contains the service name), so shortening it to "Service information" might be more sensible.

selfthinker commented 1 week ago

"Menu" was the wording suggested by the 2023 DAC audit, it was previously "Show or hide menu".

But that was for the toggle button and not for the navigation landmark, was it? Or were those two originally linked (via aria-labelledby maybe)?

querkmachine commented 1 week ago

It's for both.

They aren't linked with ARIA, but both are derived from the same Nunjucks parameter which defaults to "Menu". A service developer can customise each to make them different, but that isn't the current default, so probably isn't being done widely.

selfthinker commented 1 week ago

both are derived from the same Nunjucks parameter which defaults to "Menu". A service developer can customise each to make them different

I don't think that's generally a good idea. The word on the button is designed to take up the least amount of space, the label for the the landmark should be designed to describe it well out of context.

But because service developers can already change it, I wonder if this should be solved by adding something to the documentation, @calvin-lau-sig7?

CharlotteDowns commented 1 week ago

When we say 'left align' for the navigation, do we mean navigation taking a new line in the Service header navigation component or something different?

Visual example

Service name takes 100% width the page container and navigation takes a new line
selfthinker commented 1 week ago

When we say 'left align' for the navigation, do we mean navigation taking a new line in the Service header navigation component or something different?

No, I mean coming directly after the service name and not leaving any space between them.

Example of left-aligned service navigation
selfthinker commented 1 week ago

If you want to play around with it in the code, I have changed CSS on the div.govuk-service-header__container from justify-content: space-between to justify-content: start for that screenshot. I don't know if that is the solution or if that has other implications, especially when there is more content and it needs to wrap.

CharlotteDowns commented 1 week ago

I'm pretty happy with that suggestion, @Ciandelle do you have any additional thoughts on this?

Ciandelle commented 1 week ago

I'm pretty happy with that suggestion, @Ciandelle do you have any additional thoughts on this?

Same here, I'd be interested in seeing what does happen when there is more content but for now this looks like the best course of action to me :)