Table block: Fix semantic structure for screen readers on back-end #54324
Conversation
alexstine
added
[Focus] Accessibility (a11y)
alexstine
requested review from
ZebulanStanphill,
ellatrix,
jeryj,
joedolson and
t-hamano
alexstine
left a comment
alexstine
left a comment
There was a problem hiding this comment.
A couple quick implementation notes.
| placeholder={ placeholder[ name ] } | ||
| /> | ||
| <CellTag key={ columnIndex }> | ||
| <RichText |
There was a problem hiding this comment.
This will likely break visuals but it works due to the cell itself remaining a table cell. The editable content is now done within the cell. This will also likely cause a slightly different back-end vs. the front-end but it isn't up for debate. It simply does not work when you convert a table cell to an editable field. The entire table loses semantic meaning with screen readers.
There was a problem hiding this comment.
Why do we need role=textbox if it breaks things? Isn't contenteditable enough?
There was a problem hiding this comment.
@ellatrix With my changes, it comes with 2 bonuses.
- The table semantics are respected.
- The editable areas inside the table cells have better text input semantics.
All in all, a big win.
| columnIndex | ||
| ) => ( | ||
| <RichText | ||
| tagName={ CellTag } |
There was a problem hiding this comment.
No need for this anymore. It renders in <div> by default.
|
Size Change: +6 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
t-hamano
left a comment
t-hamano
left a comment
There was a problem hiding this comment.
I think this change makes sense. NVDA now reads correctly as follows.
Before
clickable table with 0 rows and 0 columns Body cell text edit multi line out of edit Body cell text edit multi line out of edit Body cell text edit multi line out of edit Body cell text edit multi line out of edit out of table Table caption text edit multi line Add caption
After
clickable table with 2 rows and 2 columns row 1 column 1 Body cell text edit multi line out of edit column 2 Body cell text edit multi line out of edit row 2 column 1 Body cell text edit multi line out of edit column 2 Body cell text edit multi line out of edit out of table Table caption text edit multi line Add caption
My guess is that the screen reader is not reading it correctly because there are no cells in the table with the appropriate role attribute.
I also noticed while testing this PR that the first cell is not in focus after inserting the Table block. I think that is one of the reasons why the E2E test is failing. I still don't know why it is not getting focused, but if getting focused makes sense, I think it needs to be resolved.
To solve this problem, it would be necessary to change the selector of the element that gets focus when the table is created. As another approach, how about just changing the |
|
@t-hamano Thanks for the review.
Not having I fixed the focus code, that was something I implemented. 😄 Really hoping maybe this logic could move into a hook at some point to focus the first element after the block re-renders but don't have the time to scope that out. |
t-hamano
left a comment
t-hamano
left a comment
There was a problem hiding this comment.
LGTM 👍
- ✅ After creating the table, the first cell gets focus
- ✅ No visual changes
- ✅ colspan/rowspan is maintained when pasting data containing merged cells from Google Sheets
- ✅ Can still move cells using the arrow keys
|
What does role=textbox do? Do we need it? |
|
I don't see other editors use this in their editable fields. They just use |
|
@ellatrix Here is the doc. Other editors should use it if they are not using native inputs. This role adds textbox semantics and |
|
So adding a role makes any element lose its normal semantic meaning? We're doing this for all elements including paragraph, heading etc. |
Yes, whenever a role attribute is defined, that changes the semantic definition of the element. The degree to which that matters varies, and a So in order for a screen reader to have an equal experience to the visual parity we're trying to keep for sighted users, the semantics of the elements need to be retained, as well. |
|
Let's try this one. Discussion can continue in a follow-up if there are major objections. Thanks! 👍 |
|
How does it work properly in TinyMCE without all the role=textbox? https://www.tiny.cloud/docs/tinymce/6/basic-example/ |
|
@ellatrix It isn't a 1-to-1 comparison. TinyMCE uses a single canvas editing area where all the blocks in Gutenberg are self-contained elements. The best way to compare TinyMCE would be Gutenberg code input mode vs. TinyMCE. If you inspect the iFrame, the |
What?
Currently, the table block on the back-end is not discoverable as a table by screen readers due to the
role="textbox"andcontenteditable="true"attributes set directly on<td>. The PR restructures the table so the edit field is contained within the<td>.Why?
It is important that screen readers behave well with native HTML elements.
How?
Change HTML structure.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast