Ensure code blocks (<pre>) are keyboard focusable #1636

gabalafou · 2024-01-08T04:40:31Z

Fixes #1100.
Also closes one of the subtasks of #1428.
Inspired by #1104.

Whitespace-preserving blocks in our theme, such as code blocks, currently do not have an easy way for keyboard users to focus on them and use the arrow keys to scroll the block to the right in order to read long lines that overflow the width of the block. This is also an accessibility issue; if you have to use a mouse, a pointer, or your finger—anything other than just your keyboard—to scroll a code block, then that's a violation of WCAG 2.1.1.

Check the end user effects of this PR on the Read the Docs preview build of the Kitchen Sink Blocks page

gabalafou

done self-reviewing

src/pydata_sphinx_theme/translator.py

gabalafou · 2024-01-08T04:44:02Z

src/pydata_sphinx_theme/translator.py

+            html_string = self.body[-1]
+            self.body[-1] = html_string.replace("<pre", '<pre tabindex="0"')


This is kinda dirty but... it seems to work 🤷

I'll leave it up to the reviewers to decide if it's too dirty

I am having a bit of trouble following here - is the goal of these to find and traverse any hidden/nested pretags then add the tabindex?

Exactly. When the parent class's visit_literal_block method is called, it appends an HTML string to self.body. So the very last item in the self.body list is just an HTML string containing a bunch of markup that represents the literal block. A lot of the tags in that markup are there to provide syntax highlighting, line numbers, captions, etc. The relevant tags inside that markup are the pre tags, although I would guess there's only ever one pre tag. However, I add tabindex="0" to any pre tag I find because the nature of a pre tag is that it preserves the whitespace, which means it might be scrollable (depending on how long the lines inside of it are) and should therefore be keyboard-focusable.

this doesn't seem that "dirty" to me.

Ditto not dirty just working with what we have

I read something since working on this PR that makes me wonder if this is the right thing to do. The thing I read was about tables but the same could be said about pre-blocks:

Of course, we don't want to make the table container focusable unless its contents overflow. Otherwise we're adding a tab stop to the focus order which doesn't do anything. In my opinion, that would be a fail under 2.4.3 Focus Order. Giving keyboard users elements to focus which don't actually do anything is confusing and obstructive.
Source: Inclusive Components - Data Tables - Only Focusable Where Scrollable

Which makes me wonder if we should put a data attribute on the pre-tag and then check it with JavaScript after it loads to determine if it should really be a tab stop or not.

Follow-up:

Only make scrollable code blocks into tab stops #1777

src/pydata_sphinx_theme/translator.py

trallard · 2024-01-08T13:02:26Z

src/pydata_sphinx_theme/translator.py

+            html_string = self.body[-1]
+            self.body[-1] = html_string.replace("<pre", '<pre tabindex="0"')


I am having a bit of trouble following here - is the goal of these to find and traverse any hidden/nested pretags then add the tabindex?

gabalafou

Oh, I just revisited this PR and realized that I had a bunch of pending comments

src/pydata_sphinx_theme/translator.py

gabalafou · 2024-01-08T16:12:44Z

src/pydata_sphinx_theme/translator.py

+            html_string = self.body[-1]
+            self.body[-1] = html_string.replace("<pre", '<pre tabindex="0"')


Exactly. When the parent class's visit_literal_block method is called, it appends an HTML string to self.body. So the very last item in the self.body list is just an HTML string containing a bunch of markup that represents the literal block. A lot of the tags in that markup are there to provide syntax highlighting, line numbers, captions, etc. The relevant tags inside that markup are the pre tags, although I would guess there's only ever one pre tag. However, I add tabindex="0" to any pre tag I find because the nature of a pre tag is that it preserves the whitespace, which means it might be scrollable (depending on how long the lines inside of it are) and should therefore be keyboard-focusable.

src/pydata_sphinx_theme/translator.py

drammock

This seems like a pretty efficient solution. I think another possible route would be to define a "post-transform" which alters the docutils.document before it gets to the translator. But IDK if that would actually be cleaner/easier, and this seems to give the desired result, so 🤷🏻

src/pydata_sphinx_theme/translator.py

drammock · 2024-04-04T14:17:08Z

src/pydata_sphinx_theme/translator.py

+            # 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.


