search

phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

Keyboard navigation design for RewardDialog #96

Closed pixelzoom closed 1 year ago

pixelzoom commented 2 years ago

In https://github.com/phetsims/fourier-making-waves/issues/194#issuecomment-929646387, I discovered that vegas.RewardDialog currently does not support keyboard navigation. It looks like this:

screenshot_1298

Two design questions for @arouinfar:

(1) Based on the color of the "New Level" button, that would seem to imply that it should get focus when the dialog is opened. Is that correct?

(2) Can we swap the order of the push buttons? Because this is a modal dialog, the focus traversal order will include the "New Level" button, "Keep Going" button, close (X) button, and the browser's URL textfield. If we leave the dialog as it currently is, the order will be: "New Level" button -> browser's URL textfield -> close button -> "Keep Going" button. So I recommend that we swap the order of the 2 push buttons.

arouinfar commented 2 years ago

(1) Based on the color of the "New Level" button, that would seem to imply that it should get focus when the dialog is opened. Is that correct?

Yes, I would agree. "New Level" is where I would expect a student to go. If they've triggered the RewardNode, they have shown sufficient mastery of the current level, and I would want them to move to the next level.

(2) Can we swap the order of the push buttons?

Yes, let's swap the order of the buttons. I think it will flow better visually and in terms of focus order.

pixelzoom commented 2 years ago

Changes pushed to master and fourier-making-waves-1.0 branches.

The dialog now looks like this:

screenshot_1312

And the traveral order is: "New Level" button -> "Keep Going" button -> browser URL textfield -> close (X) button

@arouinfar please review in master. You can review in the context of Fourier, with ?showAnswers&showReward, which will show the reward after every correct answer.

arouinfar commented 2 years ago

And the traveral order is: "New Level" button -> "Keep Going" button -> browser URL textfield -> close (X) button

This seems a bit odd to me. Is there a reason why focus leaves the dialog between "Keep Going" and the close button? Could the order instead be "New Level" button -> "Keep Going" button -> close (X) button -> browser URL textfield?

The focus behavior works fine when using keyboard nav to check the answer, whether by hotkey or pressing Check Answer. However, if the user uses the mouse to click on "Check Answer" the first tab will bypass "New Level" and focus on "Keep Going".

pixelzoom commented 2 years ago

Assigning to @jessegreenberg to comment on this feedback from @arouinfar:

This seems a bit odd to me. Is there a reason why focus leaves the dialog between "Keep Going" and the close button? Could the order instead be "New Level" button -> "Keep Going" button -> close (X) button -> browser URL textfield?

@jessegreenberg described this to me as the expected behavior. I suggested that Dialog's pdomOrder should be [ content, closeButton ] in general, but that's not what Dialog currently does.

The focus behavior works fine when using keyboard nav to check the answer, whether by hotkey or pressing Check Answer. However, if the user uses the mouse to click on "Check Answer" the first tab will bypass "New Level" and focus on "Keep Going".

This sounds undesirable. @jessegreenberg has been working on this aspect of Dialog in https://github.com/phetsims/sun/issues/721. I'm guessing that Dialog is setting focus regardless of how the Dialog was opened, "New Level" has the focus (but is not highlighted) when the Dialog is opened via mouse, and then pressing Tab moves to "Keep Going".

jessegreenberg commented 2 years ago

s there a reason why focus leaves the dialog between "Keep Going" and the close button? Could the order instead be "New Level" button -> "Keep Going" button -> close (X) button -> browser URL textfield?

It was a design request at one point to have the close button come first. I think so that when focus moves into the dialog reading of non-interactive content could proceed from the top. @arouinfar can you please open an issue in joist if you would like this changed?

The focus behavior works fine when using keyboard nav to check the answer, whether by hotkey or pressing Check Answer. However, if the user uses the mouse to click on "Check Answer" the first tab will bypass "New Level" and focus on "Keep Going".

This is the same issue as https://github.com/phetsims/joist/issues/750, but with the correct behavior if I understand correctly. If we proceed with https://github.com/phetsims/joist/issues/750, that is how it will always behave when using mouse + touch. Maybe we should talk over there.

arouinfar commented 2 years ago

Is there a reason why focus leaves the dialog between "Keep Going" and the close button? Could the order instead be "New Level" button -> "Keep Going" button -> close (X) button -> browser URL textfield?

It was a design request at one point to have the close button come first. I think so that when focus moves into the dialog reading of non-interactive content could proceed from the top. @arouinfar can you please open an issue in joist if you would like this changed?

Thanks for the clarification @jessegreenberg. It seems a bit odd visually, but if the current implementation is better for screen readers, that's fine with me. This behavior currently exists in the Preferences dialog of John Travoltage. Focus automatically goes to the tabs, and the traversal order is then tabs > tab contents > browser URL > close button.

pixelzoom commented 2 years ago

This is the same issue as phetsims/joist#750, but with the correct behavior if I understand correctly. If we proceed with phetsims/joist#750, that is how it will always behave when using mouse + touch. Maybe we should talk over there.

Then this issue is on hold until phetsims/joist#750 is resolved, and that issue is now blocking.

pixelzoom commented 2 years ago

Discussed via Zoom with @arouinfar. She's OK publishing Fourier 1.0 with the current behavior of this dialog. So removing the "blocking" label.

Still on hold until phetsims/joist#750 is resolved.

zepumph commented 2 years ago

Today in design meeting we wanted to put the close button last in the focus order for this dialog. This worked out well over in https://github.com/phetsims/sun/issues/724 to add an option for this. I'll use that option here.

zepumph commented 2 years ago

Over to @arouinfar to confirm this is inline with the design.

arouinfar commented 1 year ago

Thanks @zepumph. The focus order looks great in master.

I don't think there's anything left for this issue, closing.