highlightjs / vue-plugin

Highlight.js Vue Plugin
BSD 3-Clause "New" or "Revised" License
201 stars 27 forks source link

One line fix for accessibility issue #33

Closed TheJaredWilcurt closed 2 years ago

TheJaredWilcurt commented 2 years ago

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:

joshgoebel commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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