make code-block focusable #1104

12rambau · 2023-01-07T14:27:37Z

That was way more complicated than expected.

3 possible choices are available:

use pure JS to add the tabindex attributes at run time
parse the page and search for "pre" elements
override the visit_literal_block method

By looking more carefully at it I realized that not all the "pre" elements are actually generated by docutils (the one included in highlighted code for example are never parsed) so I decided to overwrite the visit_literal_block to get both the div including the code elements and the pre from non highlighted elements.

before:
https://pydata-sphinx-theme.readthedocs.io/en/stable/examples/kitchen-sink/blocks.html#code-block

after:
https://pydata-sphinx-theme--1104.org.readthedocs.build/en/1104/examples/kitchen-sink/blocks.html#literal-blocks

choldgraf · 2023-01-07T16:35:12Z

Hmm - my initial thinking after taking a look is that this is complex to understand and will be tricky to maintain over time, and really should be fixed at the level of sphinx or docutils and not this theme.

Regarding your points above, it seems like using JavaScript would be the least complex. I personally find over-writing the HTML writing methods to have created a lot of unexpected confusion over the years

12rambau · 2023-01-07T16:41:35Z

I droped the js solution because I though that relying on a JS script for a11y was not robust enough, but maybe I'm wrong @trallard do you have any insight for us ?

I personally find over-writing the HTML writing methods to have created a lot of unexpected confusion over the years

The modifications I made here are just the extra attribute, the rest is coming from the docutils/sphinx code. I would love to use super but the method are not returning anything so you can't "add" on top of them. Also the overwriting issue may be less of a burden when with the #1105 ?

12rambau · 2023-01-08T10:30:41Z

@choldgraf so new implementation idea. Instead of overriding completely the visit methods (with the high risk of not staying up to date with Sphinx or docutils) I decided to take advantage of the flexibility of Python.

Now the main override of the HTML5 builder is its capacity to add extra arguments based on the pst-atts and pst-class members. so the overrides become super simple (as we add parameters in members that are not read by other html5 builders) and we mostly rely on the "inherited" visit methods.

It simplifies a lot the overwrite of visit_literal_block and I think could be useful in the more general #1024

choldgraf

This looks way simpler for sure :-) I left a few questions in there to try and make it clearer what we're doing but in general this looks good!

Maybe @trallard can confirm that this pattern of tabindex will actually solve the original problem?

choldgraf · 2023-01-08T14:58:40Z

src/pydata_sphinx_theme/bootstrap_html_translator.py

+        # update attributes using pst_atts
+        if hasattr(self, "pst_atts"):
+            for att, val in self.pst_atts.items():
+                kwargs[att] = kwargs.pop(att, val)


Is this the same as:

if att not in kwargs: kwargs[att] = val

?

If so, I feel like that is a bit easier to understand. But maybe I am mis-reading these lines

not exactly equivalent. here what I do is :
try to find the attribute: yes: pop it else use our value

In yours you always replace it even if it's already in kwargs

not exactly equivalent. here what I do is : try to find the attribute: yes: pop it else use our value

In yours you always replace it even if it's already in kwargs

No, check the if condition. (Aside: another way to do this that avoids the conditional altogether is to use dict.setdefault)

My bad you are right. I'll have a look to setdefault I want to avoid "if" as much as possible

I do find setdefault to be easier to quickly understand. Personally I find the "if" statement to be the least "idiosyncratic" and most straightforward to understand, but setdefault works too.

choldgraf · 2023-01-08T14:59:04Z

src/pydata_sphinx_theme/bootstrap_html_translator.py

