AccessibleValueHandler makes bad assumptions about range when setting keyboard steps

[pixelzoom]

In AccessibleValueHandler.ts:

keyboardStep: ( enabledRangeProperty.get().max - enabledRangeProperty.get().min ) / 20,
shiftKeyboardStep: ( enabledRangeProperty.get().max - enabledRangeProperty.get().min ) / 100,
pageKeyboardStep: ( enabledRangeProperty.get().max - enabledRangeProperty.get().min ) / 10,

This makes some bad assumptions about the range. If I have range [-3,0] for example (which I do in acid-base-solution), then these steps will be negative, and this assertion (and others?) will fail in AccessibleValueHandler.ts:

public setShiftKeyboardStep( shiftKeyboardStep: number ): void {
  assert && assert( shiftKeyboardStep >= 0, 'shift keyboard step must be non-negative' );

It might simply be a matter of putting Math.abs around these values. But I haven't had time to test.

In the meantime, I'll explicitly set these options to workaround this.

[pixelzoom]

You could also replace the 3 occurrences of

( enabledRangeProperty.get().max - enabledRangeProperty.get().min )

with

enabledRangeProperty.get().getLength()

... and that should address this issue.

Because for example, new phet.dot.Range( -3, 0 ).getLength() => 3.

[pixelzoom]

Hmmm... Now I'm confused about why the default steps are negative. Because:

> var r = new phet.dot.Range( -3, 0 )
> r.max - r.min
3

Perhaps there's a problem elsewhere?

[jessegreenberg]

I am trying to see this on my side but haven't been able to. I ran acid-base-solutions with ?supportsInteractiveDescription=true and didn't hit an assertion and I didn't see workarounds committed related to this. I tried this patch in the sun demo (setting the range of the slider from -3 to 0) and it worked OK.

```patch Index: js/demo/components/demoSlider.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/demo/components/demoSlider.ts b/js/demo/components/demoSlider.ts --- a/js/demo/components/demoSlider.ts (revision 21bd965d24a31fa4a878c3c4532e3eb58cda0408) +++ b/js/demo/components/demoSlider.ts (date 1671742081986) @@ -33,10 +33,10 @@ }, providedOptions ); const property = new Property( 0 ); - const range = new Range( 0, 100 ); + const range = new Range( -3, 0 ); const tickLabelOptions = { font: new Font( { size: 16 } ) }; - const enabledRangeProperty = new Property( new Range( 0, 100 ) ); + const enabledRangeProperty = new Property( new Range( -3, 0 ) ); let slider: Slider; if ( orientation === 'horizontal' ) { @@ -116,11 +116,11 @@ // restrict enabled range of slider const restrictedRangeProperty = new Property( false ); restrictedRangeProperty.link( restrictedRange => { - enabledRangeProperty.value = restrictedRange ? new Range( 25, 75 ) : new Range( 0, 100 ); + enabledRangeProperty.value = restrictedRange ? new Range( -2, 0 ) : new Range( -3, 0 ); } ); const enabledRangeCheckbox = new Checkbox( restrictedRangeProperty, - new Text( 'Enable Range [25, 75]', { font: CHECKBOX_FONT } ), { + new Text( 'Enable Range [-2, 0]', { font: CHECKBOX_FONT } ), { tandem: Tandem.OPT_OUT, left: slider.left, top: enabledCheckbox.bottom + 40 ```

It may not be from the option but using the setShiftKeyboardStep function later? Is there a reproducible example you can point me to?

[pixelzoom]

I ultimately punted on what I was trying to do in https://github.com/phetsims/acid-base-solutions/issues/167, so the case where I was experiencing this problem no longer exists. And I don't have time to invent a case to investigate. So I'm just going to close this issue. Thanks for having a look @jessegreenberg, and apologies for wasting your time.