Change the "Lock Ratio" checkbox into a toggle button

[jessegreenberg]

In https://github.com/phetsims/scenery-phet/issues/794 @terracoda mentioned that it would be ideal if the "Lock Toggle" checkbox became a toggle button. There were benefits in UI consistency and also the Voicing content.

@zepumph are you OK with this? @terracoda would we need any Voicing changes in this sim if we make this change?

[zepumph]

Sounds great thanks.

[jessegreenberg]

Sounds good, let me know if you would like me to work on it!

[zepumph]

If you are converting other usages, that would be great, but otherwise it will just sit here until we next work in this sim.

[terracoda]

Looking into the description strings now.

[terracoda]

As for the description strings for both Interactive Description and Voicing, I think we only need to make the Accessible name dynamic for both.

Ratio {{Unlocked}}, button Accessible Name (dynamic):

• Ratio Unlocked
• Ration Locked

For RaP, we need the full name of the button on the screen.

If I am correct, I think the help text and the context responses will work as already implemented for both Interactive Description and Voicing.

[terracoda]

Also, this toggling button, since it has a dynamic name should NOT be read out as an HTML toggle button that is pressed or not pressed.

[terracoda]

@zepumph, assigning to you for implementation, is that correct?

[terracoda]

@zepumph, there is one help text string that is unique to Interactive Description, and that is because in Voicing there is no recognition of disabled controls. Once we can voice disabled controls, I think we could use the same help text string in Voicing.

[jessegreenberg]

From https://github.com/phetsims/ratio-and-proportion/issues/549#issuecomment-1377676561, Ill try to get to this quickly. I had recommended taking this on because I thought it might a satisfying thing to check off but did not want to implicate @zepumph and take his time.

[terracoda]

I documented the ratio lock as a Ratio Unlocked/Locked button in the Interactive Description design document here at Ratio Unlocked/Locked, button.

In Voicing Design document, I documented the Ratio Locked/Unlocked button here.

[jessegreenberg]

OK, this is done - @terracoda would you like to review?

I went with a white button to match the other buttons on the screen.

[terracoda]

I am excited to review :-)

[terracoda]

Ok, we have a few issues for both Interactive Description and Voicing:

1. Visual Design: I wanted the name on the screen and the accessible name to be the same for this sim since we have the room on the screen. Is that possible?

2. Issues with the arrangement of/firing of the actual description strings. I'll break this up by feature, but I think all but one string is used in both features:

Interactive Description

• [x] I do not hear the context responses when I toggle the button
  • When toggling to locked position I should hear: "Hands locked at challenge ratio. Move hands and observe."
  • When toggling to unlocked position I should hear: "Hands no longer locked."
  • Note: I am correctly hearing, "Hands no longer locked." when I knock the hands out of ratio, and that hands were locked.
• Note that the help text for the enabled unlocked/ enabled locked / and disabled states of the button are all correct in the PDOM.

Voicing

• [x] Context Responses: I am not hearing context responses when I toggle the button. If helpful hints is checked in Preferences, I incorrectly hear the hint response (noted in next bullet). I should hear the same context responses (after the name response) if checked in Preferences:
  • When toggling to locked position I should hear: "Ratio Locked, Hands locked at challenge ratio. Move hands and observe."
  • When toggling to unlocked position I should hear: "Ratio Unlocked, Hands no longer locked."
  • As for Interactive Description, I am correctly hearing, "Hands no longer locked." when I knock the hands out of ratio.
• [x] Helpful hints: When checked, the hint response fires correctly on focus, but is also fires when the button is toggled. The helpful hint (i.e., help text) should only fire on focus for this type of interaction, and not fire when the button is toggled.

Let me know if you have any questions. I did do a fresh pull, so hopefully, I was using the latest implementation.

The button looks good!

[jessegreenberg]

Great, thanks for the testing @terracoda, sorry I missed these things.

I wanted the name on the screen and the accessible name to be the same for this sim since we have the room on the screen. Is that possible?

Yes that is possible. But there could be a layout problem. The text is right aligned with the ResetAllButton so when it changes it is going to shift the button to the left/right. How would you like to handle that?

Interactive Description...I do not hear the context responses when I toggle the button

Sorry, added above.

Voicing - Context Responses....Helpful hints:

Both of these were caused by scenery bug https://github.com/phetsims/scenery/issues/1026, a workaround has been added above.

Back to @terracoda to review the layout question.

[terracoda]

It's sound great now @jessegreenberg!

As for the layout issue, the interaction is technically part of the Play Area. I think it would be fine visually to move it up and align the left side of the left side of the button with the My Challenge accordion box allowing more than enough space for the open accordion.

Something like this. Note that I moved the Reset All button a bit to the left to align it with the right side of the Accordion box. Is that allowable? If not, maybe it's ok to have the Reset All button a little over to the right?

I think the above would work, but I better check with @BLFiedler. What do you think @BLFiedler ?

[brettfiedler]

@terracoda's reasoning makes sense to me. Okay with this change to the layout.

[jessegreenberg]

I can't do this unfortunately. There is some layout code specific to this sim related to window resizing that places all of that content in a container and makes things are right aligned. I think we need the responsible dev to continue.

@terracoda just FYI, would you like to try a a design with a visual label that does not change? If so I think this issue will have to wait.

[terracoda]

Oh dear, our flexible architecture needs to be more flexible ;-)

Just a thought, but would it work if the button label, regardless of locked and unlocked state was always as wide as the accordion, would that make it work? I think this is called the click area or accessible bounds?

For the short term, since the responsible dev is not available, I might be able to make the button work with an unchanging label.

[terracoda]

@jessegreenberg, please change the name response and the accessible name in the PDOM back to "Ratio Lock".

I reviewed (and updated) both the Voicing Design doc and the Interactive Description design doc. While I feel having the locked states written on the screen would benefit all learners, I don't think anything will sound broken if we have the static button name, "Ratio Lock".

[terracoda]

Please assign back to me to have a listen :-)

[jessegreenberg]

Just a thought, but would it work if the button label, regardless of locked and unlocked state was always as wide as the accordion, would that make it work? I think this is called the click area or accessible bounds?

Yes, I think that could definitely work. Its possible new scenery layout code would make that trivial but I don't know how to use it yet unfortunately. So I don't feel like I can do this quickly. Touch areas do not change layout bounds. The complicated layout in this sim comes from https://github.com/phetsims/ratio-and-proportion/issues/30.

The Voicing and PDOM name for the toggle is constant again, @terracoda would you like to have a listen?

[terracoda]

Going to have a listen now, sorry, I missed this.

[terracoda]

Yes, I think it sound good. Though, I think for Voicing, it will sound better with a dynamic name, it works as is. I tested VoiceOver in Safari and in Chrome. With the additional semantics you get with Screen Reader access, e.g., "Ratio Lock, button", the name and the context responses totally work fine.