+        """
+        Perform small modification on specific tags:
+            - ensure an aria-level is set for any heading role
+            - include the pst_atts in the final attributes


Can you add a quick note of why atts and class are added / what they do?

choldgraf · 2023-01-08T15:00:20Z

src/pydata_sphinx_theme/bootstrap_html_translator.py

+        """overwrite the literal-block element to make them focusable"""
+
+        # add tabindex to the node
+        pst_atts = getattr(node, "pst_atts", {})


so pst_atts are chunks of data we are adding to the object on our own in order to re-use them later? And I guess we can't just modify atts and classes directly?

classes and atts are keyword arguments of the starttag method. They are not reachable without a strong override of the visit method (as I did in my first proposal). In this case the advantage is to carry the information within the node object allowing me to make very small changes to visit methods:

def visit_xxx(self, node): node.pst_class = "toto tutu tata" super().visit_xxx(node)

Ah wait so pst is not short for "pydata sphinx theme"? These are in-built sphinx things?

you are very right "pst" stands for pydata-sphinx-theme which guarantee that no other lib should interfere with these values.

It's the cool thing about Python I can add as many member as I want even after init.

12rambau · 2023-01-09T06:51:18Z

I converted it to draft as there is an unwanted behaviour: all the following tags have become focusable as well.

trallard · 2023-01-09T17:52:43Z

The pre elements do seem to be focusable now, but I can indeed see other items that should not be focusable, for example, this p in the Kitchen sink -> blocks section

Among many other HTML elements (span)... I am not super familiar with the Sphinx/PyData parsers/HTML converters, so I will have to do some diving into how this works before being helpful

trallard · 2023-01-14T10:36:40Z

I thought I replied again.

@12rambau I looked at your PR - and it seems the previous approach where you were overwriting the highlight tags had the desired behaviour.

Though I know there are concerns regarding overwriting the methods, so perhaps after #1105 this PR could be revisited to make the code blocks focusable only as opposed to all the literals as is currently in this PR

gabalafou · 2023-10-04T11:51:31Z

I droped the JS solution because I though that relying on a JS script for a11y was not robust enough, but maybe I'm wrong @trallard do you have any insight for us ?

I don't see that anyone responded to this point, so I'll add my two cents.

While I agree that the better solution is to add tabindex="0" in the generated HTML source code for all overflow: auto elements (such as pre), for many things accessibility-related, a partially working solution is better than no solution at all.

I suspect that we could add the following line of JavaScript to every PyData-themed page and it would fix the issue in nearly all places for nearly all users:

document.querySelectorAll('pre').forEach(el => el.tabIndex = 0);

gabalafou · 2023-10-04T11:53:08Z

That said, I also echo @choldgraf's opinion that this should ultimately be fixed upstream

gabalafou · 2024-02-21T20:57:14Z

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

gabalafou · 2024-04-17T20:54:25Z

Since #1636 was merged, I'm going to close this PR, but please feel free to re-open if I am mistaken.

12rambau added 2 commits January 7, 2023 15:18

make code-block focusable

11dd48a

use f string

1fdaf2b

12rambau marked this pull request as ready for review January 7, 2023 14:33

build: override starttag instead of visit methods

5dbb154

choldgraf reviewed Jan 8, 2023

View reviewed changes

12rambau marked this pull request as draft January 9, 2023 06:49

gabalafou mentioned this pull request Jan 8, 2024

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

Merged

trallard added the tag: accessibility Issues related to accessibility issues or efforts label Feb 23, 2024

trallard mentioned this pull request Apr 12, 2024

Team sync April 12, 2024 Quansight-Labs/czi-scientific-python-mgmt#68

Open

gabalafou closed this Apr 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make code-block focusable #1104

make code-block focusable #1104

12rambau commented Jan 7, 2023 •

edited

Loading

choldgraf commented Jan 7, 2023

12rambau commented Jan 7, 2023 •

edited

Loading

12rambau commented Jan 8, 2023

choldgraf left a comment

choldgraf Jan 8, 2023

12rambau Jan 8, 2023

drammock Jan 8, 2023

12rambau Jan 8, 2023

choldgraf Jan 9, 2023

choldgraf Jan 8, 2023

choldgraf Jan 8, 2023

12rambau Jan 9, 2023 •

edited

Loading

choldgraf Jan 9, 2023

12rambau Jan 9, 2023 •

edited

Loading

12rambau commented Jan 9, 2023

trallard commented Jan 9, 2023 •

edited

Loading

trallard commented Jan 14, 2023

gabalafou commented Oct 4, 2023

gabalafou commented Oct 4, 2023

gabalafou commented Feb 21, 2024

gabalafou commented Apr 17, 2024

make code-block focusable #1104

make code-block focusable #1104

Conversation

12rambau commented Jan 7, 2023 • edited Loading

choldgraf commented Jan 7, 2023

12rambau commented Jan 7, 2023 • edited Loading

12rambau commented Jan 8, 2023

choldgraf 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

12rambau Jan 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

12rambau Jan 9, 2023 • edited Loading

Choose a reason for hiding this comment

12rambau commented Jan 9, 2023

trallard commented Jan 9, 2023 • edited Loading

trallard commented Jan 14, 2023

gabalafou commented Oct 4, 2023

gabalafou commented Oct 4, 2023

gabalafou commented Feb 21, 2024

gabalafou commented Apr 17, 2024

12rambau commented Jan 7, 2023 •

edited

Loading

12rambau commented Jan 7, 2023 •

edited

Loading

12rambau Jan 9, 2023 •

edited

Loading

12rambau Jan 9, 2023 •

edited

Loading

trallard commented Jan 9, 2023 •

edited

Loading