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

Make scrollable area keyboard (and voice) focusable #242

Merged
merged 3 commits into from
Aug 26, 2022

Conversation

colinbm
Copy link
Contributor

@colinbm colinbm commented Jul 6, 2021

⚠️ 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

Add the main content area to the tabindex.

Why

Using keyboard (or voice) control, the only way to scroll this area is to tab to a focusable item (i.e. a link) within the area. This is a workaround and relies on there being a focusable item within the area, which is not guaranteed. Making the area itself focusable means this will always be scrollable with the keyboard.

@colinbm colinbm changed the title Make scrollable area keyboard focusable Make scrollable area keyboard (and voice) focusable Jul 6, 2021
@36degrees
Copy link
Contributor

Closing and re-opening to try and trigger Travis CI.

@36degrees 36degrees closed this Jul 7, 2021
@36degrees 36degrees reopened this Jul 7, 2021
@m-green m-green requested review from lfdebrux and removed request for lfdebrux October 14, 2021 13:42
@m-green
Copy link
Contributor

m-green commented Oct 14, 2021

@36degrees Did this look ok to you when you closed and re-opened it? Just wondered if it's a quick review, or if it needs more input from you (or another frontend person if you're busy).

@m-green
Copy link
Contributor

m-green commented Mar 31, 2022

Hi @36degrees You re-opened this PR last July. Just wondered if it looks like a quick review and approve that you could do? Or whether it needs more work?

@36degrees 36degrees removed their request for review August 24, 2022 12:25
@lfdebrux lfdebrux force-pushed the accessibility-focus-scrollable branch from 1a2cd18 to 0817f47 Compare August 26, 2022 11:51
@lfdebrux
Copy link
Member

Hi @colinbm, thank you so much for raising this PR, sorry it's taken us so long to progress it.

I've taken a look at this, after talking to @36degrees, as he hasn't had the time to.

He suggested testing this with screenreaders, which I did, I found that the announcement when using Chrome and VoiceOver on macOS was not useful by default, so I have added another commit to add an accessible name to improve the announcement slightly. See 0817f47 (Add accessible name to content pane div).

We also discussed some of the history behind the 'two-pane' layout of the tech docs template. @36degrees noted that the Design System website used to have a similar layout, but dropped it because of the issues with independently scrollable regions [1], instead using a fixed table of contents and a back-to-top link. We did think though that for tech docs where the table of contents can be very long the design we're using currently does make sense, but wonder if it needs to be refreshed to properly solve this issue. That is probably not possible in the immediate though, so for now adding tabindex="0" seems like a reasonable fix.

I also talked to some people in the accessibility community about this change, we discussed whether adding an ARIA role/landmark to the <div> would be appropriate, but there are some issues there around having the main and footer regions within other landmarks according to Deque, and using the focus outline from the GOV.UK Design System. I'll leave these as ideas for another person another day.

colinbm and others added 3 commits August 26, 2022 12:56
When tabbing into the content pane with a screenreader we want
screenreaders that announce this to give a useful message to the user.

On VoiceOver and Chrome without this commit the announcement is
'and two items, group', after this commit the announcement is
'Content, group'. With NVDA and Firefox tabbing into the div is not
announced.
@lfdebrux lfdebrux force-pushed the accessibility-focus-scrollable branch from 0817f47 to 763e057 Compare August 26, 2022 11:57
@lfdebrux lfdebrux merged commit 007096e into master Aug 26, 2022
@lfdebrux lfdebrux deleted the accessibility-focus-scrollable branch August 26, 2022 12:04
@lfdebrux lfdebrux mentioned this pull request Dec 22, 2022
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