[Keyboard Nav] Change default step size for position sliders to 0.2 km

[arouinfar]

For phetsims/qa#199

@mbarlow12 for GFL, @terracoda suggested a default step size for the sphere position sliders to be 0.2 m, with the shift increment being 0.1 m. I quite like that suggestion, and think it should also apply to GFLB, with the default step size being 0.2 km.

Additionally, a step of 0.2 km would correspond to the tick marks on the ruler, and would still feel pretty smooth when using the keyboard.

[mbarlow12]

@arouinfar default keyboard step size updated. May require some tweaks to GFL & CL (but they appear ok at the moment). Feel free to review and close.

[arouinfar]

@mbarlow12 the step size can apply to all ISLC sims. Generally speaking, the shift increment should move to the adjacent gridline, while arrows would skip over a gridline.

I tested master after b5209a0, but I'm still noticing a few strange issues.

Case A

If the distance between the masses is odd (because the shift modifier was used), the arrow key only steps 0.1 km.

1. On startup, focus on mass 1, press shift+right arrow. Mass 1 moves 0.1 km, reducing the distance to 3.9 km. This is the correct behavior.
2. Arrow to the right or left (without the shift modifier), and the mass only moves 0.1 km.
3. Shift+Right arrow twice to move an even distance.
4. Arrow to the left or right, and the mass will move 0.2 km.

Case B

This could be the same as Case A, but since it deals with the minimum separation, I thought I should document it separately.

1. On startup, focus on mass 1 and tap the right arrow key until the distance between the masses is 1.8 km
   

2. The minimum distance between these masses is 1.7 km, so pressing the right arrow again will only move the mass 0.1 km (this is to be expected).
   

3a. Press the left arrow key once. Mass 1 only moves 0.1 km. I would have expected it to move 0.2 km to increase the distance to 1.9 km.

3b. Alternatively, after step 2, tab to mass 2, then hit the right arrow key once. Mass 2 moves 0.2 km, increasing the separation to 1.9 km.

[terracoda]

If I am understanding this right, I think the default step size (no shift key) should always be 0.2 km unless 0.1 kilometers from an edge in which case the next step will go 0.1 km as that is all that is possible in that step.

And when using the shift key, the step is always 0.1 km.

[arouinfar]

That's correct @terracoda. Currently, the shift modifier is working exactly as it should.

If the mass is at an edge (approaching edge of screen or the other mass), then it's acceptable for the arrow key to move 0.1 km instead of 0.2 km. However, there are just a few strange cases where an arrow key only moves 0.1 km, even though there is room for the mass to move 0.2 km.

[mbarlow12]

This is an artifact of the way we implement accessible sliders. Note that in Ohm's law, if you increment with shift from 4.5V to 4.4V then press down again, you jump to 4.0V not 3.9V. This is effectively the same behavior.

@jessegreenberg would you like to pair on this later today or discuss at our a11y meeting tomorrow? One option might be to make a more generic unconstrained slider type that doesn't restrict the values resulting from keypresses. Another options would be to not use sliders to represent the masses.

[arouinfar]

Thanks for the clarification @mbarlow12.

It might be worthwhile to have a general conversation about accessible sliders, because the behavior in #72 (comment) seems a bit odd to me.

That said, I don't think it's particularly harmful, so I don't think it's worth representing the masses as some other element.

[jessegreenberg]

@mbarlow12 I agree with #72 (comment), this is the behavior for accessible sliders right now. The step size is not only the delta for the value, but the value will always be rounded to the nearest step size on change.

Maybe roundValue could just be optional, set to false here? Maybe that option should default to false, because that was really only needed in RIAW and OL to easily reach pedagogically useful values.

[terracoda]

@jessegreenberg, I like the idea of the roundValue being optional because it is really necessary in sims like RIAW for making easy calculations, but in a sims like GFL the rounding is not needed.

There are likely many sims in both categories.

It is quite interesting as we do each new sim we see the different kinds of options that we might need and which ones are generalisable.

[mbarlow12]

@jessegreenberg I actually think this is pretty straightforward to update, but I'd want to to double check the desired behavior for the common code elements. Right now the direct usages are in: FaucetNode, NumberControl, SpectrumSlider, and Slider.

Aside from that, it's only used in john-travoltage, area-model-common, and inverse-square-law-common.

[jessegreenberg]

Sounds great! I almost think that the default should be false. Normal sliders that will have shift + non shift steps probably wouldn't want rounding for the same reasons as this sim. Then we would just need to have roundValue: true in OL and RIAW SliderUnits. Thoughts?

[mbarlow12]

I had assumed that since the existing implementations are using the rounding, they should all be passed true. Definitely not sure for FaucetNode and Area Model Common.

Some questions that might help determine when rounding is necessary:

1. Are the specific values pedagogically significant?
2. Is the slider essentially a freely moving object with restricted movement (e.g. JT & CL/GFL:B)?
3. Is/are there any mapping(s) between model/view/a11y properties?

[jessegreenberg]

Discussed during an a11y dev meeting, we agree that the default should be false, but existing implementations should remain as they are to avoid disruption.

[mbarlow12]

@jessegreenberg I've added the option in the above updates. All existing uses of AccessibleSlider are setting roundToStepSize to true to maintain the current functionality. Would you be willing to review?

[arouinfar]

I believe this is the only remaining issue before GFLB moves into RC. @jessegreenberg do you think this is something you'll be able to review?

[jessegreenberg]

Sure thing, Ill take a look now.

[jessegreenberg]

Commits looks great, the only thing I did was move the roundToStepSize option to the options block for things in scenery-phet. I am still not sure whether this should be enabled by default for things like FaucetNode and SpectrumSlider, I could imagine those sliders having issues like this one. But we can discuss that at meeting today, and I don't think there is anything left to do for gravity-force-lab-basics. @mbarlow12 assigning back to you.

[mbarlow12]

Thanks @jessegreenberg. I think the commit was referenced in a different issue; linking here for record keeping: phetsims/scenery-phet@6ab271b.

I'll review the dev test issues and release a new dev version today.

[jessegreenberg]

Discussed in a11y meeting today. Ill go ahead and remove the snapping from all things in scenery-phet (everything else mixing in AccessibleSlider is sim specific, and should be investigated on a case by case basis).

[mbarlow12]

New dev version to confirm the issues are fixed:

https://phet-dev.colorado.edu/html/gravity-force-lab-basics/1.0.0-dev.28/phet/gravity-force-lab-basics_en_phet.html?ea&a11y

[arouinfar]

Looks great!