so... docutils raises an exception when the method executes successfully? What happens when it fails? 😅

I think it would raise a different exception but I'm not sure.

I think it has something to do with the way that docutils traverses the doc tree, see docutils/nodes.py::Node.walkabout()

drammock · 2024-04-04T14:18:05Z

src/pydata_sphinx_theme/translator.py

+            html_string = self.body[-1]
+            self.body[-1] = html_string.replace("<pre", '<pre tabindex="0"')


this doesn't seem that "dirty" to me.

Co-authored-by: gabalafou <[email protected]>

trallard · 2024-04-05T15:09:00Z

I think this PR is in a good shape to merge and has been approved so will go ahead and merge 🚀

there is one question from Dan somewhere but it is non-blocking at all

drammock · 2024-04-05T15:56:55Z

only fail is codecov; in it goes! thanks @gabalafou

#1787) 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 #1636 and #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

* Ensure code blocks (<pre>) are keyboard focusable * Also cover <pre> tags not covered by starttag * Update src/pydata_sphinx_theme/translator.py Co-authored-by: gabalafou <[email protected]> --------- Co-authored-by: Tania Allard <[email protected]>

pydata#1787) One of many fixes for the failing accessibility tests (see pydata#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 pydata#1636 and pydata#1777. These were due to Jupyter notebook outputs that have scrollable content. This PR extends the functionality of PRs pydata#1636 and pydata#1777 to such outputs. - Adds a test for tabindex = 0 on notebook outputs after page load This also addresses one of the issues in pydata#1740: missing horizontal scrollbar by: - Adding CSS rule to allow scrolling - Add ipywidgets example to the examples/pydata page

gabalafou added 2 commits January 7, 2024 22:15

Ensure code blocks (<pre>) are keyboard focusable

a0eafea

Also cover <pre> tags not covered by starttag

de64db1

gabalafou commented Jan 8, 2024

View reviewed changes

trallard reviewed Jan 8, 2024

View reviewed changes

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

gabalafou commented Feb 19, 2024

View reviewed changes

trallard mentioned this pull request Feb 21, 2024

Keyboard Navigation Quansight-Labs/czi-scientific-python-mgmt#56

Open

gabalafou mentioned this pull request Feb 21, 2024

make code-block focusable #1104

Closed

Carreau approved these changes Apr 4, 2024

View reviewed changes

drammock approved these changes Apr 4, 2024

View reviewed changes

trallard mentioned this pull request Apr 5, 2024

Enable navigation_with_keys by default in themes with responsive design sphinx-doc/sphinx#12227

Closed

Update src/pydata_sphinx_theme/translator.py

6b6faae

Co-authored-by: gabalafou <[email protected]>

drammock merged commit 525e52d into pydata:main Apr 5, 2024
18 of 19 checks passed

gabalafou deleted the pre-tabindex branch April 19, 2024 14:24

This was referenced Apr 19, 2024

Only make scrollable code blocks into tab stops #1777

Merged

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure code blocks (<pre>) are keyboard focusable #1636

Ensure code blocks (<pre>) are keyboard focusable #1636

gabalafou commented Jan 8, 2024 •

edited

Loading

gabalafou left a comment

gabalafou Jan 8, 2024

trallard Jan 8, 2024

gabalafou Jan 8, 2024

drammock Apr 4, 2024

trallard Apr 5, 2024

gabalafou Apr 5, 2024 •

edited

Loading

gabalafou Apr 24, 2024

trallard Jan 8, 2024

gabalafou left a comment

gabalafou Jan 8, 2024

drammock left a comment

drammock Apr 4, 2024

gabalafou Apr 5, 2024

drammock Apr 4, 2024

trallard commented Apr 5, 2024

drammock commented Apr 5, 2024

		html_string = self.body[-1]
		self.body[-1] = html_string.replace("<pre", '<pre tabindex="0"')

Ensure code blocks (<pre>) are keyboard focusable #1636

Ensure code blocks (<pre>) are keyboard focusable #1636

Conversation

gabalafou commented Jan 8, 2024 • edited Loading

gabalafou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabalafou Apr 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabalafou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drammock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trallard commented Apr 5, 2024

drammock commented Apr 5, 2024

gabalafou commented Jan 8, 2024 •

edited

Loading

gabalafou Apr 5, 2024 •

edited

Loading