Closed samreid closed 4 years ago
@zepumph and I collaborated to set the correct order and it didn't take long. Everything seems keyboard accessible aside from the toolbox elements. @emily-phet would you like to review?
UPDATE: This is pushed to master, you can review on phettest.
@samreid @zepumph Wonderful to hear this seemed quick and easy! Let's just take the keyboard nav for a spin during our next Waves Intro sound meeting. See you then!
In https://github.com/phetsims/waves-intro/issues/6#issuecomment-509767647 we agreed to focus on publishing 1.0 with the "tone" sound. It was unclear to me whether we would also focus on accessibility later or try to have that ready for 1.0. For now I have removed that flag in the preceding commit. @kathy-phet and @emily-phet at what point do you want to focus on finalizing publishing keyboard navigation?
a) as part of the initial 1.0 release (with "tone" sound) b) at the same time full sonification is published c) as a separate release between (a) and (b) d) as a separate release after (b)
I would vote for (b). Keyboard navigation also is tied to static description, so it seems we should think about that while doing keyboard nav. Emily?
@samreid Thanks for checking in on this.
I agree with @kathy-phet and option b) - let's consider the sonification and keyboard accessibility as a coupled pair of features, and plan to publish with these together.
@samreid I reviewed this, and found two issues:
On the home screen, i could use the keyboard to move focus, but could not use space or enter to select a screen. I needed to click with my mouse to select a screen. Enter and space should select the buttons on the home screen.
That was a problem caused by the refactoring done over in https://github.com/phetsims/joist/issues/601, but should have been fixed over a week ago in master. I confirmed fixed on master before writing this.
@zepumph Thanks!
I was thinking we could punt on #487 until a later release, but since it will impact keyboard navigation, we should do that now.
I updated to use the TimeControlNode and placed it before the ResetAllButton in navigation order. It seems like the only thing missing keyboard navigation is the toolbox and its tools:
Are we adding that for the upcoming release?
Thanks @samreid.
No, the toolbox and tools will not be accessible by alternative input for this release. Because of this, the sim should also not be publicly identified as having alternative input.
It would take more resources than we have available at the moment to support alternative input for three custom objects, each with sub-controls. Additionally, any solutions identified in the absence of interactive description may need to be revised if/when interactive description is added to the sim. Early in the design process we decided we would not tackle alternative input for the toolbox currently.
Thanks @emily-phet. This keyboard accessibility implementation is ready for review.
@samreid I agree with @emily-phet. Since the tools do not have alternative input design patterns (which would take quite a bit of work), I think we should pass on including it in the upcoming release with sound.
For the record, I would place the toolbox between the main control panel (frequency, amplitude, checkboxes) and the continuous/pulse radio buttons. I think the tools are less important that the controls in in the main control panel, and more important than the controls near the bottom of the lattice.
This will put the focus order a bit out-of-order visually, but I don't think it's extreme. The toolbox is on top of the control panel so it has a consistent position on all three screens. The shifting toolbox position would have been particularly jarring in Wave Interference which has significantly different UI controls across its screens. In interviews on wave-interference and waves-intro, I didn't notice any flow issues with the tool placement at the top.
@samreid The focus order is ultimately up to the team, with the caveat that "reset all comes last" before navigation moves into the nav bar has become a convention. When making final decisions, it may be helpful to consider that the focus order will also need to align with the order in which objects are described, if/when this sim gets description.
Regarding placement of the toolbox - I believe we decided to move it below the frequency/amplitude toolbox in our last meeting. In multiple interviews @BLFiedler has conducted, learners pulled out tools before moving sliders - which makes sense to me. It was brought up why the toolbox was located on top for the Wave Interference team, but those present in the meeting had no reservations at the time for moving the tools below the frequency/amplitude control panel at the time.
Ultimately, up to you all, but just wanted to note the previous conversation on this, and @BLFiedler's interviews.
At today's design meeting, we reviewed the tab order today and agreed it seems reasonable. @jbphet asked if @terracoda should take a look.
@kathy-phet agreed @terracoda can take a look, but this is somewhat different since it doesn't have interactive descriptions, can you take 15 minutes or less looking specifically at the tab order, for visual users only?
Regarding placement of the toolbox - I believe we decided to move it below the frequency/amplitude toolbox in our last meeting.
In today's meeting, we decided to defer this discussion until we add keyboard navigation for those tools.
Keyboard order looks fine to me. I did not listen with a screen reader :-)
Note that scenery-phet now has a hotkey for Pause/Play in common code.
Also, not in Molecules in Light we decided to move the Sims Speeds to the left of the Pause/Play. The slow motion is much more useful for blind learners.
Ultimately, it is up to the team to decide if the Sim Speed radios are a preferred option to Pause/Play/Step and then reverse the order, or leave as is.
I think it works OK as it is, though my only hesitation with moving the radio buttons to the left is they be right next to another set of radio buttons:
Or we could move the play/pause button further to the right?
After looking at these, I'm inclined to leave it as it is visually, though it may be OK to put the speed radio buttons in order before the play/pause buttons.
@terracoda I am hesitant to generalize a pattern that worked in MAL to all sims. In MAL, play/pause is a much more secondary action for all users and it simplified the described experience.
Having the radio buttons on the left will always be worse for i18n since the strings have less room to expand. The trapped space between the ragged edge of the strings and the play/pause button is also less aesthetically pleasing.
Aside from appearance, there are also pedagogical reasons to keep the play/pause/step buttons on the left side of the TimeControlNode. In wave-interference and waves-intro, play/pause/step are much more critical than slow motion.
@samreid please keep the normal/slow radio buttons on the right side of the TimeControlNode.
Thanks, it seems this issue is ready to close.
@zepumph asked if it would be easy to set the correct keyboard navigation order. We will take a look.