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

Position slider information ready for implementation #124

Closed
terracoda opened this issue Dec 10, 2018 · 36 comments
Closed

Position slider information ready for implementation #124

terracoda opened this issue Dec 10, 2018 · 36 comments

Comments

@terracoda
Copy link
Contributor

@mbarlow12, @emily-phet and I discussed changes to how we could provide position information.

Please see the design document for details. An example is provided here:

https://docs.google.com/document/d/1-37qAgde2XrlXBQae2SgjartM35_EnzDD9pdtd3nXAM/edit#heading=h.ostp3ek0rn9e

I'll be adding more examples, but would love to know if this is clear.

mbarlow12 added a commit to phetsims/inverse-square-law-common that referenced this issue Dec 10, 2018
…ng content, implement functions to return text for subtype aria-valuetext, remove force vector information from position alert, see phetsims/gravity-force-lab#109 and phetsims/gravity-force-lab#124
@mbarlow12
Copy link
Contributor

@terracoda the aria-valuetext has been implemented and should be functioning correctly in both GFL & GFLB. Let me know if we should tweak the region boundaries. The aria-live text will be forthcoming.

@terracoda
Copy link
Contributor Author

@mbarlow12, this sounds great! It would be great to save this work for comparison with using "distance in meters".

@emily-phet and I discussed a new approach yesterday that still relies on aria-valuetext, but with different parameters. Using actual distance in meters.

Please see the updated focus examples and the updated 8-step interaction.

The strings for vector alerts and force magnitude are still the same, and should be nicely handled with aria live region alert

We can discuss at today's meeting.

@terracoda
Copy link
Contributor Author

@mbarlow12, oh, I missed that the object string containing mm1 or m2 is missing from the implementation. For example,

  • closer to m2
  • farther from m2
  • closer to m2

We'll need that string in there, too.

@terracoda
Copy link
Contributor Author

@mbarlow12, that string is actually in there sometimes. We'll need it in there for directional changes with the new strategy using quantitative distance in meters instead of described qualitative distance.

mbarlow12 added a commit to phetsims/inverse-square-law-common that referenced this issue Dec 12, 2018
mbarlow12 added a commit to phetsims/gravity-force-lab-basics that referenced this issue Dec 12, 2018
mbarlow12 added a commit to phetsims/inverse-square-law-common that referenced this issue Dec 12, 2018
@terracoda
Copy link
Contributor Author

terracoda commented Dec 12, 2018

This is sounding good @mbarlow12. Please assign to me when you want me to review it :-)

@mbarlow12
Copy link
Contributor

@terracoda there's an issue that I've realized comes from the aria-valuetext implementation. If the value in the slider doesn't change (e.g. sphere is at either end of the range), screen readers will not repeatedly read the value if the user continues attempting to move in the same direction (confirmed this same behavior in JT as well).

Here's an example sequence:

  • user moves a sphere all the way to the other sphere
  • when they first arrive at the sphere, screen reader reads the 'last stop...' text from aria-valuetext
  • user tries moving in the same direction
    • NOTE: this doesn't change the aria-valuetext
  • nothing read from screen reader
  • aria-live still read out (e.g. "Vectors {{medium size}}{{, forces {{0.463461}} micronewtons}}.")

Just want to ensure that this behavior is ok and we don't need to repeatedly communicate the "last stop..." information.

@terracoda
Copy link
Contributor Author

@mbarlow12, I think we can leave as-is, but it is possible that having the alert change on the step that actually reaches the last stop could be potentially useful in this case.

mbarlow12 added a commit to phetsims/gravity-force-lab-basics that referenced this issue Dec 13, 2018
mbarlow12 added a commit that referenced this issue Dec 13, 2018
mbarlow12 added a commit to phetsims/inverse-square-law-common that referenced this issue Dec 13, 2018
@mbarlow12
Copy link
Contributor

New dev versions for GFL:
https://phet-dev.colorado.edu/html/gravity-force-lab/2.2.0-dev.10/phet/gravity-force-lab_en_phet.html?a11y
https://phet-dev.colorado.edu/html/gravity-force-lab/2.2.0-dev.10/phet/gravity-force-lab_a11y_view.html

Note: I somehow missed adding 'kilograms' to the interactive alerts for changing mass. Will be fixed in the next update.

@mbarlow12
Copy link
Contributor

Mass alerts are a little messy in general—on my list to fix.

@terracoda terracoda assigned terracoda and unassigned mbarlow12 Jan 17, 2019
@terracoda
Copy link
Contributor Author

Re-assigned to myself to review.

@terracoda
Copy link
Contributor Author

@zepumph, I have done a very careful review of all position/distance aria-valuetext (and alerts).

Much of the implementation is working, but of course I found some issues and have some questions.

I have updated the tables in the design document (green cells mark things working well and red cells and red text identify issues that I found & iterations that I would like to see if possible).

I am marking this for discussion for tomorrow's meeting before I create issues.

Here's a link to the Position/distance section in the design doc

zepumph added a commit to phetsims/gravity-force-lab-basics that referenced this issue Feb 12, 2019
zepumph added a commit that referenced this issue Feb 12, 2019
@zepumph
Copy link
Member

zepumph commented Feb 12, 2019

The above commits should update the sim according to this line in the REGULAR design doc table:
image

UPDATE: Actually I think the above commits apply to sections 3-5 in that table.

@zepumph
Copy link
Member

zepumph commented Feb 12, 2019

From here I feel like I need to focus on BASICS for interview. Though much of this may apply over there. I'm going to look at phetsims/gravity-force-lab-basics#88 for now.

@terracoda
Copy link
Contributor Author

terracoda commented Feb 13, 2019

@zepumph, you have done a great job implementing the aria-valuetext and ARIA live alerts for position and distance. I tested everything and there just a few small issues. The issues are highlighted in red in right-hand column of the tables in the design doc, but I will list them here for you.
I don't think any would significantly impact being ready for interviews next week unless the mistake was copied over to BASICS, which I will be testing later.

Adding: The first 3 items are also in Gravity Force Lab BASICS.

  • In Focus Table - aria-valuetext is working in all focus scenarios, except in the single case where m1 is touching m2 and m2 is at the right edge and has focus. In this single case, m2’s landmark is incorrectly showing as "left edge" when it should be "right edge". All other edge cases seem to be correct. Probably just an incorrect string somewhere.

  • See Step 8 in changing position table - On the last step when a sphere reaches the other sphere, we provide "state information". I still want the word “now” before the force value because the force actually changed on that last step. For the next step, Step 9, there is no change in position and no change in force value (thus no change in arai-valuetext), so in that case the word "now" is not included. If you could make it, so we always hear "now" when there is a force change, that would be great.

  • n/a Step 17 in changing position table - On the last step when the sphere reaches an edge, I would like the same "state-information" alert as in Step 8. When I tested yesterday, I got the "changing force" alert on the last step when reaching an edge.

  • Punctuation clean-up - also in Focus Table - Remove period at end of distance string in aria-valuetext. VoiceOver puts in a comma automatically. It has taken me a long time to understand this for VoiceOver, and it might vary between AT. That said, I think it is safe to remove the period at the end of the distance string in the aria-valuetext. All examples in the table were updated.

There are 2 still-todo's that do not affect GFL Basics:**

  • using "Force vectors" instead of "Vectors" alone when Force values are not checked, and
  • an issue about reading out the times sign when scientific notation is checked.

I can move the still-todo's to new issues, so they do not fall off the radar. They are not high priority at the moment as they do not affect anything in GFL Basics.

Edited May 28 - Step 17 no longer relevant.
Edited Feb 14 - Added Step 17 issue to check list.

@terracoda terracoda removed their assignment Feb 13, 2019
@terracoda
Copy link
Contributor Author

@zepumph, you only need to address the above checklist if they have a direct impact on GFL Basics.

@terracoda
Copy link
Contributor Author

terracoda commented Feb 13, 2019

@zepumph, In Step 8, I swear I was hearing the correct alert (below) but minus the word "now" when I tested this morning.

  • Vectors {{medium size}}{{, forces now {{0.463461}} micronewtons}}.

I am now hearing the regular changing force alert on the last step, which does indeed contain "now", but on the last step (edge or last stop) I would like alert above. This special case alert appears in Step 8 and Step 17 in the design doc.

The same issue exists in GFL Basics.

@terracoda
Copy link
Contributor Author

@zepumph, I see, I am hearing two different alerts. The issue is is that Step 8 has the correct alert with "now" missing, and Step 17 has the wrong alert.

I would like the special "state information" alert in Step 17 as well. I would like this special case alert at all last steps, the ones that reach an edge and the ones that reach the opposite sphere. Apologies if that was not clear.

@terracoda
Copy link
Contributor Author

terracoda commented Feb 14, 2019

@zepumph I added the Step 17 issue into the check list, and I added that the first 3 items are also in the BASICS sim.

Ideally, if you could address the first three items, that would be great. The first 2 items are the most important if you are short on time.

@terracoda
Copy link
Contributor Author

@zepumph, I have verified both sims, just tagging BASICS' related issue phetsims/gravity-force-lab-basics#88

zepumph added a commit to phetsims/inverse-square-law-common that referenced this issue Feb 19, 2019
zepumph added a commit to phetsims/gravity-force-lab-basics that referenced this issue Feb 19, 2019
@zepumph
Copy link
Member

zepumph commented Feb 19, 2019

@terracoda I think I got the first two bullets. Please review those, and I can work on them more tomorrow.

@terracoda
Copy link
Contributor Author

The first two checkboxes are verified. Great work!
That's the critical stuff on position. I'll just go make sure it works in BASICS, too.

@terracoda
Copy link
Contributor Author

@zepumph, yes the first 2 checkboxes are verified in BASICS as well.

@terracoda
Copy link
Contributor Author

I am pretty sure all the issues about position will be address by GFLB's issue phetsims/gravity-force-lab-basics#124

Removing the priority label.

@terracoda
Copy link
Contributor Author

After discussing Step 17 checkbox, I changed my mind. We are not providing state information at last step, only at going beyond an edge. We may consider this further, but for now. It's not an issue. I've updated the #124 (comment).

Punctuation looks good.
Closing this issue.

Stale distance information should be handled by phetsims/gravity-force-lab-basics#128

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

3 participants