Improving accessability - added aria presentation role for layout tables #10108
Conversation
There was a problem hiding this comment.
Thanks for the patch, a few thoughts:
- consider spelling out UA? In the context of browsers and JS, it usually means "user agent", and I think you probably mean something else like usability/accessibility? Substituting "user agent"(e.g. "the browser") doesn't make sense in your title or description, and I don't see a obvious definition searching for the term in the context of dom and aria.
Co-authored-by: Colin Alworth <[email protected]>
Lonzak
changed the title
|
UA stands for Universal Accessibility but as always abbreviations mean something different in different contexts. Just accessibility is fine. I also changed the code to use 'Roles' class which is much better. Thanks for the pointer. |
I think the "space after Thanks! |
| //aria role to indicate it is a plain layout table | ||
| tableElem.setAttribute("role", "presentation"); | ||
| // aria role to indicate it is a plain layout table | ||
| Roles.getPresentationRole().set(tableElem); |
There was a problem hiding this comment.
In the other cases it's clear that the table is just for presentation, here it's not obvious, people might actually use this to display tabular data. Since users can easily add the role when needed, I would suggest leaving HTMLTable and its subclasses like Grid as is.
There was a problem hiding this comment.
Interesting - for me Grid and Flextable was only for layouting (... and in most cases it is probably used for this) if a data table is needed one should use CellTable or DataGrid. But sure there is no restriction and e.g. FlexTable could be manually extended with ...
There was a problem hiding this comment.
This should be removed.
CellTable and DataGrid are advanced widgets that had been introduced as a faster alternative to Grid/FlexTable, especially for slow browsers like IE 6 (or if the data table is really large). For smaller/normal data tables using Grid / FlexTable is the way to go. Of course you can always misuse them to do pure layout but then you have to set the aria role yourself.
There was a problem hiding this comment.
Ok removed the role setting for HTMLTable
| //aria role to indicate it is a plain layout table | ||
| tableElem.setAttribute("role", "presentation"); | ||
| // aria role to indicate it is a plain layout table | ||
| Roles.getPresentationRole().set(tableElem); |
There was a problem hiding this comment.
This should be removed.
CellTable and DataGrid are advanced widgets that had been introduced as a faster alternative to Grid/FlexTable, especially for slow browsers like IE 6 (or if the data table is really large). For smaller/normal data tables using Grid / FlexTable is the way to go. Of course you can always misuse them to do pure layout but then you have to set the aria role yourself.
| @@ -15,6 +15,7 @@ | |||
| */ | |||
| package com.google.gwt.user.client.ui; | |||
|
|
|||
| import com.google.gwt.aria.client.Roles; | |||
I think the "space after Thanks! |
There was a problem hiding this comment.
Looks like this is failing tests (verified in github actions by pushing to aria-test on my fork):
[junit] Running com.google.gwt.user.UiSuite
[junit] Tests run: 1008, Failures: 1, Errors: 38, Skipped: 0, Time elapsed: 143.649 sec
[junit] Test com.google.gwt.user.UiSuite FAILED
https://github.com/niloc132/gwt/actions/runs/14194633693
One of the stack traces:
java.lang.AssertionError: Element cannot be null.
at java.lang.Throwable.Throwable(Throwable.java:73)
at java.lang.Error.Error(Error.java:30)
at java.lang.AssertionError.AssertionError(AssertionError.java:51)
at com.google.gwt.aria.client.RoleImpl.set(RoleImpl.java:219)
at com.google.gwt.user.client.ui.TreeItem$TreeItemImpl.initializeClonableElements(TreeItem.java:92)
at com.google.gwt.user.client.ui.TreeItem$TreeItemImpl.TreeItem$TreeItemImpl(TreeItem.java:65)
at com.google.gwt.user.client.ui.TreeItem.$clinit(TreeItem.java:255)
at com.google.gwt.user.client.ui.TreeItem.TreeItem(TreeItem.java:307)
at com.google.gwt.user.client.ui.Tree.init(Tree.java:1088)
at com.google.gwt.user.client.ui.Tree.Tree(Tree.java:204)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.build_myTree(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:2162)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.get_myTree(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:2158)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.build_f_HTMLPanel1(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1350)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.get_f_HTMLPanel1(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1209)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.build_root(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1078)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.get_root(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1070)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.createAndBindUi(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:185)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.createAndBindUi(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:182)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest$MyWidgetBasedUi.init(UiBinderJsInteropTest.java:40)
at com.google.gwt.uibinder.test.client.WidgetBasedUi.WidgetBasedUi(WidgetBasedUi.java:277)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest$MyWidgetBasedUi.UiBinderJsInteropTest$MyWidgetBasedUi(UiBinderJsInteropTest.java:30)
at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest.testJsElementTypes(UiBinderJsInteropTest.java:45)
From looking at TreeItem.java, you are calling Roles.getPresentationRole().set(BASE_BARE_ELEM) before BASE_BARE_ELEM was first assigned in initializeClonableElements().
I did not check other stack traces.
According to accessibility rules layout tables exist merely to position content visually and should not be used in HTML5.
Screen readers may interpret them as data tables (i.e., announcing column and row numbers) etc). To change that in GWT would be a bigger task. So as a minor improvement those tables should be marked as layout tables. This can be done by assigning the role="presentation" to ensure it is not identified as a table to screen reader users.