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

[Dialog] does not scope tabbing #156

Closed
ryanflorence opened this issue Dec 19, 2014 · 10 comments
Closed

[Dialog] does not scope tabbing #156

ryanflorence opened this issue Dec 19, 2014 · 10 comments
Assignees
Labels
accessibility a11y bug 🐛 Something doesn't work design: material This is about Material Design, please involve a visual or UX designer in the process

Comments

@ryanflorence
Copy link

Accessible dialogs need to scope tabbing inside the dialog:

  1. Open the dialog
    • Dialog should get focused (or the close button in the dialog)
  2. Hit "tab" at the last tabbable element
    • focus should move to the first tabbable element
  3. Hit "shift + tab" on the first tabbable element
    • focus should move to the last tabbable element

You can see how we do it in react modal https://github.com/rackt/react-modal/blob/master/lib/helpers/scopeTab.js

There are numerous other issues, like handling a focus from the browser chrome into the app with an active dialog, it should focus the first tabbable element in the dialog. Also, after closing the dialog you need to return focus to the element that triggered the dialog to open. https://github.com/rackt/react-modal/blob/master/lib/helpers/focusManager.js

Finally, please consider this: http://instructure.github.io/blog/2014/09/25/accessible-dialogs/

@mewben
Copy link

mewben commented Apr 17, 2015

It would be good too if it has a feature to set the default button autofocused just like a normal dialog behaves. :)

@hai-cea hai-cea changed the title Dialog does not scope tabbing [Dialog] does not scope tabbing Jun 18, 2015
@hai-cea hai-cea mentioned this issue Jun 27, 2015
@alitaheri alitaheri added new feature New feature or request and removed Usability labels Dec 8, 2015
@EloB
Copy link
Contributor

EloB commented Apr 7, 2016

Interesting that this is labeled "feature request". I would rather say this is a bug. There isn't anyone that excepts you to focus and also edit elements under a overlay.

@nathanmarks nathanmarks added bug 🐛 Something doesn't work and removed new feature New feature or request labels Apr 7, 2016
@nathanmarks
Copy link
Member

@EloB agreed

@nathanmarks
Copy link
Member

@newoga

Can we look at the implementations in these libs:
https://github.com/reactjs/react-modal
https://github.com/react-bootstrap/react-overlays/blob/master/src/Modal.js

they both support the correct behaviour here

@nathanmarks
Copy link
Member

@mbrookes

Spec also requires this behaviour: https://www.google.com/design/spec/components/dialogs.html#dialogs-behavior

Dialogs should never be obscured by other elements or appear partially on screen. Dialogs always retain focus until dismissed or a required action has been taken, such as choosing a setting.

@nathanmarks nathanmarks added the design: material This is about Material Design, please involve a visual or UX designer in the process label Apr 7, 2016
@nathanmarks nathanmarks added this to the 0.16.0 Release milestone Apr 8, 2016
@newoga
Copy link
Contributor

newoga commented Apr 8, 2016

@nathanmarks I agree -- first time seeing this issue. I'll explore this more as this really needs to get fixed.

@nathanmarks
Copy link
Member

nathanmarks commented Apr 21, 2016

@newoga

I started looking into this. Would you be open to the idea of drawing aspects from both the https://github.com/reactjs/react-modal repo that @ryanflorence shared a long! time ago and react-overlays?

I'm interested in putting together a proof of concept that would draw inspiration from these libraries, but be an implementation that is catered to our use case and use our own internal dependencies rather than all the utils etc that a library like react-overlays will add to our build. I don't think we should underestimate the value of having full control over a component that is at the centre of important core functionality.

Edit: Changed my mind

@nathanmarks
Copy link
Member

@newoga

Well well... maybe i'll try react-overlays 😄 reactjs/react-modal#156

@nathanmarks
Copy link
Member

Just an update here. I have a dialog rewrite ready (will be submitting it after the styling update) with a much improved API and this issue fixed.

@theboyknowsclass

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y bug 🐛 Something doesn't work design: material This is about Material Design, please involve a visual or UX designer in the process
Projects
None yet
Development

No branches or pull requests

8 participants