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

the cell is landmark for a form #63

Merged
merged 36 commits into from
Jun 10, 2023
Merged

the cell is landmark for a form #63

merged 36 commits into from
Jun 10, 2023

Conversation

tonyfast
Copy link
Contributor

@tonyfast tonyfast commented May 31, 2023

this pull request began as an exploration of potential configurations for an accessible cell form.

  • the cell is significant enough feature to be a landmark.
  • the cell has a form
  • the cell has input and output regions
  • the cell complies with web accessibility guidelines

the implementation uses semantic elements. Another implementation could use the implicit roles and proper ARIA to create an explicit accessible experience. features will need to be added and they should include accessible elements that are prescribed by references like ARIA Authoring Practices Guide

example outputs below:

this work includes testing on NVDA, VoiceOver, Orca, and ChromeVox.

the cell landmark

the cell is a landmark with role=region when there is an ARIA tag. an implementation would choose whether some, all, or none of the cells are labeled.

the cell form

the cell form is an empty element. all the content exists outside the form to allow other forms; form elements cannot be nested. to tether a cell's form associated elements to. the document.forms object provides nominal and ordinal access to each form.

the cell input region

the cell input fieldset has an implicit role=group. the disabled attribute applies to all form descendants. a static, resting cell then is <fieldset disabled> which allows for custom styling based on state.

the cell output region

the cell output holds computed values from the input. we represent these with the output tag which implicitly is an aria live element. instead we apply role=presentation to reduce the screen verbosity.

the id patterns

a notebook is data and a document. we use a consistent json pointer naming convention that would provide consistent access to either the json document or html document.

styling

styling is a non-goal of this pull request it uses minimum styling to convey the message of the notebook. we do demonstrate how semantic css selectors naturally facilitate formatting the document. using flex box will help with reactive experiences at high zoom magnifications.

references

output

fieldset

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 2, 2023

it seems the <section aria-labelledby=X><fieldset><legend id=X> configuration will always repeat the legend contents on VoiceOver. this would only be favorable in an editing mode.

the fieldset was added because we needed to group multiple projections of the same input. right now, the markdown cells don't use the output region because the markdown source and output are in the input region. if the rendering is moved to the output region then the experience would be good. in that sense, the markdown to html operation is the kernel, it could be remote or local.

basically, we need an input group for any real world applications. the input group could be a fieldset, tabs, accordion, or other accessible patterns.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 2, 2023

this working is tedious, but the questions are changing and this is good. the execution count is still a stinking nuisance .

the execution count

the cell is form and effectively the execution count, cell index, or cell (insert custom name) is the label for it. the label needs to be exposed, but not too much.

does the execution count landmark the cell, cell input, or cell output?

the label is actually a generated output. <output name="execution_count"><label>29</label></output>

the rendered markdown and the rendered code

i had markdown output being rendering in the input group. @bollwyvl said that the rendered markdown is an output, and i agree with that. now i don't know if the rendered highlight pygments code is part of the input or the output.

as i 🦆 this, i think pygments representation is in the input group because it is a dual of the input text. the markdown view is the source interpretted, but pygments does no interpretation so there is no information loss.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 3, 2023

the redundancy of In[x] and Out[x] only provides noise on the screen reader. there is no reason those numbers will ever differ and one of the them is superfluous while the other is a label. the only way to advance the execution count is to interact with a kernel that increments it. in theory, the Out[x] is sufficient enough to label an executed cell.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 3, 2023

i just added the smol variant that is meant to be accessible only and not using jupyter conventions, no pygments, just bare tags. this variant lets us understand how create the notebook variant. in this variant, all cells are labelled ordinally and the execution count is applied to only the output. its clear thought that if there is only one cell then there is a consistent naming for any genus of cell. the cell index in a notebook is a default, but we could custom name cells like observable offers.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 4, 2023

for this idea we need a canonical toggle for read mode to execute mode. if the starting point is accessible then the application can be developed accessibly, and accessibility guides design decisions.

in smol, i ripped out the input fieldset because it was causing some verbosity issues on the screen readers. it turns out the fieldset[name=input] was not the problem. when we were using it we were giving markdown and code cells different naming heuristics. duh, there was a heuristic in the way! we gotta get rid of everything stinking heuristic.

the fieldset makes semantic sense, but since there are currently less than 3 inputs it will have role=presentation.

fieldset[name=input] comes back so that <fieldset name=input disabled> can disable the form and fieldset[name=input]:disabled is the toggle for our primary states. we are doing this in the sections template already, but the labeling is bunk.

we will now label each section by a cell name with the an ordinal default that could be modified later. each input and output should be marked.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 4, 2023

When the cell label is hidden so is the prefix to the screen reader. The cell, in, out prefixes need to be in the dom.

