phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

Rename and validate AccessibleValueHandler "step" options #702

Open pixelzoom opened 3 years ago

pixelzoom commented 3 years ago

AccessibleValueHandler option keyboardStep needs a new name and revised documentation. It is not specific to keyboard and works for other alternative-input devices.

Slack thread Chris Malley 1:50 PM Question about these Slider options: `keyboardStep`, `shiftKeyboardStep`, `pageKeyboardStep`. Are they keyboard-specific, or do they determine the step for other input devices? If the former, will they be ignored for other input devices, will other input devices need additional options, etc? Jesse Greenberg 1:55 PM Hmm... Chris Malley 1:55 PM What I’m getting at is that they options all have “keyboard” in their name. Are they really keyboard specific? Do you really want them to be keyboard specific? Jesse Greenberg 1:55 PM `shiftKeyboardStep` and `pageKeyboardStep` are specific to keyboard. `keyboardStep` however works as the step size for other alternative input devices. Chris Malley 1:56 PM Then `keyboardStep` needs a new name. Jesse Greenberg 1:56 PM Yea, I agree! Chris Malley 1:57 PM Nothing in AccessibleValueHandler.js that indicates that `keyboardStep` works for other input devices. In fact the doc makes it seem specific to keyboard: ```js // {number} - delta for the valueProperty for each press of the arrow keys keyboardStep: ( rangeProperty.get().max - rangeProperty.get().min ) / 20, ``` Jesse Greenberg 2:00 PM Indeed! These were probably added before we were thinking about other forms input and gesture controls.
pixelzoom commented 3 years ago

While we're at it.... Are there other classes that have similar "keyboard" options that should be renamed?

jessegreenberg commented 3 years ago

Was brainstorming different names:

pdomStep
alternativeInputStep
accessibleValueStep
inputStep
valueStep

I like accessibleValueStep because it is clearly specific to the trait. I could imagine collisions with inputStep and valueStep someday.

pixelzoom commented 3 years ago

Also need to validate that:

shiftKeyboardStep < keyboardStep < pageKeyboardStep

pixelzoom commented 3 years ago

@jessegreenberg said:

I like accessibleValueStep because it is clearly specific to the trait.

I agree -- it's specific to AccessibleValueHandler, so accessibleValueStep makes for a nice programming API. I realize that there were other suggestions in a Slack discussion, but I think that the name should be informed mainly by the programming API in this case.

zepumph commented 2 years ago

Subset of https://github.com/phetsims/scenery/issues/1298