mozilla-lockwise / lockbox-extension

Experimental Firefox extension for login management experiences, not being actively developed
Mozilla Public License 2.0
127 stars 26 forks source link

React error with modals affecting screen reader users #532

Open kimberlythegeek opened 6 years ago

kimberlythegeek commented 6 years ago

While testing the 0.1.6 milestone we came across a react-modal traceback. It appears that this behavior was introduced during a dependency update, issue #472.

After talking to @jimporter it sounds like a fix is a bit involved and should be scoped appropriately. This impacts accessibility for screen reader users.

STR:

  1. Install #513
  2. Sign in to Lockbox
  3. Click portrait drop-down

Actual Results in this error in the console:

Warning: react-modal: App element is not defined.
Please use `Modal.setAppElement(el)` or set `appElement={el}`.
This is needed so screen readers don't see main content when modal is opened.
It is not recommended, but you can opt-out by setting `ariaHideApp={false}`.
jimporter commented 6 years ago

Mostly for my own reference so I don't forget, but as implied in #513, the main issue is that a simple fix for this will break tests. We'll basically need to pass appElement through our <ModalRoot/> widget so that we can set appElement to the main body in our real code, and to some dummy element in our test code.

@kimberlythegeek Have you taken a look at what happens with a screen reader when the dropdown is open? I'm not super familiar with how screen readers work, so I don't have an intuitive idea of how bad the behavior is currently. That would probably help us in prioritizing this. (Personally, I have a soft preference for fixing it relatively quickly no matter what, since I think accessibility is pretty important!)

kimberlythegeek commented 6 years ago

@jimporter I am running through basic functionalities using VoiceOver right now. I will share my notes soon.

kimberlythegeek commented 6 years ago

@jimporter I don't know a whole lot about how screen readers work, but when I turn on VoiceOver and open the drop down, as far as I can tell, the rest of the content is not visible (to the screen reader).

Tabbing just goes through the drop down items.

I have found other things that may need to be addressed as far as screen readers, though, and will list them in a separate issue (or multiple issues).

m8ttyB commented 6 years ago

@kimberlythegeek If it's helpful, we can also pair with Yura on this task and then share our findings with engineering.

m8ttyB commented 6 years ago

Un-assigning @kimberlythegeek. If this work gets picked up again and their is a need, we can investigate it then.