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

Table block: the RichText within the table cells shouldn't have role=textbox and aria-multiline=true #12525

Open
Tracked by #32400
afercia opened this issue Dec 2, 2018 · 11 comments
Labels
[Block] Table Affects the Table Block Needs Decision Needs a decision to be actionable or relevant [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@afercia
Copy link
Contributor

afercia commented Dec 2, 2018

For all the usages of the RichText component within the various blocks, Gutenberg uses a role=textbox and an aria-multiline=true attribute to make the editable field perceived by assistive technologies as equivalent of a <textarea> element.

The only exception used to be the Table block: a specific exception was coded a while ago:

/*
* The role=textbox and aria-multiline=true must always be used together
* as TinyMCE always behaves like a sort of textarea where text wraps in
* multiple lines. Only the table block editable element is excluded.
*/
if ( tagName !== 'table' ) {
ariaProps.role = 'textbox';
ariaProps[ 'aria-multiline' ] = true;
}

The rationale behind this exception is:

  • all the other blocks editable areas need to be communicated to assistive technologies like a textarea (some doubts were expressed about the List block)
  • instead, the Table block needs to keep the table native semantic, and the table cells need to be communicated as editable cells

This worked but the Table block and RichText have seen some refactoring over time and now seems it doesn't work any longer:

screenshot 2018-12-02 at 17 00 32

As a consequence, screen readers announce a textarea within a table cell, which is potentially confusing for users:

screenshot 2018-12-02 at 16 32 54

@afercia afercia added [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release labels Dec 2, 2018
@ellatrix
Copy link
Member

ellatrix commented Dec 3, 2018

Current behaviour seems fine to me? What do you want to change? You are in a text area in a table cell.

@afercia
Copy link
Contributor Author

afercia commented Dec 3, 2018

I'd like the cells editable areas to not be communicated as texareas. That means: removing role=textbox and aria-multiline=true when RichText is inside the table.

That worked with the previous implementation of the Table block because the passed tag name was table. Now, at the very least the exception

if ( tagName !== 'table' ) { ...

doesn't make sense because tagName is the default div. Checking for tagName wouldn't work. The only option I can see would be checking the className but that's a bit meh... there's the need to pass something from the Table block to RichText to make it aware it's within a Table block and not render role=textbox aria-multiline=true.

@ellatrix
Copy link
Member

ellatrix commented Dec 3, 2018

Before the whole table was an editable field, so that made sense to me.

Currently each cell is a editable field, not the whole table, so it is the same as placing a text area in each cell. They are no different from any other editable field, e.g. a caption. Both are text boxes and multiline.

@afercia
Copy link
Contributor Author

afercia commented Dec 3, 2018

From a semantic perspective and considering how screen readers announce the current semantic, it's not the same thing.

I'm just asking to restore the previous semantics, when the table cells were announced as editable table cells. Instead, now screen readers will announce a textarea within a table cell, which is potentially greatly confusing for users.

This was the only reason why that exception was introduced. Can we please restore a similar behavior? 🙂

@ellatrix
Copy link
Member

ellatrix commented Dec 3, 2018

But they are text areas in a table cell?

What can we change?

@ellatrix
Copy link
Member

ellatrix commented Dec 3, 2018

Atm, we cannot have <td contenteditable> as there are wrapping elements which would be invalid HTML.

@afercia
Copy link
Contributor Author

afercia commented Dec 3, 2018

They're not textareas 🙂 They're <div> elements with a contenteditabe attribute.

  • role="textbox" aria-multiline="true" give them the semantic of a textarea
  • if we remove role="textbox" aria-multiline="true" they will be just div elements, and only the parent cell will be announced

@ellatrix
Copy link
Member

ellatrix commented Dec 3, 2018

:)

I'll rephrase my question...

But they are semantically text areas?

if we remove role="textbox" aria-multiline="true" they will be just div elements, and only the parent cell will be announced

That seems wrong? Why is a table cell any different from any other rich text instance? The cell is announced, so everything seems fine to me?

@afercia
Copy link
Contributor Author

afercia commented Dec 3, 2018

A table element is very different from all the other elements, as there are relationships between the cells that are announced by screen readers. Quoting from the original PR #4074 (comment)

we should not set the role="textbox" on blocks that are completely different from inputs or textareas:

  • list: loses its native semantics: number of items etc. are no longer announced
  • table: same, the number of rows and columns are no longer announced, as well as the current position in the table

Later, we changed our mind about the List block and kept the exception only for the Table.

@ellatrix
Copy link
Member

ellatrix commented Dec 3, 2018

So if we remove the attributes, rows and columns and current position is announced, and that's currently not the case?

we should not set the role="textbox" on blocks that are completely different from inputs or textareas

The current implementation is the same as putting a text area element inside a table cell, so I don't really get what's different about it.

@ellatrix ellatrix added the [Block] Table Affects the Table Block label Feb 8, 2019
@talldan talldan added the Needs Decision Needs a decision to be actionable or relevant label Jul 19, 2019
@alexstine
Copy link
Contributor

Is this one still relevant?

Body cell text  edit  multi line  
blank
Body cell text  edit  multi line  column 2  
Body cell text  edit  multi line  row 2  
Body cell text  edit  multi line  column 1  

Seems like the table info is still read. Not sure if that would be the case for Voiceover or not, but works well enough for NVDA. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Table Affects the Table Block Needs Decision Needs a decision to be actionable or relevant [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

No branches or pull requests

4 participants