{% macro input_group(cell, nb) %}
{%- set count = nb["cells"].index(cell)-%}{%- set ID = "/cells/" + str(count) -%}
{{ cell_form_element(cell, nb)}}
<fieldset for="{{ID}}" name="input" disabled>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is expected that in the resting state, markdown cells should as standalone content appear. NVDA announces the region as "grouping unavailable" and it is confusing.

the fix down grades the fieldset to role=presentation.


{%- macro cell_outputs(cell, nb, parts) -%}
{%- set count = nb["cells"].index(cell)-%}{%- set ID = "/cells/" + str(count) -%}
<fieldset name="outputs" id="{{ID}}/outputs" {%- if not cell.outputs %}hidden{% endif %}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

VoiceOver is verbose when the fieldset is exited "End Out ##" "End Out ##". it is repeated because the section and fieldset are labelled.

NVDA "out of group" "out of region"

it seems like fieldset should have role=presentation so that we remove some verbosity on two major readers.

@smythp
Copy link
Member

smythp commented Jun 6, 2023

Hi Tony, sorry I got pulled away yesterday afternoon after going through the first notebook. Let me quickly give you notes from the first notebook and I'll finish the rest after meetings today.


Honestly, this looks great, at least with Orca, which is what I've tested it with so far.

Some things that are working:

  • Really like the landmark movement. Bringing up the list of landmarks and all the cells being there is a joy.
  • In Orca, the way the cell is read partially when you hit a landmark is quite nice. I'm pretty sure NVDA won't do this but looking forward to trying it out.
  • On a lot of sites Orca gets badly tripped up or starts using a lot of memory. (At least on my machine, using Cinnamon, MATE seems to work better.) Not seeming to get that here.

Some notes:

  • On the small example notebook (the first link you shared), there's a cell toward the end that's labeled cell 7. It seems to just be a separator. It's where cell 20 should be. Haven't dug into the source to see why it's occuring.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 6, 2023

Honestly, this looks great, at least with Orca, which is what I've tested it with so far.

OMG. so stinking happy about this. Orca just fell into place when i started working around NVDA, VoiceOver, and ChromeVox.

  • Really like the landmark movement. Bringing up the list of landmarks and all the cells being there is a joy.

🤗

  • In Orca, the way the cell is read partially when you hit a landmark is quite nice. I'm pretty sure NVDA won't do this but looking forward to trying it out.

you're going to be surprised i think. i took a rip around NVDA a little bit ago and there are some nice affordances:

  • you can navigate to cell landmarks with D
  • you can navigate to code cells input form elements with F
  • the cell number, input and output are announced when entering the cells. so Page + Up/Down sound great! (at least to my untrained 👂)
  • On a lot of sites Orca gets badly tripped up or starts using a lot of memory. (At least on my machine, using Cinnamon, MATE seems to work better.) Not seeming to get that here.

that is good to know. the sad part of this story is at the end of the day we did some really basic accessibility work. it is just some landmarks and top level conventions most web devs know. i think this exercise though did reduce the number of elements that need to be rendered and improve the quality of the accessibility tree.

  • On the small example notebook (the first link you shared), there's a cell toward the end that's labeled cell 7. It seems to just be a separator. It's where cell 20 should be. Haven't dug into the source to see why it's occuring.

this is a 🐛 i introduce. good 👀 . with the nbconvert machinery i couldn't get cell indexes so i am using nb.cells.index(cell) to get an index. there must be a duplicate cell screwing up that ordering.

thanks so much for taking the time to evaluate this notebook. i really appreciate any time you can put forward to this. i think i am ready for NVDA to be tested. VoiceOver isn't the preferred experience, but where it is it might be the best we are getting.

UX question: do you think the type of cell should be announced? like markdown cell, python cell, or code cell? is it important to distinguish? im thinking in the future there could be double digit cell types in a document. curious about your thoughts.

@smythp
Copy link
Member

smythp commented Jun 6, 2023

Yeah, I was thinking about that. I think before I would have said yes, we should have it announce the cell type, now I'm slightly less sure, just because I was getting more context than I was expecting while navigating by landmark, and it was evident pretty fast what kind of cell it was. Still leaning toward having it say the cell type, though, if I had to come down on it.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 6, 2023

it probably makes sense to make this configurable. in our use case, with just python and markdown, we don't need a differentiation. if there were more languages i think we'd want to make it configurable. i opened #64 to talk about configurations if you have ideas.

it is recommended that the aria label is short around 3 words. now it is "Cell 1". saying "Python Cell 1", "SQL Cell 2", "markdown Cell 3" might make more sense in that scenario.

@smythp
Copy link
Member

smythp commented Jun 6, 2023

I think "code cell" is probably fine if providing the language will be an issue. It's not like the language is labeled every time for sighted folks (unless I'm misremembering), there's just visual context on whether the cell is a code cell (presence of the "in [0]" text to the left of the cell.) As I recall, the UX also allows "code cell," "markdown cell," and "raw," unless things have changed, so it would also be in line with that.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 6, 2023

there is a visual UX with code highlighting. SQL and Python could show visually different. this is a hypothetical notebook we are taling about. so maybe just the code cells would need to be distinguished.

@smythp
Copy link
Member

smythp commented Jun 7, 2023

OK, just went through the second notebook (imaging sky). Here are my notes using Orca.

Second notebook only labels the cell number (for example "8") rather than labeling with the word "cell" ("cell 8") when moving by landmark. I think having it say "cell 8" is more useful context, especially since it's not unlikely for numbers to be in the cells. When moving element by element (i.e., by pressing down repeatedly, I get labelling related to region, i.e. "region 4", "leaving region."

For some of the cells (see cell 10) the "in [4]" text that usually appears on the left in a notebook is read while moving from line to line, i.e. we hear:

In 4 shrink_factor = 2  # To make the images smaller for faster execution
In 4 nrow = ncol = default_size // shrink_factor  # Image size
In 4 row, col = np.mgrid[0:nrow, 0:ncol]

See also i.e. cell 12.

In cell 16, I only get the message about alternatively opening the tool (no title, etc.), though likely out of scope for the work we're doing.

Overall, the labels seemed to work better in the first notebook (not sure if we're doing a/b here, or was the second notebook a different version?)

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 7, 2023

Second notebook only labels the cell number (for example "8") rather than labeling with the word "cell" ("cell 8") when moving by landmark. I think having it say "cell 8" is more useful context, especially since it's not unlikely for numbers to be in the cells. When moving element by element (i.e., by pressing down repeatedly, I get labelling related to region, i.e. "region 4", "leaving region."

you are correct that "cell 8" is better. i made a change recently that i should revert. i added a visual cell number to the page because it seems weird to hide that and loosely it would fail a WCAG. im going to revert things and see if it removes the region bit. i think this added visible element really screwed things up.

For some of the cells (see cell 10) the "in [4]" text that usually appears on the left in a notebook is read while moving from line to line, i.e. we hear:

In 4 shrink_factor = 2  # To make the images smaller for faster execution
In 4 nrow = ncol = default_size // shrink_factor  # Image size
In 4 row, col = np.mgrid[0:nrow, 0:ncol]

See also i.e. cell 12.

In cell 16, I only get the message about alternatively opening the tool (no title, etc.), though likely out of scope for the work we're doing.

i think that this is the content. i'd like to open another pull request that improves the accessibility of the content of testing notebooks.

Overall, the labels seemed to work better in the first notebook (not sure if we're doing a/b here, or was the second notebook a different version?)

here's what changed. i carelessly added a visual cell number indicator. i was too excited about your positive initial review that i was like "let's give it some more gas". i'm going to revert these visual changes back for meantime because including the cell count is non-canonical.

i do think the cell number is important. when you were reporthing this issue. i could easily just the orca's landmarkdown nagivation to quickly scroll to cell (10/12/16) like you referenced. i think cell numbers are really important after this exercise. they should be used to facilitate conversation like this did in this pull request.

@tonyfast
Copy link
Contributor Author

tonyfast commented Jun 7, 2023

i removed the visible cells labels because they are non-canonical. however, without them it appears much harder to talk about our test cases. cell numbers belong somewhere and we'll want to figure that out. with the screen reader on, the lack of cell numbers for markdown becomes really apparent.

i was thinking about the empty cells, or placeholder cells. they probably should be mentioned as empty; maybe role=separator has meaning here. Removing cells from the screen reader would violate an expectation of countability. in theory, we shouldn't ever hide anything without announcing the lack. it leads me to think that a best practice for cells would be always having an output. even if there is stateful work done. an output could say "the foo object has changed state".

@smythp
Copy link
Member

smythp commented Jun 7, 2023

Yeah, interesting about the empty or seperator cells. I was wondering if I was missing content when going through nonvisually. I suppose they are cells like any other.

+1 to the adding commits, I was just wondering if I was missing something, but that makes sense.

@tonyfast
Copy link
Contributor Author

i think im going to merge this in before i make anymore of a mess. things that need to be done to finish.

  • include cell and notebook metadata
  • include notebook toolbar
  • include visual cell label

@tonyfast
Copy link
Contributor Author

on orca and voiceover the cell numbers are only announced when there is more than one element in a cell. this means a code cells has visible outputs or markdown is non-empty. in fact, we decided to move the markdown outputs outside the output region so that we would announce markdown cells on VoiceOver as reference for position.

one of the things we learn from this experience is that code cells should have outputs. a cell without outputs is not announced in most screen readers. in the case that cells modify state or set variables. it would make sense for that to be announced visually.

i think it is going to be a problem for assistive tech users to relate an output with the variable it is associated with. currently that relationship is visually implicit and should be made semantically explicit.

@tonyfast
Copy link
Contributor Author

closing. please feel free to keep commenting here or open up another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants