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

(docs) make demo page more keyboard accessible #3371

Merged
merged 6 commits into from Nov 2, 2021

Conversation

Hirse
Copy link
Contributor

@Hirse Hirse commented Oct 25, 2021

Changes

Adds basic keyboard accessibility to the demo page by adding a-tags to react to keydown events.

hljs-demo-keyboard

Checklist

  • Added markup tests, or they don't apply here because no highlight changes
  • Updated the changelog at CHANGES.md No published changes

demo/demo.js Outdated Show resolved Hide resolved
@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented Oct 27, 2021

I think of both these lists as navigation (not buttons). Just because the URL doesn't change doesn't mean they aren't navigation. If they are filtering or navigating you to a different view, I think that's nav... so we should probably just make them <A> tags and then take advantage of the browsers built-in accessibility support rather than hack on our own.

The CSS should be pretty tightly scoped so it shouldn't pose a huge problem.

@Hirse
Copy link
Contributor Author

@Hirse Hirse commented Oct 29, 2021

I think of both these lists as navigation (not buttons). Just because the URL doesn't change doesn't mean they aren't navigation.

Fair enough. I removed the listener for the space key.

so we should probably just make them tags and then take advantage of the browsers built-in accessibility support rather than hack on our own.

Making them <a> tags only comes with its own accessibility support if we use the default action, i.e. navigation / changing the URL.
There is unfortunately no "trigger" event that we could use.

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented Oct 30, 2021

The onclick event handler (and click event) is triggered when the mouse is pressed and released when over a page element or when the Enter key is pressed while a keyboard-navigable element has focus.

Shouldn't the existing onclick handler just work? A target of "#" could be added if necessary...

@Hirse
Copy link
Contributor Author

@Hirse Hirse commented Oct 31, 2021

Added <a>-tags and removed custom keydown handler.

demo/demo.js Outdated Show resolved Hide resolved
@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented Nov 1, 2021

I really like that all the tabindex=0 disappears, feels like a step in the correct direction.

demo/demo.js Outdated Show resolved Hide resolved
@joshgoebel joshgoebel changed the title Make demo page keyboard accessible (docs) make demo page more keyboard accessible Nov 2, 2021
@joshgoebel joshgoebel merged commit 7eb21e5 into highlightjs:main Nov 2, 2021
14 checks passed
@Hirse Hirse deleted the keyboard-demo branch Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants