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

Accessibility: scrollable regions should be reached via keyboard #1514

Closed
4 tasks done
jaimeiniesta opened this issue Mar 17, 2020 · 10 comments
Closed
4 tasks done

Accessibility: scrollable regions should be reached via keyboard #1514

jaimeiniesta opened this issue Mar 17, 2020 · 10 comments
Labels
change request Issue requests a new feature or improvement

Comments

@jaimeiniesta
Copy link
Contributor

I checked that...

  • ... the documentation does not mention anything about my problem
  • ... the problem doesn't occur with the default MkDocs template
  • ... the problem is not in any of my customizations (CSS, JS, template)
  • ... there are no open or closed issues that are related to my problem

Description

Keyboard navigation in this template is great, but there is a case to be improved: scrollable code blocks. In code blocks that have long lines, a horizontal scroll bar appears, which only can be accessed using the mouse. It should be accessible via the keyboard, so you can reach that area using tab and then scroll it using the arrow keys.

Expected behavior

Scrollable areas should be reachable via keyboard.

https://dequeuniversity.com/rules/axe/3.5/scrollable-region-focusable?application=rocketvalidator

https://webaim.org/techniques/keyboard/tabindex

Actual behavior

Horizontal scrollbars in pre code blocks are not accessible via keyboard.

Steps to reproduce the bug

Navigate via the keyboard on a page that has code blocks with long lines so the horizontal scrollbars appear. Try to scroll it without using the mouse.

Package versions

  • Python: 3.7.7
  • MkDocs: 1.1
  • Material: 5.0.0rc2

Project configuration

site_name: Rocket Validator Documentation
nav:
    - Home: index.md
    - API: api.md
theme:
  name: material
  language: en
  logo: img/rocket-icon-300x300.png
extra:
  social:
    - icon: fontawesome/brands/github-alt
      link: https://github.com/rocketvalidator
    - icon: fontawesome/brands/twitter
      link: https://twitter.com/rocketvalidator
    - icon: fontawesome/brands/linkedin
      link: https://pr.linkedin.com/company/rocket-validator
google_analytics:
  - UA-81204018-1
  - auto
markdown_extensions:
- admonition
- pymdownx.superfences
- codehilite:
    guess_lang: false
extra_css:
    - css/extra.css

System information

  • OS: macOS 10.15.3
  • Browser: Chrome 80

Proposed solution

Adding tabindex=0 to the pre code elements makes them focusable via keyboard without altering the tab order. This is the proposed solution in the linked pages above.

I've tried this in https://docs.rocketvalidator.com/api/ and it works fine (you may need to reload the page if you have visited it recently to refresh the cache).

To add the attribute I'm using JS:

rocketvalidator/docs-rocketvalidator@d2e1d90

This works fine, but I need to deactivate the instant loading feature as it looks like the JS won't be executed on page change. Is there a way to fix this?

Maybe tabindex=0 could be included in the template itself so we don't need this JS fix?

Are there other options like fixing this via CSS or maybe an extension configuration?

@squidfunk
Copy link
Owner

squidfunk commented Mar 17, 2020

This works fine, but I need to deactivate the instant loading feature as it looks like the JS won't be executed on page change. Is there a way to fix this?

You must re-bind your event handlers with:

window.addEventListener("DOMContentLoaded", ...)
window.addEventListener("DOMContentSwitch", ...)

or via the reactive architecture:

app.document$.subscribe(...)

The latter will trigger on both, load and switch event.

Maybe tabindex=0 could be included in the template itself so we don't need this JS fix?

Not possible, pre and code is all generated via Python Markdown and/or Pymdown Extensions, this theme can only work with what it gets as HTML. JavaScript is the only solution.

Are there other options like fixing this via CSS or maybe an extension configuration?

I don't really understand the question. How should this be fixed with CSS? tabindex is a semantic attribute and specific to HTML.

@squidfunk
Copy link
Owner

... however, you very probably only want to add tabindex in case a code block overflows, or there will be a lot of useless focus events. For this reason you should check whether the container overflows and bind your events. This check needs to be done every time the document is loaded and/or the viewport is resized.

@jaimeiniesta
Copy link
Contributor Author

Thanks, you're right that this can't be done via CSS, and can only be fixed via JavaScript.

It also makes sense to only apply it when there is overflow, and also reviewed when the viewport is resized. I'm afraid I can't help with a PR here as I'm more a backend guy and don't know much about JS really. But I'll try.

Are you willing to incorporate the JS fix on the theme? I think it's an important fix to make to improve accessibility but maybe an explanation on how to set this up on the README can be enough. Same as the tip on disabling hyphenation via CSS.

@jaimeiniesta
Copy link
Contributor Author

I've changed the JS to make the code blocks focusable only if they overflow, and also recalculated that on the resize events:

https://github.com/rocketvalidator/docs-rocketvalidator/blob/master/docs/js/extra.js

I'm not sure how to rebind the events to be able to enable the instant feature, though. If you can help, that will be much appreciated, thanks.

@squidfunk
Copy link
Owner

Just call your function also on the DOMContentLoaded and DOMContentSwitch events and you should be good.

@jaimeiniesta
Copy link
Contributor Author

Thanks, but something else must be missing. This is my JS:

// Makes code blocks accessible via keyboard,
// so they can be scrolled without a mouse.
//
// More info:
// https://dequeuniversity.com/rules/axe/3.5/scrollable-region-focusable
// https://webaim.org/techniques/keyboard/tabindex
// https://github.com/squidfunk/mkdocs-material/issues/1514

function makeScrollableBlocksFocusable() {
  console.log("making scrollable blocks focusable...");

  var codeblocks = document.querySelectorAll('pre code');
  for (var i=0; i < codeblocks.length; i++) {
    if (isOverflown(codeblocks[i])) {
      codeblocks[i].setAttribute("tabindex", "0");
    } else {
      codeblocks[i].removeAttribute("tabindex");
    }
  }
}

function isOverflown(element) {
  return element.scrollHeight > element.clientHeight || element.scrollWidth > element.clientWidth;
}

window.addEventListener("DOMContentLoaded", makeScrollableBlocksFocusable);
window.addEventListener("DOMContentSwitch", makeScrollableBlocksFocusable);
window.addEventListener("resize", makeScrollableBlocksFocusable);

This works fine without the instant feature enabled but if I enable it, the function is not called when I click links.

@squidfunk
Copy link
Owner

Okay, so as instant loading is still experimental, I suggest we leave this issue open for later integration. I think we can add this behavior to the master, but currently there are other priorities - getting v5 out the door first 😊

@squidfunk squidfunk added the change request Issue requests a new feature or improvement label Mar 17, 2020
@squidfunk
Copy link
Owner

Added as part of c6e2e3a.

@squidfunk squidfunk added resolved Issue is resolved, yet unreleased if open and removed resolved Issue is resolved, yet unreleased if open labels May 16, 2020
@squidfunk
Copy link
Owner

Released as part of 5.1.7

@squidfunk
Copy link
Owner

ad27da1 adds support for tabbed code blocks. Previously, only the initial code blocks would be checked for overflow and bound to resize events. The latest commit also adds this feature for code blocks, which are hidden behind tabs:

Ohne.Titel.mp4

Change is in master, can be tested on the live documentation site and will be part of the 7.0.0 release that is currently in beta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change request Issue requests a new feature or improvement
Projects
None yet
Development

No branches or pull requests

2 participants