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

Grid has hardcoded tabIndex=0 that interferes with focusable contents #275

Closed
trevorr opened this issue Jun 15, 2016 · 7 comments
Closed

Comments

@trevorr
Copy link

trevorr commented Jun 15, 2016

I'm working on a component somewhat similar to react-virtualized-select, where I have drop-down menu containing a VirtualScroll that virtualizes a large number of choices, but I'd like the individual rows to be focusable. (The alternative of styling to appear focused, as is done in react-virtualized-select, can lead to accessibility issues, since things like screen readers don't understand styles.) I have focus mostly working by setting tabIndex=-1 on each item and handling keyboard up/down events to move the focus. However, I notice that the underlying Grid has a hardcoded tabIndex=0 [here], which causes the Grid div to become focused and (thanks to Bootstrap) display a blue outline whenever the scroll bar is clicked on. Is there a reason for this, or would it be possible to remove it? When I remove the property using the Chrome element inspector, I don't notice any loss of normal functionality (e.g. default keyboard scrolling still works), and my row stays focused (while still visible). If there's some use case it is supporting, perhaps it could be made optional?

@bvaughn
Copy link
Owner

bvaughn commented Jun 15, 2016

Hi @trevorr,

Grid is focusable (eg tabIndex=0) because it supports keyboard navigation for accessibility reasons. The focused outline can certainly be suppressed in a variety of ways (CSS class or style) if you dislike it.

The Grid itself being focusable shouldn't prevent cells/rows within it from being focused. If you click on a focusable child within a focusable parent, the child should be focused (and show the focus outline).

So I'm not sure what the exact problem is? Mind elaborating a bit more? :)

PS. I'd love to see the component you're working on if it's something you plan on sharing at some point. I enjoy seeing components built on top of react-virtualized!

@trevorr
Copy link
Author

trevorr commented Jun 15, 2016

@bvaughn, thanks for the quick reply! Yes, I could suppress the focus outline, but the real problem is that the Grid steals focus from the current row when the scrollbar is clicked. The easiest way to see it is to have a focused row, then click the scrollbox/thumb. Here are some screen shots, though the mouse cursor isn't visible:

https://www.evernote.com/l/AJgGb0Vtc0BFu7Aocx0N88Ql-Ebu_LKSkrU
https://www.evernote.com/l/AJjmVmDXkGxBwrjd4UmTfZ8RhkypCr-TvzI

When the items of the Grid are focusable, I don't think it makes sense for the container itself to be focusable. Perhaps rather than hardcoding it to be so, it should just allow tabIndex to pass through. If that might break existing apps, the default could be 0, but setting it to null would suppress it. (Of course, HOCs like VirtualScroll would need to behave similarly.)

PS. I definitely plan to open source the component soon as soon as it's stable and documented. It's essentially a native React and Bootstrap (a la react-bootstrap) "combobox" component that acts as a select (similar to select2) or input (by optionally not requiring the value come from the list).

@bvaughn
Copy link
Owner

bvaughn commented Jun 15, 2016

Ah, I see. Thank you for elaborating. I can see how that's not ideal for what you're describing.

I will add a tabIndex property and do a release momentarily.

Looking forward to seeing your component. :)

@bvaughn
Copy link
Owner

bvaughn commented Jun 15, 2016

New tabIndex property is available on Grid, FlexTable, and VirtualScroll as of release 7.7.0.

Cheers.

@bvaughn bvaughn closed this as completed Jun 15, 2016
@trevorr
Copy link
Author

trevorr commented Jun 15, 2016

Awesome, thanks!

@trevorr
Copy link
Author

trevorr commented Jun 15, 2016

I've verified that setting tabIndex={null} on VirtualScroll stops it from taking focus from its contents when the scrollbar is used. Thanks again!

@bvaughn
Copy link
Owner

bvaughn commented Jun 15, 2016

Thanks for the confirmation! :)

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

No branches or pull requests

2 participants