-
Notifications
You must be signed in to change notification settings - Fork 232
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
Examine and resolve accessibility findings #3811
Comments
I have a PR for the breadcrumb labelling open here: alphagov/govuk-frontend#4995 |
That wouldn't solve the issue when what's in between is other things in the global header, like the One Login menu. |
@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. |
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, Here is a screenshot of an example where the phase banner is between the service name and the service navigation. Will this change in landmarks also help resolve 'NVDA in Chrome behaving weird'? |
I just also found this accessibility concern Review the use of |
That generally looks good to me. I would probably choose to use
I doubt it. |
That seems to be quite complicated. From what I can tell that is pretty unconventional code. I don't think we use any such behaviour in the new component, so I don't think it is relevant. |
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?? |
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 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. 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.
|
I looked into this. It turns out that some of this was fixed in Chrome version 125, so was present until version 124. 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 An NVDA bug report suggests that this is intended behaviour. While I get their point that any elements in an |
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.
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? |
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?
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. |
@querkmachine, I have retested everything yesterday and couldn't find any issue with this change. Thanks for finding a better way to fix 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? |
Good point about it missing from the decision log. I've just added it. |
There are two other things I noticed while re-testing the component.
|
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".
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. |
But that was for the toggle button and not for the navigation landmark, was it? Or were those two originally linked (via |
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. |
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? |
If you want to play around with it in the code, I have changed CSS on the |
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 :) |
I think all these issues have been addressed, we will create a new card to track if not |
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
The text was updated successfully, but these errors were encountered: