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

Rename and validate AccessibleValueHandler "step" options #702

Open
pixelzoom opened this issue May 28, 2021 · 5 comments
Open

Rename and validate AccessibleValueHandler "step" options #702

pixelzoom opened this issue May 28, 2021 · 5 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented May 28, 2021

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:

          // {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
Copy link
Contributor Author

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

@jessegreenberg
Copy link
Contributor

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 pixelzoom changed the title Rename AccessibleValueHandler option keyboardStep Rename AccessibleValueHandler "step" options Jun 1, 2021
@pixelzoom
Copy link
Contributor Author

Also need to validate that:

shiftKeyboardStep < keyboardStep < pageKeyboardStep

@pixelzoom pixelzoom changed the title Rename AccessibleValueHandler "step" options Rename and validate AccessibleValueHandler "step" options Jun 9, 2021
@pixelzoom
Copy link
Contributor Author

@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
Copy link
Member

zepumph commented Oct 6, 2022

Subset of phetsims/scenery#1298

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

5 participants