Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement both hands voicing #432

Closed
zepumph opened this issue Feb 24, 2022 · 18 comments
Closed

Implement both hands voicing #432

zepumph opened this issue Feb 24, 2022 · 18 comments

Comments

@zepumph
Copy link
Member

zepumph commented Feb 24, 2022

From #363. @terracoda, I'm not sure the status on the both hands design yet. Can you let me know when it's ready. This isn't blocking me or anything, I'm just making some organizational issues for the remaining work.

@terracoda
Copy link
Contributor

Not ready yet. Will get back to that now that Indy hands are working.

@zepumph
Copy link
Member Author

zepumph commented Mar 4, 2022

Today during @terracoda and my RAP meeting, we decided it would be nice to have the current PDOM descriptions for the both hands interaction added to voicing as a baseline. We noticed this was already implemented for the individual hands when the ratio is locked (forwards to both-hands description content). I'll add that for BothHandsPDOMNode.

@zepumph
Copy link
Member Author

zepumph commented Mar 4, 2022

I was able to hide the both hands interactive highlight, but I made phetsims/scenery#1372 to find a better way to do it.

@zepumph
Copy link
Member Author

zepumph commented Mar 4, 2022

Alright. Implemented. Let me know how the design goes @terracoda and let me know if you have any questions or thoughts to bounce back and forth.

@zepumph
Copy link
Member Author

zepumph commented Apr 1, 2022

From today's design meeting with @terracoda:

  1. Both hands need the same implementation that the indy hands have for mouse (with ratio locked), where you get distance progress while dragging, and distance region on end drag. Keyboard will still alternate like description.
  2. Next step: shorten the description by removing the distance region (when applicable) when tick marks are not hidden. For mouse this is just on end drag, for keyboard this means finding a way to not alter description, but still keeping distance progress every other for voicing. SEe Shorten both hands description when tick marks are visible by removing distance region #460
  3. We think it is a bug that we never get distance progress while using the both-hands interaction. Perhaps we are wrong, but it needs investigation. (see HandPositionDescriber should not be dependency injected #458)
  4. When ratio is locked, let's try renaming each hand's name response to be "Both Hands" (done in a6d0f8a)
  5. When ratio is locked, change the interactiveHighlight of each indy hand to be the both hands focus highlight (around both ratio halves) See Implement both hands voicing #432 (comment)

@terracoda
Copy link
Contributor

@zepumph, at our meeting on Friday, I didn't mention that the Hint Responses are custom strings:

Before interaction or before full interaction with each of the Both Hands interaction when icons are still on the screen:

  • Move hands. {{W and S keys move left hand, Up and Down arrow keys move right hand.}}

After successful interaction with each of the Both Hands hands:

  • Move hands.

@terracoda
Copy link
Contributor

@zepumph, and when ratio is locked, and one of the hands (of Both Hands) is at an edge, the hint will need the special wording mentioned here #452 (comment)

This should help guide the direction at start of drag rather than at end of drag when ratio is locked.

@terracoda
Copy link
Contributor

@zepumph in your comment:

  1. We think it is a bug that we never get distance progress while using the both-hands interaction. Perhaps we are wrong, but it needs investigation.

You meant to say:
3. We think it is a bug that we never get distance progress while using the both-hands interaction with Ration Locked. Perhaps we are wrong, but it needs investigation.

I am investigating.

@terracoda
Copy link
Contributor

In the published version, I do indeed hear distance-progress. So this is currently a bug on master and this bug is also appearing to our default PDOM implementation for voiced Both Hands interaction.

@terracoda
Copy link
Contributor

In the Interactive Description design document, there is a lot of work on how to switch back and forth distance-progress and distance-regions, so the distance-region would not repeat, and we could have shorter responses using distance-progress.

That logic is now missing in master.
I think, this is the same logic we have for the indy hand sliders (alternating back and forth between distance-progress and distance-region), though the slider experience uses different strings.

For Interactive Description (Both Hands and Both Hands ratio locked) we need to find out where/when we lost the logic that was already there. We still have distance-progress logic for the indy hand slides (ID & Voicing).

For Voicing, I think we should continue making much greater use of the distance-progress which means following the points 1 & 2 in comment

@terracoda
Copy link
Contributor

Note that on Master with interactive description using Both Hands, I can get the occasional distance-progress, but often when I am just starting out the distance-progress is the wrong one.

In the published version, I can get distance-progress sometimes when I land on ratio. As the logic is cleaned up, let's fix that.

I think generally across the board for discrete input we want to be alternating between distance-progress&position and distance-region&position except for 1 case:

  • when moving into ratio we always get distance-region&position

When looking specifically at Voicing and the Both Hands interaction, because the distance-region makes the responses super long for a combined visual experience, we want to only have distance-region when Tick Marks are hidden.

For Voicing Both Hands interaction with discrete input when Tick Marks are enabled we will alternate between getting distance-progress&position and just position and then when moving into ratio we will only get position.

Also, there is special wording for Hint Responses, please see #432 (comment) and #457

@zepumph
Copy link
Member Author

zepumph commented Apr 6, 2022

When ratio is locked, change the interactiveHighlight of each indy hand to be the both hands focus highlight (around both ratio halves)

  • This is non-trivial. It would create a dependency where each ratio half needs to know about the both hands object (the "whole") while being created, but the whole needs to know about each half to be created. How important does this feel to you @terracoda?

@zepumph
Copy link
Member Author

zepumph commented Apr 6, 2022

It looks like item (3) was actually 2 issues/bugs. I created #458 and #459 to discuss them. I think that #459 is going to be where @terracoda and I discuss all of @terracoda's comments since #432 (comment)

@zepumph
Copy link
Member Author

zepumph commented Apr 6, 2022

Up next is the main portion of this issue, where we divide out the both-hands interaction to focus/drag/enddrag like for the indy hands. I will get to this soon.

@terracoda
Copy link
Contributor

terracoda commented Apr 13, 2022

Re #432 (comment) @zepumph, let's forget about changing any interactive highlights. I like the way the Voicing is sounding by calling all three hand interactions Both Hands when the Ratio Lock is checked.

zepumph added a commit that referenced this issue Apr 14, 2022
…roportionOverridesDistanceProgress and so that both hands still get distance progress, #459 #432
@zepumph
Copy link
Member Author

zepumph commented Apr 14, 2022

Alright. I believe that this is working now. I was able to utilize the current implementation of distanceResponseType being passed through on mouse input to make the both hands (ratio locked + indy hands + mouse) responses correct. @terracoda can you please review this and #459

@terracoda
Copy link
Contributor

terracoda commented Apr 15, 2022

Ok, I hear distance-progress and distance-region when using bi-manual keyboard input with Both Hands (i.e., Ratio Lock is not checked).

It seems to be connected to when entering and traversing the same distance-region, so this seems to be correct and the same as PDOM logic (I haven't directly compared).

We still need to work on the edge responses, though. I think that will be addressed in a separate issue - #450 and #457.

@terracoda
Copy link
Contributor

terracoda commented Apr 15, 2022

I think this is working well, except fr items in other issues, so closing.

Also, I am now not convinced that we need to implement item 2 in comment

I'll test and comment in the appropriate issue.

zepumph added a commit to phetsims/perennial that referenced this issue Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants