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

Ruler Movement Repsonses ready for implementation #215

Closed
terracoda opened this issue Nov 28, 2019 · 8 comments
Closed

Ruler Movement Repsonses ready for implementation #215

terracoda opened this issue Nov 28, 2019 · 8 comments

Comments

@terracoda
Copy link
Contributor

terracoda commented Nov 28, 2019

@zepumph, I made new tables to lay out all the common strings and to provide examples of all the context responses need for grabbing, moving and releasing.

I think things are much more general now and I have provided some fuzzy pseudo logic to help with implementation. Please see this section and these tables in the design document

Context Responses for Measure Distance Ruler (done Nov 28)

I hope you like it and I hope it is logical!
These are the goals I am going for:

  • generalize the hints.
    • I tried to generalize everything but the first initial grab.
    • Initial grabs for both keyboard and gesture have custom content to provide device specific instructions.
    • Except for responses to J+C and J+H, I think I generalized move responses so they are the same for both gesture and keyboard.
    • I provide some pseudo-logic for when the responses should fire.
@terracoda
Copy link
Contributor Author

If you have questions just change the label to meeting, and we can discuss in our next meeting.

@terracoda terracoda changed the title Movement Repsonses ready for implementation Ruler Movement Repsonses ready for implementation Dec 9, 2019
zepumph added a commit to phetsims/inverse-square-law-common that referenced this issue Dec 31, 2019
zepumph added a commit to phetsims/inverse-square-law-common that referenced this issue Jan 1, 2020
zepumph added a commit to phetsims/utterance-queue that referenced this issue Jan 1, 2020
zepumph added a commit that referenced this issue Jan 1, 2020
zepumph added a commit to phetsims/inverse-square-law-common that referenced this issue Jan 1, 2020
@zepumph
Copy link
Member

zepumph commented Jan 1, 2020

I think that everything in the google doc has been implemented for the ruler. I have not tested gesture at all, but keyboard alerts were sounding good. @terracoda please review.

@terracoda
Copy link
Contributor Author

@zepumph, I noticed the old hint for moving left and right...

  • "Move ruler left and right with letter keys A and D."

Please remove that one. I decided to drop it as it was too Keyboard centric.

  • It appeared on regrab after a successful J+C and release that left the ruler "just below centers"

Screen Shot 2019-12-31 at 9 55 47 PM

@terracoda
Copy link
Contributor Author

Starting a new issue for the above comment.

@terracoda
Copy link
Contributor Author

terracoda commented Jan 1, 2020

I found 4 other things which I will make issues for:

  • 1.Not hearing "Released." when using J+H. I think this is a timing issue as I see it in the A11y View.
  • 2.After a reset with the Reset All button, I get a vertical region change early - on the first step which triggers an early region change combinedReleaseAndExploreHint on the second step. This only seems to happen after a reset.
  • 3.Found an "s" on meters when centers were 1 meter apart. Can you add logic to remove the "s" in that case?
  • 4.I'm thinking, "Centers {{4}} meters apart." sounds a bit strange. Considering returning to "Centers of spheres {{4}} meters apart." I'll ask Amy if she has a preference.

@terracoda
Copy link
Contributor Author

terracoda commented Jan 2, 2020

@zepumph, I reviewed everything you implemented, and it all seems to be working nicely for the keyboard experience (except noted issues (minor)).

Please post a dev version when it is ready to test on mobile. Assigning back to you and removing ready to review label.

Nice work!

@zepumph
Copy link
Member

zepumph commented Jan 7, 2020

Alright, I have noted all of the separate issues created in response to this review. Thank you so much for having them separate; it made my work today much easier and I appreciate it. Is there anything else for this issue?

@terracoda
Copy link
Contributor Author

Everything in this issue is good. Remaining bits and pieces (if any) can be followed in other issues.

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