Components behind the notepad remain focusable

phetsims / mean-share-and-balance

"Mean: Share and Balance" is an educational simulation in HTML5, by PhET Interactive Simulations.

GNU General Public License v3.0

2 stars 1 forks source link

Components behind the notepad remain focusable #272

Closed jessegreenberg closed 3 months ago

jessegreenberg commented 3 months ago

During #270 I noticed you can focus and interact with components behind the notepad. Should that be prevented?

amanda-phet commented 3 months ago

Yeah.. this is maybe because we aren't using a modal dialog? We did something custom for this sim so the dialog could be visible while still interacting with data. I think it would be wise to prevent this (on all screens).

marlitas commented 3 months ago

This is definitely because of the custom info dialog. I'm curious as to how we want to resolve this.

Do we want alt-input users to not be able to focus the elements at all (temporarily remove them from the traversal), or they can focus, but interactions are not possible? The latter will most definitely be easier. I will need to look into the former although I'm sure @jessegreenberg has thoughts as well.

I'm not positive, but this might be one of the reasons we currently do not support a non-modal dialog.

jessegreenberg commented 3 months ago

I wanted to make sure this wasn't already discussed - but if we agree it should be prevented, I think it would be best to remove them from the traversal order.

The way to do that would be to set pdomVisible on the relevant Nodes when meanInfoPanelVisibleProperty value changes.

jessegreenberg commented 3 months ago

The above is one way to do this, @marlitas can you please review?

amanda-phet commented 3 months ago

Should the focus move to the close button when this "dialog" is open?

jessegreenberg commented 3 months ago

Good question, I wasn't sure so I looked around on google - and I am seeing suggestions for both. This one says we shouldn't, this one said we should. Gemini said we shouldn't, ChatGPT said we should.

Thinking ahead to screen reader accessibility, I think it makes sense to focus the close button - this makes it clear that there is new content and that will be the mechanism for reading out the information in the panel/non-modal dialog.

We could also add an 'escape' hotkey to close it.

@terracoda which do you prefer?

amanda-phet commented 3 months ago

I think it makes sense for the next focus element to be the close button, since they just opened the dialog. Then, I could see the tab order going back to the number pickers (the data on the bottom).

jessegreenberg commented 3 months ago

OK, sounds good @amanda-phet. Over slack @terracoda said that focus should move to the close button, and a global hotkey should close the non-modal dialog. So the changes are

[x] Move focus to the close button when the non-modal dialog opens.
[x] Add a global hotkey to close it.
[x] Put the close button after the info button in the traversal order.
[x] When the close button is pressed, focus should go back to the info button. That is not the case for the global hotkey.

jessegreenberg commented 3 months ago

OK, the above three commits are for https://github.com/phetsims/mean-share-and-balance/issues/272#issuecomment-2163347057.

https://github.com/phetsims/mean-share-and-balance/commit/9fa448c5f7761c04b1776e36492a414a4ee68497 is for the original issue comment. @marlitas back to you for review.

marlitas commented 3 months ago

I reviewed and also checked the behavior in the sim. It looks like this is working well. Thanks for catching this and applying the fixes @jessegreenberg. I think we can close!

amanda-phet commented 3 months ago

I noticed that I can tab to the prediction line (on the Distribute screen) even when the dialog is covering it.

marlitas commented 3 months ago

Done above. @amanda-phet can you confirm all is looking well?

amanda-phet commented 3 months ago

Looks good. Thanks!