Skip to content

[Form][WCAG] Added role="presentation" on tables & removed bootstrap4 table #26326

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

Closed
wants to merge 3 commits into from

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Feb 26, 2018

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

According to my friend and WCAG2 expect Sandra:

Tables works best for table data, it should not be used for doing layouts. If you really really want to use the label add role="presentation". This will make screen readers to ignore the table structure which will make it easier to navigate. It will also prevent screen readers to read "row 1, column 1".
But we should consider not using a table here.

This will prevent screen readers to read "row 1, column 1".
@ogizanagi
Copy link
Contributor

But we should consider not using a table here.

I agree. I'm the one who introduced the table layout in #20887 for the bootstrap 3 layout in order to provide a good-enough default output for this type. At the time, bootstrap 3 wasn't offering much (AFAIK) for displaying such a heavy type and my guess is that the base theme is overridden for most accessibility needs.
However, bootstrap 4 is now offering form grid with col-auto sizing which may have suited better 😕

@fabpot
Copy link
Member

fabpot commented Feb 27, 2018

As we are still doing fundamental changes to the Bootstrap 4 integration, wouldn't it be better to remove usage of tables as indicated by @ogizanagi?

@Nyholm
Copy link
Member Author

Nyholm commented Feb 27, 2018

Thank you @ogizanagi for clearing this out. I did not know why we are using a table.

@fabpot: Yes, we are still doing changes to the Bootstrap 4 theme. I will be done within a few (2-3) weeks. I suggest to "release" Bootstrap 4 theme with 4.1.

I agree with you both. I think we should redo this without tables. I'll update the PR today

@nicolas-grekas
Copy link
Member

Status: needs work

@Nyholm
Copy link
Member Author

Nyholm commented Feb 28, 2018

What do you think about something like this:

Before:
screen shot 2018-02-28 at 18 21 37

After:
screen shot 2018-02-28 at 18 25 18

@Nyholm Nyholm changed the title [Form][WCAG] Added role="presentation" on tables [Form][WCAG] Added role="presentation" on tables & removed bootstrap4 table Feb 28, 2018
@nicolas-grekas
Copy link
Member

Thank you @Nyholm.

nicolas-grekas added a commit that referenced this pull request Mar 1, 2018
… bootstrap4 table (Nyholm)

This PR was squashed before being merged into the 3.4 branch (closes #26326).

Discussion
----------

[Form][WCAG] Added role="presentation" on tables & removed bootstrap4 table

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

According to my friend and WCAG2 expect [Sandra](https://twitter.com/sandrability):

> Tables works best for table data, it should not be used for doing layouts. If you really really want to use the label add `role="presentation"`. This will make screen readers to ignore the table structure which will make it easier to navigate. It will also prevent screen readers to read "row 1, column 1".
> But we should consider not using a table here.

Commits
-------

635220a [Form][WCAG] Added role=\"presentation\" on tables & removed bootstrap4 table
@Nyholm Nyholm deleted the 2-wcag-tables branch March 1, 2018 08:59
@Nyholm
Copy link
Member Author

Nyholm commented Mar 1, 2018

Thank you for merging

This was referenced Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants