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

Adds a skip to content link to all themes that is tab navigable #4332

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

ajrbyers
Copy link
Member

Screenshot 2024-07-22 at 12 20 43 Screenshot 2024-07-22 at 12 20 32 Screenshot 2024-07-22 at 12 20 16

@StephDriver
Copy link
Contributor

Andy, I'm not too keen on this. Pretty much everywhere I've seen "skip to content" implemented, it doesn't appear over the top of existing content. I've seen some places do it by having strategically positioned whitespace, others by dynamically scrolling the whole page down or sideways to make space for new accessible menu items like this. Here's an example of the functionality on the Access to Work site from .gov.uk - i.e. aimed at disabled users. (tabs are highlighting in yellow)

skip_to_content.mov

Copy link
Contributor

@StephDriver StephDriver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment on above - I don't think having this over the top of other content is an accessible implementation. It won't just be people who can't see the page who use this functionality. It shouldn't be obscuring other content.

@StephDriver StephDriver assigned ajrbyers and unassigned StephDriver Jul 23, 2024
@ajrbyers
Copy link
Member Author

Screenshot 2024-07-24 at 10 05 23 Screenshot 2024-07-24 at 10 05 17 Screenshot 2024-07-24 at 10 05 57 Screenshot 2024-07-24 at 10 05 52 Screenshot 2024-07-24 at 10 05 32 Screenshot 2024-07-24 at 10 05 42

@ajrbyers ajrbyers requested a review from StephDriver July 24, 2024 09:10
@ajrbyers ajrbyers assigned StephDriver and unassigned ajrbyers Jul 24, 2024
Copy link
Contributor

@StephDriver StephDriver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the behaviour here is quite right yet.

When I tested this, while the "skip to content" link did move the page (✅ ) if I then tab again, I'm back at the top at the next item after skip to content. So basically having had the skip, I can't continue from that point ( ❌ ). After skipping the next tab item should be the next one after the start of hte main content, not taking me back to the top of the page.

I've tested this on Chrome with a press site and journal site - finding the same behaviour on both.

@StephDriver StephDriver assigned ajrbyers and unassigned StephDriver Jul 26, 2024
@ajrbyers
Copy link
Member Author

I don't think the behaviour here is quite right yet.

When I tested this, while the "skip to content" link did move the page (✅ ) if I then tab again, I'm back at the top at the next item after skip to content. So basically having had the skip, I can't continue from that point ( ❌ ). After skipping the next tab item should be the next one after the start of hte main content, not taking me back to the top of the page.

I've tested this on Chrome with a press site and journal site - finding the same behaviour on both.

Which theme did you test this with?

@StephDriver
Copy link
Contributor

Which theme did you test this with?

OLH.

I've just tried Clean and Material - and those are both fine. So I think just an OLH issue?

@ajrbyers
Copy link
Member Author

Which theme did you test this with?

OLH.

I've just tried Clean and Material - and those are both fine. So I think just an OLH issue?

Yeah I just tested it there too. I think its intercepting the click so the anchor link isn't set and the focus doesn't move.

@StephDriver
Copy link
Contributor

StephDriver commented Jul 26, 2024

I'll leave it with you then - I think that's the only problem remaining - from what I can see the rest of the requirements are there:

✅ link is the first focusable control on the Web page.
✅ description of the link communicates that it links to the main content.
✅ (the link is either always visible - No) or visible when it has keyboard focus (Yes).
✅ activating the link moves the focus to the main content.
( ❌ OLH, ✅ Clean & Material) after activating the link, the keyboard focus has moved to the main content.

from WCAG G1

@ajrbyers ajrbyers requested a review from StephDriver August 1, 2024 13:25
@ajrbyers ajrbyers assigned StephDriver and unassigned ajrbyers Aug 1, 2024
@ajrbyers
Copy link
Member Author

ajrbyers commented Aug 1, 2024

@StephDriver I've updated OLH's JS to stop it intercepting the anchor hash being added to the URL bar. All looks okay now. To test this locally you will need to run the build_assets command.

@StephDriver
Copy link
Contributor

StephDriver commented Aug 5, 2024

I have been testing it with random pages, and this time that ended up being a homepage, where it skipped to a search bar rather than the start of the main content. But it works fine on an article page, and on the list of articles too. I wonder whether at this point it would be best to implement this solution, since it seems fine for 99% of the site, and then raise a new issue for reviewing its functionality on the landing page?

I am suggesting that because I think the problem is actually with the landing page structure not this issue. The landing page's main content starts with a search bar, before having a heading. Rather than change this new functionality to look for the heading which isn't where it should be on that page, I'd rather improve the structure of the landing page.

image

Copy link
Contributor

@StephDriver StephDriver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left a comment on the behaviour on landing pages, which I think should be addressed separately. Otherwise I'm happy to approve this.

@StephDriver StephDriver requested a review from joemull August 5, 2024 13:17
@StephDriver StephDriver assigned joemull and unassigned StephDriver Aug 5, 2024
@joemull joemull merged commit 15a708d into master Aug 7, 2024
@joemull joemull deleted the 3932-skip_to_content branch August 7, 2024 09:59
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.

is it possible to skip straight to the main content?
3 participants