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

One line fix for accessibility issue #33

Merged
merged 2 commits into from
Aug 22, 2022

Conversation

TheJaredWilcurt
Copy link
Contributor

@TheJaredWilcurt TheJaredWilcurt commented Aug 10, 2022

This change allows you to tab to the code box when a scrollbar appears. Making the scrollbar keyboard accessible.

This fixes the following issue:

scroll-a11y

Essentially if there is a code block with a line of code so long that it breaks the box, then a overflow: auto scroll bar will appear, but keyboard users need to be able to TAB to the element and scroll it. If no scrollbar appears, the element is be skipped during tabbing, like usual.

More details:

@TheJaredWilcurt TheJaredWilcurt changed the title Fix A11Y issue One line fix for accessibility issue Aug 11, 2022
@joshgoebel
Copy link
Member

Are there any downsides here? Does this now make all code snippets "steal focus" if someone starts tabbing and they all have scrollable regions? Are there cases where someone might want tabindex=1000000 instead or to custom configure the index?

@TheJaredWilcurt
Copy link
Contributor Author

If a codebox does not have a scrollbar, tabbing will skip it.

If a codebox does have a scrollbar, you will tab to the box and be able to use the arrow keys to scroll in the box. It doesn't steal focus, it is just inserted into the list of tab-able items in the DOM.

tabindex="0" just means that it will follow the natural flow of the DOM. Compared to setting it to a specific value which would likely take it out of the flow, giving the feeling of "stealing" focus.

It may be possible that someone would want to modify the value of this, or any attribute. If you want to support that usecase I would suggest pre-attributes and code-attributes props. So users have full control over setting the attributes on these elements.

<template>
  <highlightjs
    language="xml"
    code="<h1>Hello world</h1>"
    :pre-attributes="{
      title: 'Example',
      style: 'border: 10px solid blue'
    }"
    :code-attributes="{
      title: 'Example',
      focusable: 'focusable',
      tabindex: '10000'
    }"
  />
</template>

But even if you do allow for setting/overriding attributes, I would still say this PR should be merged as is, so that the <code> block has a default of tabindex="0". So it is accessible by default.

@joshgoebel
Copy link
Member

tabindex="0" just means that it will follow the natural flow of the DOM.

Ah, right. In that case this seems a reasonable default.

It doesn't steal focus, it is just inserted into the list of tab-able items in the DOM.

Yeah, I was forgetting the behavior of 0...

@TheJaredWilcurt
Copy link
Contributor Author

TheJaredWilcurt commented Aug 22, 2022

@joshgoebel So can this be merged and added as a release? I'm currently reaching in via a $ref to override this in my project, and would prefer a more elegant solution.

@joshgoebel
Copy link
Member

Please add a changelog entry and we'll get this merged.

(this repo is looking for a maintainer, it's not official part of HLJS anymore)

@TheJaredWilcurt
Copy link
Contributor Author

Updated the changelog, let me know if anything else is needed.

@joshgoebel joshgoebel merged commit c4e3a91 into highlightjs:main Aug 22, 2022
@TheJaredWilcurt TheJaredWilcurt deleted the patch-1 branch August 22, 2022 15:52
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.

None yet

2 participants