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

Fix keyboard access for scrollable regions created by notebook outputs #1787

Merged
merged 20 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
221d4ec
Fix keyboard access for scrollable regions created by notebook outputs
gabalafou Apr 25, 2024
008cc9a
Remove data-tabindex, add test
gabalafou Apr 26, 2024
f759a69
Merge branch 'main' into fix-scrollable-notebook-outputs
Carreau May 21, 2024
6ac0dbb
[pre-commit.ci] Automatic linting and formatting fixes
pre-commit-ci[bot] May 21, 2024
f749d8b
mark as failing test instead of comment
Carreau May 22, 2024
8037ae3
Update tests/test_a11y.py
Carreau May 23, 2024
fe1c837
update comments
gabalafou May 23, 2024
b750602
Update tests/test_a11y.py
gabalafou May 23, 2024
dac259f
Merge branch 'main' into fix-scrollable-notebook-outputs
Carreau May 23, 2024
a8762a8
Add CSS to allow scrolling of ipywidget (#1760)
gabalafou May 23, 2024
2788f60
Fix keyboard access for scrollable regions created by notebook outputs
gabalafou Apr 25, 2024
ab616e6
Remove data-tabindex, add test
gabalafou Apr 26, 2024
e3b7dfc
mark tab stop tests as a11y
gabalafou May 24, 2024
a0dc755
[pre-commit.ci] Automatic linting and formatting fixes
pre-commit-ci[bot] May 24, 2024
658e099
fix bad rebase
gabalafou May 24, 2024
b44c530
call addTabStops after more important stuff
gabalafou May 24, 2024
a4b8b79
Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js
gabalafou May 24, 2024
e9ca671
Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js
gabalafou May 24, 2024
a583f6d
Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js
gabalafou May 24, 2024
0740a28
Merge branch 'main' into fix-scrollable-notebook-outputs
gabalafou May 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 44 additions & 11 deletions src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js
Original file line number Diff line number Diff line change
Expand Up @@ -693,19 +693,45 @@ function setupMobileSidebarKeyboardHandlers() {
}

/**
* When the page loads or the window resizes check all elements with
* [data-tabindex="0"], and if they have scrollable overflow, set tabIndex = 0.
* When the page loads, or the window resizes, or descendant nodes are added or
* removed from the main element, check all code blocks and Jupyter notebook
* outputs, and for each one that has scrollable overflow, set tabIndex = 0.
*/
function setupLiteralBlockTabStops() {
function addTabStopsToScrollableElements() {
const updateTabStops = () => {
document.querySelectorAll('[data-tabindex="0"]').forEach((el) => {
el.tabIndex =
el.scrollWidth > el.clientWidth || el.scrollHeight > el.clientHeight
? 0
: -1;
});
document
.querySelectorAll(
"pre, " + // code blocks
".nboutput > .output_area, " + // NBSphinx notebook output
".cell_output > .output, " + // Myst-NB
".jp-RenderedHTMLCommon", // ipywidgets
)
.forEach((el) => {
el.tabIndex =
el.scrollWidth > el.clientWidth || el.scrollHeight > el.clientHeight
? 0
: -1;
});
};
window.addEventListener("resize", debounce(updateTabStops, 300));
const debouncedUpdateTabStops = debounce(updateTabStops, 300);

// On window resize
window.addEventListener("resize", debouncedUpdateTabStops);

// The following MutationObserver is for ipywidgets, which take some time to
// finish loading and rendering on the page (so even after the "load" event is
// fired, they still have not finished rendering). Would be nice to replace
// the MutationObserver if there is a way to hook into the ipywidgets code to
// know when it is done.
const mainObserver = new MutationObserver(debouncedUpdateTabStops);

// On descendant nodes added/removed from main element
mainObserver.observe(document.getElementById("main-content"), {
subtree: true,
childList: true,
});

// On page load
gabalafou marked this conversation as resolved.
Show resolved Hide resolved
updateTabStops();
}
function debounce(callback, wait) {
Expand All @@ -717,15 +743,22 @@ function debounce(callback, wait) {
}, wait);
};
}
// Determining whether an element has scrollable content depends on stylesheets,
// so we're checking for the "load" event rather than "DOMContentLoaded"
if (document.readyState === "complete") {
addTabStopsToScrollableElements();
} else {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to add the document.readyState check in order for the Playwright tests to pass. It may or may not be necessary in other browsers, but it makes sense to keep it as a bit of defensive programming.

window.addEventListener("load", addTabStopsToScrollableElements);
}

/*******************************************************************************
* Call functions after document loading.
*/

documentReady(addModeListener);
documentReady(scrollToActive);
documentReady(addTOCInteractivity);
documentReady(setupSearchButtons);
documentReady(initRTDObserver);
documentReady(setupMobileSidebarKeyboardHandlers);
documentReady(fixMoreLinksInMobileSidebar);
documentReady(setupLiteralBlockTabStops);
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
html div.rendered_html,
// NBsphinx ipywidgets output selector
html .jp-RenderedHTMLCommon {
// Add some margin around the element box for the focus ring. Otherwise the
// focus ring gets clipped because the containing elements have `overflow:
// hidden` applied to them (via the `.lm-Widget` selector)
margin: $focus-ring-width;

table {
table-layout: auto;
}
Expand Down
21 changes: 0 additions & 21 deletions src/pydata_sphinx_theme/translator.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import types

import sphinx
from docutils import nodes
from packaging.version import Version
from sphinx.application import Sphinx
from sphinx.ext.autosummary import autosummary_table
Expand All @@ -27,32 +26,12 @@ def starttag(self, *args, **kwargs):
"""Perform small modifications to tags.

- ensure aria-level is set for any tag with heading role
- ensure <pre> tags have tabindex="0".
"""
if kwargs.get("ROLE") == "heading" and "ARIA-LEVEL" not in kwargs:
kwargs["ARIA-LEVEL"] = "2"

if "pre" in args:
kwargs["data-tabindex"] = "0"

return super().starttag(*args, **kwargs)

def visit_literal_block(self, node):
"""Modify literal blocks.

- add tabindex="0" to <pre> tags within the HTML tree of the literal
block
"""
try:
super().visit_literal_block(node)
except nodes.SkipNode:
# If the super method raises nodes.SkipNode, then we know it
# executed successfully and appended to self.body a string of HTML
# representing the code block, which we then modify.
html_string = self.body[-1]
self.body[-1] = html_string.replace("<pre", '<pre data-tabindex="0"')
raise nodes.SkipNode

Comment on lines -30 to -55
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I realized as I was working on this that this deleted code was essentially trying to put a data-tabindex attribute on every <pre> tag. So, in the JavaScript file, instead of iterating through elements with data-tabindex, which is either the same or a sub set of all the pre elements on the page, we can simplify and iterate through all the pre elements instead.

In other words, it only makes sense to keep this code if we add the tabindex attribute at build time. But since we're not doing that anymore (as of #1777), we might as well delete this code, and just check all pre elements on page load.

def visit_table(self, node):
"""Custom visit table method.

Expand Down
38 changes: 37 additions & 1 deletion tests/test_a11y.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,9 @@ def test_code_block_tab_stop(page: Page, url_base: str) -> None:
"""Code blocks that have scrollable content should be tab stops."""
page.set_viewport_size({"width": 1440, "height": 720})
page.goto(urljoin(url_base, "/examples/kitchen-sink/blocks.html"))

code_block = page.locator(
'css=#code-block pre[data-tabindex="0"]', has_text="from typing import Iterator"
"css=#code-block pre", has_text="from typing import Iterator"
)

# Viewport is wide, so code block content fits, no overflow, no tab stop
Expand All @@ -265,3 +266,38 @@ def test_code_block_tab_stop(page: Page, url_base: str) -> None:
# Narrow viewport, content overflows and code block should be a tab stop
assert code_block.evaluate("el => el.scrollWidth > el.clientWidth") is True
assert code_block.evaluate("el => el.tabIndex") == 0


def test_notebook_output_tab_stop(page: Page, url_base: str) -> None:
"""Notebook outputs that have scrollable content should be tab stops."""
page.goto(urljoin(url_base, "/examples/pydata.html"))

# A "plain" notebook output
nb_output = page.locator("css=#Pandas > .nboutput > .output_area")

# At the default viewport size (1280 x 720) the Pandas data table has
# overflow
gabalafou marked this conversation as resolved.
Show resolved Hide resolved
assert nb_output.evaluate("el => el.scrollWidth > el.clientWidth") is True
assert nb_output.evaluate("el => el.tabIndex") == 0
gabalafou marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.fail(reason="fail until #1760 is merged")
Carreau marked this conversation as resolved.
Show resolved Hide resolved
def test_notebook_output_tab_stop_1760(page: Page, url_base: str) -> None:
"""# TODO: this was part of test_notebook_output_tab_stop.

I is now separated into it's own failing test until #1760 is merged.
gabalafou marked this conversation as resolved.
Show resolved Hide resolved
"""
page.goto(urljoin(url_base, "/examples/pydata.html"))

# An ipywidget notebook output
ipywidget = page.locator("css=.jp-RenderedHTMLCommon").first

# As soon as the ipywidget is attached to the page it should trigger the
# mutation observer, which has a 300 ms debounce
ipywidget.wait_for(state="attached")
page.wait_for_timeout(301)

# At the default viewport size (1280 x 720) the data table inside the
# ipywidget has overflow
gabalafou marked this conversation as resolved.
Show resolved Hide resolved
assert ipywidget.evaluate("el => el.scrollWidth > el.clientWidth") is True
assert ipywidget.evaluate("el => el.tabIndex") == 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I made this a separate test with @pytest.mark.fail in order to not forget to uncomment it when working on #1760

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not familiar with fail mark, is it an alias for xfail? I would have expected maybe pytest.mark.xfail(strict=True) to make sure we remember to remove it after #1760 is in

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, let's do xfail(strict=True), it's clearer.

gabalafou marked this conversation as resolved.
Show resolved Hide resolved
Loading