apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
62.24k stars 13.65k forks source link

SQL results tables: apparent autoscrolling is inaccessible to screen readers #24688

Open Grahamwp opened 1 year ago

Grahamwp commented 1 year ago

When I make an SQL query with Superset, the resulting table seems to have some sort of autoscrolling features that barely work with either of my Windows screen readers, JAWS (my primary screen reader) and NVDA. They say the table has 55 rows (the number of rows in the query result) but I can barely scroll past four or five rows without either being told I'm at the bottom of the table or sometimes it autoscrolls to something like the 6th or 15th row. It barely works on NVDA and almost doesn't work at all with JAWS. I'm using the Superset instance at: https://superset.wmcloud.org/superset/sqllab/

wsxover commented 1 year ago

This is probably because the result table is using React Virtualized which only renders "visible" rows. As such only about 5 rows are going to be present in the DOM where a screen reader can get at them. In visual user agents it is apparently trapping scroll events to then add the next rows into the DOM so they can be rendered, but this seems to not be working with the navigation events the mentioned screen readers are using.

The fix would require @bvaughn to figure out the events triggered by JAWS/NVDA and use those; or for Superset to switch to something other than React Virtualized as a framework (if so I would recommend something that uses actual table markup instead of a bunch of div elements tweaked up to kinda sorta behave like one).

That the result table is not accessible to screen readers is a rather critical problem IMO.

bvaughn commented 1 year ago

In visual user agents it is apparently trapping scroll events to then add the next rows into the DOM so they can be rendered

Libraries like react-virtualized and react-window can be configured to over-render rows so that there are some "next" and "previous" rows (not visible, but in the DOM) for screen readers.

(if so I would recommend something that uses actual table markup instead of a bunch of div elements tweaked up to kinda sorta behave like one).

That the result table is not accessible to screen readers is a rather critical problem IMO.

So far as I understand, ARIA roles should be sufficient here (but maybe I'm mistaken).

When I last looked into using actual table elements for virtualization, it didn't work well. I think I would have had to have overridden a lot of default tbody styles to get it to behave correctly, and it didn't seem worth pursuing.

wsxover commented 1 year ago

Libraries like react-virtualized and react-window can be configured to over-render rows so that there are some "next" and "previous" rows (not visible, but in the DOM) for screen readers.

Hmm. Depending on what exactly is going on that might be sufficient. But the symptoms Graham describe above sound more like react-virtualized getting confused by the navigation events JAWS and NVDA generate (jumping around, etc.) to me.

So far as I understand, ARIA roles should be sufficient here (but maybe I'm mistaken).

As MDN repeats (no less than three times): "Using a native HTML table element whenever possible is strongly encouraged." And Caniuse adds "Support for ARIA is rather complex and currently is not fully supported in any browser."

There's some info on JAWS support for ARIA in JAWS-ARIA-Support.doc on Freedom Scientific's web site (note absence of any mention of tables). I haven't found similar documentation for NVDA. The Accessibility Tree in Chrome DevTools may also help to figure out what JAWS' view of the faux table is (it uses the browser's accessibility API to get information about the web page).

I've no direct direct data on this, but in general terms using div+aria sounds iffy and I wouldn't personally have bet on it working correctly. Not that tables in HTML are that great, but they're a bit more of a known quantity. Especially when you go beyond the Big 4. JAWS in particular used to have heuristics to determine whether a table is a layout table or a data table, and a special "table mode" for navigating data tables, and this is the kind of thing I would have expected to break when not using a real table element.

Grahamwp commented 1 year ago

I don't personally know much about ARIA but I'd strongly recommend using a native HTML table here, for reliability and consistency, if nothing else. I asked a friend of mine who also uses a screen reader (primarily NVDA on Windows) and knows more about this sort of thing, @codeofdusk, and he very much agreed with me.

codeofdusk commented 1 year ago

Also CCing @dbjorge and @madalynrose for ARIA and Reactjs expertise.

Simon818 commented 1 year ago

The fix would require @bvaughn to figure out the events triggered by JAWS/NVDA and use those...

I can't speak to JAWS but when you have the option "Automatically set system focus to focusable elements" disabled. NVDA doesn't generate events at all. it is possible for NVDA's virtual cursor to be on a section of the page that isn't visually visible. I don't know a way around this besides simply rendering all rows in the DOM. I am also a bit skeptical about fudging an entire table structure with ARIA but I have technically seen it done in a way that is readable with NVDA, so I'm not going to definitively say it's impractical. My understanding is that when ARIA roles are defined properly, the controls should "look" like their given ARIA role and the screen reader may not even be able to tell the difference between that and native controls. The main problem I can foresee with tables is the complexity of maintaining the table itself, the headers, the rows, and the cells and having the ARIA line up well enough for screen readers to read them; however, if the framework claims to take care of that we could at least test to see whether screen readers play nice with the fake table as long as all rows are rendered. It seems like a reasonable initial step.

codeofdusk commented 1 year ago

"Automatically set system focus to focusable elements" disabled

This is the default (nvaccess/nvda#11190).

rusackas commented 6 months ago

Hi all... just wondering if there are any volunteers for this one. We'd love to see more volunteers on the Accessibility project.