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

gabalafou · 2024-04-25T16:47:10Z

One of many fixes for the failing accessibility tests (see #1428).

The accessibility tests were still reporting some violations of:

Scrollable region must have keyboard access (https://dequeuniversity.com/rules/axe/4.8/scrollable-region-focusable) even after merging Ensure code blocks (<pre>) are keyboard focusable #1636 and Only make scrollable code blocks into tab stops #1777.

These were due to Jupyter notebook outputs that have scrollable content. This PR extends the functionality of PRs #1636 and #1777 to such outputs.

Adds a test for tabindex = 0 on notebook outputs after page load

This also addresses one of the issues in #1740: missing horizontal scrollbar by:

Adding CSS rule to allow scrolling
Add ipywidgets example to the examples/pydata page

gabalafou · 2024-04-25T16:52:01Z

TODO:

add test for tabindex = 0 on notebook outputs after page load

gabalafou · 2024-04-26T12:11:01Z

src/pydata_sphinx_theme/translator.py

-        - 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
-


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.

gabalafou

done self-reviewing, going to mark this as ready for review

gabalafou · 2024-04-26T12:13:57Z

src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

+if (document.readyState === "complete") {
+  addTabStopsToScrollableElements();
+} else {


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.

gabalafou · 2024-04-26T12:18:15Z

The failing check has to do with tokenless CodeCov rate limiting, so it's not related to this PR per se

github-actions · 2024-05-21T08:47:48Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
src/pydata_sphinx_theme
translator.py
Project Total

_{This report was generated by python-coverage-comment-action}

Carreau · 2024-05-22T12:53:10Z

tests/test_a11y.py

+@pytest.mark.fail(reason="fail until #1760 is merged")
+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.
+    """
+    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
+    assert ipywidget.evaluate("el => el.scrollWidth > el.clientWidth") is True
+    assert ipywidget.evaluate("el => el.tabIndex") == 0


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

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

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

tests/test_a11y.py

Carreau · 2024-05-23T09:06:58Z

resolved some conflicts.

This addresses one of the issues in pydata#1740: missing horizontal scrollbar. - Add CSS rule to allow scrolling - Add ipywidgets example to the examples/pydata page

src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

gabalafou · 2024-05-24T17:32:17Z

The "defusedxml" error means we can't actually see in the CI that the tests I added actually work, since I tagged them as a11y and for some reason that test path is triggering that error (but I think there's some setting on the step that makes the failure not appear at first glance in the workflow run report)

trallard · 2024-05-25T00:58:28Z

Hmmm this error should not be present anymore. But since we have the CI to pass even if we have a11y errors this means this is getting obscured.

Can you merge main on your branch to see if the recently merged CI changes fixes it? If not I can add a fix.

gabalafou · 2024-05-27T06:52:17Z

That worked! Happy to see that CI PR merged (#1759) :)

Fix keyboard access for scrollable regions created by notebook outputs

221d4ec

gabalafou force-pushed the fix-scrollable-notebook-outputs branch from d3f6357 to 221d4ec Compare April 25, 2024 16:48

Remove data-tabindex, add test

008cc9a

gabalafou commented Apr 26, 2024

View reviewed changes

gabalafou marked this pull request as ready for review April 26, 2024 12:15

trallard added the tag: accessibility Issues related to accessibility issues or efforts label May 8, 2024

Carreau and others added 2 commits May 21, 2024 01:33

Merge branch 'main' into fix-scrollable-notebook-outputs

f759a69

[pre-commit.ci] Automatic linting and formatting fixes

6ac0dbb

mark as failing test instead of comment

f749d8b

Carreau reviewed May 22, 2024

View reviewed changes

Carreau reviewed May 23, 2024

View reviewed changes

tests/test_a11y.py Outdated Show resolved Hide resolved

Update tests/test_a11y.py

8037ae3

gabalafou commented May 23, 2024

View reviewed changes

tests/test_a11y.py Outdated Show resolved Hide resolved

gabalafou commented May 23, 2024

View reviewed changes

tests/test_a11y.py Outdated Show resolved Hide resolved

tests/test_a11y.py Outdated Show resolved Hide resolved

tests/test_a11y.py Show resolved Hide resolved

update comments

fe1c837

gabalafou commented May 23, 2024

View reviewed changes

tests/test_a11y.py Outdated Show resolved Hide resolved

gabalafou and others added 2 commits May 23, 2024 03:35

Update tests/test_a11y.py

b750602

Merge branch 'main' into fix-scrollable-notebook-outputs

dac259f

gabalafou and others added 7 commits May 24, 2024 16:25

Add CSS to allow scrolling of ipywidget (pydata#1760)

a8762a8

This addresses one of the issues in pydata#1740: missing horizontal scrollbar. - Add CSS rule to allow scrolling - Add ipywidgets example to the examples/pydata page

Fix keyboard access for scrollable regions created by notebook outputs

2788f60

Remove data-tabindex, add test

ab616e6

mark tab stop tests as a11y

e3b7dfc

[pre-commit.ci] Automatic linting and formatting fixes

a0dc755

fix bad rebase

658e099

call addTabStops after more important stuff

b44c530

gabalafou commented May 24, 2024

View reviewed changes

src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js Outdated Show resolved Hide resolved

Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

a4b8b79

gabalafou commented May 24, 2024

View reviewed changes

src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js Outdated Show resolved Hide resolved

Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

e9ca671

gabalafou commented May 24, 2024

View reviewed changes

src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js Outdated Show resolved Hide resolved

Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

a583f6d

Merge branch 'main' into fix-scrollable-notebook-outputs

0740a28

Carreau approved these changes May 27, 2024

View reviewed changes

Carreau merged commit c839490 into pydata:main May 27, 2024
30 checks passed

Carreau mentioned this pull request May 27, 2024

Inproper styling of dataframes for myst-nb + pydata theme when contained in ipywidget (2) #1740

Open

gabalafou deleted the fix-scrollable-notebook-outputs branch May 27, 2024 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

gabalafou commented Apr 25, 2024 •

edited by Carreau

gabalafou commented Apr 25, 2024 •

edited

gabalafou Apr 26, 2024

gabalafou left a comment

gabalafou Apr 26, 2024

gabalafou commented Apr 26, 2024 •

edited

github-actions bot commented May 21, 2024 •

edited

Carreau May 22, 2024

drammock May 22, 2024

Carreau May 23, 2024

Carreau commented May 23, 2024

gabalafou commented May 24, 2024

trallard commented May 25, 2024

gabalafou commented May 27, 2024

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

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

Conversation

gabalafou commented Apr 25, 2024 • edited by Carreau

gabalafou commented Apr 25, 2024 • edited

gabalafou Apr 26, 2024

Choose a reason for hiding this comment

gabalafou left a comment

Choose a reason for hiding this comment

gabalafou Apr 26, 2024

Choose a reason for hiding this comment

gabalafou commented Apr 26, 2024 • edited

github-actions bot commented May 21, 2024 • edited

Coverage report

Carreau May 22, 2024

Choose a reason for hiding this comment

drammock May 22, 2024

Choose a reason for hiding this comment

Carreau May 23, 2024

Choose a reason for hiding this comment

Carreau commented May 23, 2024

gabalafou commented May 24, 2024

trallard commented May 25, 2024

gabalafou commented May 27, 2024

gabalafou commented Apr 25, 2024 •

edited by Carreau

gabalafou commented Apr 25, 2024 •

edited

gabalafou commented Apr 26, 2024 •

edited

github-actions bot commented May 21, 2024 •

edited