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

a11y: 'tab' stops scrolling through Table with onRowClick #625

Closed
Dattaya opened this issue Mar 22, 2017 · 10 comments
Closed

a11y: 'tab' stops scrolling through Table with onRowClick #625

Dattaya opened this issue Mar 22, 2017 · 10 comments

Comments

@Dattaya
Copy link

Dattaya commented Mar 22, 2017

Seems like accessibility is broken in Table and maybe other components (I haven't tried).
First case: it happens if I press 'Shift-Tab' so it moves the scroll bar up as you can see on the next image. After that the last item is the last visible item, because it then switches focus to a button:
giphy

Second case if I press 'Up Arrow' key in a way that partially or fully hides focused item:
giphy

You can find this table on codepen: http://codepen.io/dattaya/full/evrYqM/

@bvaughn
Copy link
Owner

bvaughn commented Mar 22, 2017

This is an unfortunate side-effect of windowing. (There are some others, such as "find in page" browser features won't work in the normal way since content isn't actually on the page.)

I'm not sure how easy this would be to fix, to be honest, nor if it belongs in the base react-virtualized library. (It could make a nice addon though.)

@Dattaya
Copy link
Author

Dattaya commented Mar 22, 2017

Thank you for the explanation, @bvaughn. I don't yet know how this library functions internally so I'm might be saying something stupid but would rendering few more rows that are not visible above and bellow help to resolve this particular issue?

@bvaughn
Copy link
Owner

bvaughn commented Mar 22, 2017

That is already possible, actually. By default, react-virtualized renders 10 rows below the threshold. (Check out the overscanRowCount property.)

@Dattaya
Copy link
Author

Dattaya commented Mar 23, 2017

I've just realized that Shift-Tab doesn't work at all unless half of an element is shown even then the behavior is buggy as demonstrated above.

nor if it belongs in the base react-virtualized library (It could make a nice addon though.)

Can this be solved in an add-on or requires changes to the core?

@bvaughn
Copy link
Owner

bvaughn commented Mar 23, 2017

I don't know. That sounds like built-in browser behavior, considering the next row(s) are in the DOM just not visible.

@bvaughn
Copy link
Owner

bvaughn commented Mar 24, 2017

I see the problem. If you scroll down first, then use Shift+TAB to scroll back, Grid shifts the rows as described here. The result of this though is that there are no rows forward to tab to.

You could work around this by specifying a naive custom overscanIndicesGetter as shown here:
http://codepen.io/bvaughn/pen/oZyZKg

Admittedly this isn't a very satisfying default behavior though. Hm.

@Dattaya
Copy link
Author

Dattaya commented Mar 28, 2017

Thank you, @bvaughn. I've tested your example with a custom overscanIndicesGetter in most of the browsers on my PC (Chrome, Firefox, Edge, IE10-11) and it works fine. In FF one row above and bellow the threshold is not enough for speedy (when you hold the key pressed) scrolling with Tab or Shift-Tab but 2 is sufficient.
However I'm not closing this issue in case you want to make it the default behavior.

@bvaughn
Copy link
Owner

bvaughn commented Mar 28, 2017

Thanks for the follow-up info!

I'm going to leave it open for now and give it some more thought. I think maybe I can improve this by default without impacting perf too much.

bvaughn pushed a commit that referenced this issue Apr 5, 2017
…s always at least 1 row overscanned in a given direction. This helps with a keyboard accessibility (TAB / TAB+SHIFT) problem reported in issue #625. Grid overscan behavior is not adjusted by default because I didn't want to impact performance too much.
@bvaughn
Copy link
Owner

bvaughn commented Apr 5, 2017

Default behavior has been fixed/corrected via e6656c1. It will go out with the next point release.

@bvaughn bvaughn closed this as completed Apr 5, 2017
@bvaughn
Copy link
Owner

bvaughn commented Apr 5, 2017

9.6.0 release just went out with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants