search

alphagov / tech-docs-gem

Gem to distribute the tech docs project
https://tdt-documentation.london.cloudapps.digital/
MIT License
15 stars 38 forks source link

Add hints to search results to indicate it should be closed #239

Closed colinbm closed 2 years ago

colinbm commented 3 years ago

⚠️ Don't forget to update the gem version in the CHANGELOG before merging! When you're ready to release bump version file and generate a tag. ⚠️

What

Why

DAC report

The main page region appears to be hidden from assistive technology when the dialog is triggered. For sighted users, the dialog obscures the main page region, and this acts as a prompt to close it. However, this dialog is non-modal so for screen reader users, this content appears as if it was simply inset on the page after the search button. This means that users navigating with screen reading software may not make the association between needing to close the dialog in order to access the main page content again.

This region is marked up as a dialog with a clear label, however, as focus is not trapped it may not be easy for screen reader users to determine the boundaries of the dialog. When a search is explicitly submitted, focus is taken to the dialog, but lands on the ‘Offer GovWifi in your public sector organisation’ link, so that users either have to tab backwards to close it or otherwise tab through the page. Screen reader users may not be aware that the ‘close’ button is present, as they are navigated straight past it, and as much of the surrounding page appears to be available, would not necessarily infer the need to ‘close’ it.

Additionally, the results dialog appears as a user types in the search field without being explicitly requested. This means that typing within the edit field has the unintended direct consequence of hiding the main page content. Further, unless the form is explicitly submitted, screen reader users are not informed that the page has updated. This means that users who type in the field mistakenly may struggle to infer why the main content has now become hidden.

DAC proposed solution

It may assist users to apply the WAI-ARIA design pattern for modal dialogs, including limiting keyboard focus and screen reading software in reading mode to the contents of the dialog when it is open. For more information, please refer to the WAI-ARIA Authoring Practices 1.1 Modal Dialog Example. Otherwise, ensure that focus is taken to the ‘Close’ button upon activation.

Please note, that there are currently varying degrees of support for the aria-modal attribute so it will not always trap screen reader focus in all browsers. It is also very important that when the modal is closed, the element with aria-modal="true" set is hidden with ‘display:none;’, otherwise it can prevent screen reader users from interacting with the page.

36degrees commented 3 years ago

Thanks for looking at this. This is a really difficult interaction to make accessible, and unfortunately I don't think this is going to be quite as easy as DAC have made it sound!

Add aria-hidden attribute to main content (true when menu open).

I don't believe this is doing anything, because the use of visibility: hidden on technical-documentation when the has-search-results-open class is applied to the <html> element means that the main content is already hidden from everyone, including screen readers.

Best practise would be to not use aria-hidden in such cases.

Set aria-modal on search results, as hint that this is modal content and should be closed to return to main content.

The use of aria-modal on the search results pane does much more than that – it also means that everything outside of the search results is hidden from screen readers. However, we're not doing anything to ensure that the focus is retained within the search results pane. It's therefore possible to tab to elements outside of that area, and because those elements are hidden nothing gets announced. From the perspective of someone who cannot see the screen, they now have no idea where the focus is.

In this video, note how when the search results are displayed, the screen reader makes no announcement when the focus moves to the links in the navigation:

https://user-images.githubusercontent.com/121939/124277018-7c02a180-db3c-11eb-9ea1-50a87c77c728.mov

In terms of the WAI-ARIA modal dialog example, the missing piece of the puzzle is the section on keyboard support, which dictates that when the focus is moved from the last element (or first element, when tabbing backwards) it should 'wrap' within the modal.

This is what DAC are referring to when they talk about 'focus trapping', and we're not currently doing it, which is what is breaking the pattern. I'd suggest spending some time looking at the example implementation to understand how this is meant to work.

Having said all of that, I'm not entirely sure that it does make sense to treat the search results page as a modal dialog. When the search results are visible the navigation remains visible and interactive if you're using a mouse or touch, so I'm not sure it makes sense to capture focus and prevent users being able to reach them using the keyboard.

And, even if you trap keyboard focus, a visually-sighted screen reader user may try to interact with the visible parts of the page (again, using the mouse or touch), but will not get any screen reader feedback.

To be honest, I'd suggest booking a slot at the next accessibility clinic to get input from an accessibility specialist. There's a lot of things to consider here…

colinbm commented 3 years ago

I've gone back to DAC to enquire on this... it's possible that focussing on "Close" when submitted may be sufficient, but that doesn't really resolve the issue that the content disappears when typing.

They may come back and suggest that we remove the results as you type, and only display them on submit.

colinbm commented 3 years ago

From DAC:

I would say that the biggest problem is that you are hiding the page from assistive technology when a use types in the search field. Without the visual prompt of the form covering the content, there is no reason why a screen reader users would know that the dialog needs to be closed for the page to be read. There is no association between the close button they locate under the search field and the rest of the page content. I would strongly suggest allowing a user to explicitly submit the search rather than the search being submitted ‘live’ as a user types. I would then make the results dialog modal and would not allow users to access the left navigation when it was open. The only other alternative I can think of is to maintain the current implementation but not to hide the page when the results dialog pops up; however, this may be frustrating for keyboard only users, as the dialog covers half the page, so users would be focusing components which are visually hidden.

Ideally, I would really recommend an entirely separate search results page.

So as I thought, the minimal fix would be to remove the "live" updating of results. I'm still not sure about the modal and preventing nav access, unless we add a visual indication that it's disabled.

I think a separate page for search results would be a bigger job than planned for here so I won't look at that too much unless we decide on that directiom.

colinbm commented 3 years ago

From DAC:

Removing the live updating will certainly help, but I also think the main page content should not be hidden when the search dialog is open. A good way to visualise what a screen reader user would encounter is to disable the CSS and disregard everything we know will be hidden from assistive technology with display:none;.

Sighted users can see the search results visually covering the main page content, but this is not so for a screen reader user – they encounter the content linearly in the order it appears in the DOM. The ‘Close search results’ button may be easily missed and there is no reason for a user to make the association between search results which appear in the navigation region and the main content disappearing much further down the page. Unless you can immediately see that making a search has triggered the main page content to be hidden, it would be very difficult to infer that connection.

Based on that, the latest change removes the live updating, leaves the content visible, and reduces the content's opacity when the search results are visible.

36degrees commented 3 years ago

The tests are also failing:

ReferenceError: Can't find variable: Map

Unfortunately, I think this is coming from the Inert polyfill we're now including.

36degrees commented 3 years ago

The approach outlined in this PR makes use of the inert attribute which is relatively new and still has patchy support in browsers.

We're vendoring in a polyfill for it (WICG/inert, included as part of 4eff8ac461af154d8624524f6dc3f5a9241247f7) but that polyfill itself relies on Map, Set, Element.prototype.matches and Node.prototype.contains (as per https://github.com/WICG/inert#legacy-browser-support).

Of those:

Notably Jasmine, which we use to run our JS tests, uses PhantomJS under the hood which does not support Map or Set either. This is why the tests are currently failing with ReferenceError: Can't find variable: Map.

So, there are two related issues:

Options at this point:

36degrees commented 3 years ago

Based on the size of the response from polyfill.io, I think polyfilling Map and Set would add about 34kb (roughly 8kb gzipped) to the application JS.

lfdebrux commented 2 years ago

Hi @colinbm, thank you for the investigation and the PR.

The working group have been looking at options to fix the accessibility issue from the DAC report, and we think we’re going to try adding a new search page as DAC recommended, so we will probably not end up merging your changes.

Your effort is still very much appreciated though, I think it’s good that you did investigate making the dialog more accessible.