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

Investigate best min and max values for mass sliders #181

Closed
terracoda opened this issue Oct 1, 2019 · 11 comments
Closed

Investigate best min and max values for mass sliders #181

terracoda opened this issue Oct 1, 2019 · 11 comments

Comments

@terracoda
Copy link

From issue phetsims/qa#429 (comment)

JAWS screen reader reads out the min and max values for the mass sliders which sounds like 1 zero zero zero zero...

min="1000000000" max="10000000000" step="1000000000"

VoiceOver only seems to read out the aria-valuetext of the current value which sounds like "4 billion kilograms"

Is it possible to use simplified values instead?

min="1" max="10" step="1"

Of course the aria-valuenow attribute would also need to change to a single digit.

@zepumph, is this possible without messing up the numbers needed for the actual model?

@terracoda terracoda changed the title Investigate best min and max values fro mass sliders Investigate best min and max values for mass sliders Oct 1, 2019
@zepumph
Copy link
Member

zepumph commented Oct 1, 2019

This thought adds another layer of complexity to AccessibleValueHandler. Currently here are the supported options that involve mapping a value to something in the PDOM:

  • a11yValuePattern: // {string} if you want units or additional content, add to pattern
  • a11yDecimalPlaces: // number of decimal places for the value when formatted and read by assistive technology
  • roundToStepSize: // {boolean} - Whether or not to round the value to a multiple of the keyboardStep.
  • a11yMapValue: // Map the valueProperty value to another number that will be read by the assistive device on valueProperty changes. {function(number):number}
  • a11yRepeatEqualValueText: // If true, the aria-valuetext will be spoken every value change, even if the aria-valuetext doesn't actually change.
  • a11yCreateValueChangeAriaValueText: // Custom aria-valuetext creation function, called when the valueProperty changes.
  • a11yCreateValueChangeAlert: // Create content for an alert that will be sent to the utteranceQueue when the user interacts with the input.

I wanted to give an overview of current options in case we could try to co-opt some into something that could be supported for this case. I don't want to just add something else on top.

That said, it seems like what we need here is a modelViewTransform type thing, that maps from values in the PDOM slider to actual model values. Currently, AccessibleValueHandler, is taking the min/max and value directly from the valueProperty and rangeProperty. By "directly" I mean that the value passes through "a11yMapValue" but the min/max currently aren't. Perhaps that is the function we need.

@terracoda
Copy link
Author

@zepumph, I think you are correct, we need something that says:

  • 1 in the PDOM mass slider == 1 000 000 000 in the sim's model

In the options above, my first guess would be the a11yMapValue option, just as you have pointed out.

Map the valueProperty value to another number that will be read by the assistive device on valueProperty changes. {function(number):number}

So would that translate to:

  • pdom number = sim number/1 000 000 000

@zepumph
Copy link
Member

zepumph commented Oct 2, 2019

Over in phetsims/sun#530 we worked on a11yMapValue, and were able to get things working for this issue. This is now what the input looks like:

image

There is some concern that to get the step like this, we used a pretty sim-specific implementation, but I think that will be sorted out as part of phetsims/sun#530 (comment)

@terracoda can you please review master before we merge changes over to the rc branch?

@terracoda
Copy link
Author

I hadn't actually experienced this read out (recently anyways) until today before I emptied my caches:
Screen Shot 2019-10-02 at 2 04 32 PM

Happily, after I emptied my caches and reloaded from master, I got the beautiful aria-valuetext and no more 2000000000:
Screen Shot 2019-10-02 at 2 08 12 PM

Nice work! I think you can move to RC branch! Assigning back to you @zepumph!

@terracoda
Copy link
Author

Safe to close, @zepumph?

@zepumph
Copy link
Member

zepumph commented Oct 3, 2019

Yes I think there is more work to be done, but that will be over in phetsims/sun#530.

@terracoda
Copy link
Author

Ok, closing ;-)

@zepumph zepumph reopened this Oct 7, 2019
@zepumph
Copy link
Member

zepumph commented Oct 7, 2019

Sorry I'm a liar. We should keep this open for QA to ensure the fix makes it successfully into the release branch.

@zepumph
Copy link
Member

zepumph commented Oct 7, 2019

I will commit the following commits to branches over in #179, in order:

From phetsims/sun#530:
ISLC:
phetsims/inverse-square-law-common@1d37094
phetsims/inverse-square-law-common@b2132e7

GFLB:
a95d679

GFL:
phetsims/gravity-force-lab@01b59ad

SUN: EDIT: to get these to work I actually needed all commits from Commits on Sep 17, 2019
to Commits on Oct 1, 2019, which I got from the github tree.
phetsims/sun@f036536
phetsims/sun@b365934

phetsims/sun@f65e4de
phetsims/sun@3667e40
phetsims/sun@4e588f4
phetsims/sun@7006f9f
phetsims/sun@40ab4e6

Then these:
SUN: phetsims/sun@3667e40
GFLB: 477075b

@zepumph zepumph mentioned this issue Oct 7, 2019
7 tasks
@zepumph
Copy link
Member

zepumph commented Oct 14, 2019

QA: to tests, the bug is described in #181 (comment), basically there shouldn't be many zeros read out for the min/max values. I think this was only present for JAWS.

@loganbraywork
Copy link

Issue fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants