Some keyboard shortcuts don't work

[Nancy-Salpepi]

Test device MacBook Air M1 chip

Operating System 13.5.2

Browser Safari 16.6

Problem description For https://github.com/phetsims/qa/issues/985, the "A" and "D" keys do not move the soccer balls or cards.

Visuals

[marlitas]

@catherinecarter do we want to include A and D as ways to move a grabbed ball or card, or would you like us to remove that shortcut from the keyboard dialog?

[catherinecarter]

I believe A and D are standard keys to move a grabbed object, so yes, that would be great if we could put those in.

I did notice two other things in the keyboard shortcut:

1. the up/down arrow keys don't move the ball left and right:

   

   Let's remove the up/down arrows from the keyboard dialog help box.

2. Also curious how challenging/time consuming it would be to add the page up/down and move in larger steps (shift right/left) to the soccer ball group and cards for the selection of which ball/card to move (prior to pressing space or enter to grab)? I'm finding myself wanting to choose which ball to move by skipping to the ends of the number line, but I currently cannot.

[marlitas]

Okay perfect. I can add A and D for both the balls and the cards.

Let's remove the up/down arrows from the keyboard dialog help box.

• Great

Also curious how challenging/time consuming it would be to add the page up/down and move in larger steps (shift right/left) to the soccer ball group and cards for the selection of which ball/card to move (prior to pressing space or enter to grab)

• I don't think this would be too challenging. I can try it out and if things start to get sticky let you know.

[marlitas]

Let's remove the up/down arrows from the keyboard dialog help box.

Okay I didn't realize we were using a commonCode element for the Basic Actions section. I would actually just recommend adding the up and down arrows to how you can select different balls or cards in a group since this is also referring to radioButtonGroups. @catherinecarter thoughts?

[marlitas]

Hmmm... but now I'm also noticing that we would need to add pageUp and pageDown to the keyboard help dialog for Basic Actions... which wouldn't work for radioButtonGroup. I think we need to chat about this before I keep moving forward.

Here's the patch for implementing pageUp, pageDown, home, end into selecting a different ball.

```diff Subject: [PATCH] Add pageup/down and home/end keyboard functionality --- Index: js/view/SoccerSceneView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/view/SoccerSceneView.ts b/js/view/SoccerSceneView.ts --- a/js/view/SoccerSceneView.ts (revision 9f7d68577bcd8efba85522150a07a73488ac0439) +++ b/js/view/SoccerSceneView.ts (date 1696025387349) @@ -229,6 +229,11 @@ } ); + const setFocusedSoccerBall = ( nextIndex: number, topBallNodes: SoccerBallNode[], numberOfTopSoccerBalls: number ) => { + const clampedIndex = Utils.clamp( nextIndex, 0, numberOfTopSoccerBalls - 1 ); + focusedSoccerBallProperty.value = topBallNodes[ clampedIndex ].soccerBall; + }; + const keyboardListener = new KeyboardListener( { fireOnHold: true, keys: [ 'd', 'arrowRight', 'a', 'arrowLeft', 'enter', 'space', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', 'home', 'end', 'escape', 'pageUp', 'pageDown' ], @@ -237,20 +242,12 @@ // Select a soccer ball if ( focusedSoccerBallProperty.value !== null ) { - if ( ( keysPressed === 'arrowRight' || keysPressed === 'arrowLeft' || keysPressed === 'a' || keysPressed === 'd' ) ) { - if ( ( keysPressed === 'arrowRight' || keysPressed === 'arrowLeft' ) && !isSoccerBallKeyboardGrabbedProperty.value ) { - soccerModel.hasKeyboardSelectedDifferentBallProperty.value = true; - - const delta = keysPressed === 'arrowRight' ? 1 : -1; - const numberOfTopSoccerBalls = sceneModel.getTopSoccerBalls().length; - - // We are deciding not to wrap the value around the ends of the range because the grabbed soccer ball - // also does not wrap. - const currentIndex = topBallNodes.indexOf( soccerBallMap.get( focusedSoccerBallProperty.value )! ); - const nextIndex = Utils.clamp( currentIndex + delta, 0, numberOfTopSoccerBalls - 1 ); - focusedSoccerBallProperty.value = topBallNodes[ nextIndex ].soccerBall; - } - else if ( isSoccerBallKeyboardGrabbedProperty.value ) { + if ( [ 'enter', 'space' ].includes( keysPressed ) ) { + isSoccerBallKeyboardGrabbedProperty.value = !isSoccerBallKeyboardGrabbedProperty.value; + hasKeyboardGrabbedBallProperty.value = true; + } + else if ( isSoccerBallKeyboardGrabbedProperty.value ) { + if ( [ 'arrowRight', 'arrowLeft', 'a', 'd' ].includes( keysPressed ) ) { soccerModel.hasKeyboardMovedBallProperty.value = true; const delta = keysPressed === 'arrowLeft' || keysPressed === 'a' ? -1 : 1; @@ -258,14 +255,7 @@ soccerBall.valueProperty.value = physicalRange.constrainValue( soccerBall.valueProperty.value! + delta ); soccerBall.toneEmitter.emit( soccerBall.valueProperty.value ); } - } - else if ( keysPressed === 'enter' || keysPressed === 'space' ) { - isSoccerBallKeyboardGrabbedProperty.value = !isSoccerBallKeyboardGrabbedProperty.value; - hasKeyboardGrabbedBallProperty.value = true; - } - else if ( isSoccerBallKeyboardGrabbedProperty.value ) { - - if ( keysPressed === 'escape' ) { + else if ( keysPressed === 'escape' ) { isSoccerBallKeyboardGrabbedProperty.value = false; } else { @@ -290,6 +280,29 @@ } } } + else if ( !isSoccerBallKeyboardGrabbedProperty.value ) { + const currentIndex = topBallNodes.indexOf( soccerBallMap.get( focusedSoccerBallProperty.value )! ); + const numberOfTopSoccerBalls = sceneModel.getTopSoccerBalls().length; + + if ( [ 'arrowRight', 'arrowLeft' ].includes( keysPressed ) ) { + soccerModel.hasKeyboardSelectedDifferentBallProperty.value = true; + + const delta = keysPressed === 'arrowRight' ? 1 : -1; + + // We are deciding not to wrap the value around the ends of the range because the grabbed soccer ball + // also does not wrap. + const nextIndex = currentIndex + delta; + setFocusedSoccerBall( nextIndex, topBallNodes, numberOfTopSoccerBalls ); + } + else if ( [ 'home', 'end', 'pageDown', 'pageUp' ].includes( keysPressed ) ) { + const nextIndex = keysPressed === 'home' ? physicalRange.min : + keysPressed === 'end' ? physicalRange.max : + keysPressed === 'pageDown' ? currentIndex - 3 : + currentIndex + 3; + setFocusedSoccerBall( nextIndex, topBallNodes, numberOfTopSoccerBalls ); + } + + } // When using keyboard input, make sure that the "focused" ball is still displayed by panning to keep it // in view. `panToCenter` is false because centering the ball in the screen is too much movement. ```

[marlitas]

We chatted about this today and decided:

• [x] We will not add pageUp/Down or home/end to how a user can select a different ball or card in a group
• [x] We will add functionality that users can both select and move cards/balls with the up and down arrow keys. Additionally, users can move a card/ball with w and s.
• [x] Change the "pointer, interval tool" keyboard help info to match sliders, however keep the "move" language currently used.

[marlitas]

I updated the keyboard help dialogs.

I did notice a discrepancy between the headings for the prediction tools on the variability and Median/Mean & Median screens.

The variability screen heading starts with 'move' but the other screens do not.

Over to @catherinecarter

[marlitas]

I noticed that the escape key is not included in the Grab or Release Ball or Card section, even though that functionality exists. Do we want to add a row to include the escape key, or are we okay with not mentioning it?

I also wanted to show a couple different ways to format the moved grabbed card or ball row since it's now much longer with the all the added keys:

Example 1:

Example 2:

Example 3:

```diff Subject: [PATCH] Move to second column --- Index: center-and-variability/js/common/view/CAVKeyboardHelpNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/common/view/CAVKeyboardHelpNode.ts b/center-and-variability/js/common/view/CAVKeyboardHelpNode.ts --- a/center-and-variability/js/common/view/CAVKeyboardHelpNode.ts (revision 698de1a7c7ec748e6c055de38f5332504ffb79a1) +++ b/center-and-variability/js/common/view/CAVKeyboardHelpNode.ts (date 1696273432229) @@ -15,9 +15,9 @@ export const SECTION_LABEL_OPTIONS = { labelOptions: { lineWrap: 200 } }; export default class CAVKeyboardHelpNode extends TwoColumnKeyboardHelpContent { - public constructor( leftContent: KeyboardHelpSection[] ) { + public constructor( leftContent: KeyboardHelpSection[], sliderContent: KeyboardHelpSection ) { KeyboardHelpSection.alignHelpSectionIcons( leftContent ); - super( leftContent, [ new BasicActionsKeyboardHelpSection( { + super( leftContent, [ sliderContent, new BasicActionsKeyboardHelpSection( { withCheckboxContent: true } ) ] ); } Index: center-and-variability/js/median/view/MedianKeyboardHelpNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/view/MedianKeyboardHelpNode.ts b/center-and-variability/js/median/view/MedianKeyboardHelpNode.ts --- a/center-and-variability/js/median/view/MedianKeyboardHelpNode.ts (revision 698de1a7c7ec748e6c055de38f5332504ffb79a1) +++ b/center-and-variability/js/median/view/MedianKeyboardHelpNode.ts (date 1696273432221) @@ -28,9 +28,8 @@ CenterAndVariabilityStrings.keyboardHelpDialog.medianScreen.moveGrabbedBallOrCardStringProperty, CenterAndVariabilityStrings.keyboardHelpDialog.medianScreen.jumpToStartOfCardsOrNumberLineStringProperty, CenterAndVariabilityStrings.keyboardHelpDialog.medianScreen.jumpToEndOfCardsOrNumberLineStringProperty - ), - new MedianKeyboardHelpPredictMedianSection() - ] ); + ) + ], new MedianKeyboardHelpPredictMedianSection() ); } } Index: center-and-variability/js/common/view/CAVKeyboardHelpMoveGrabbedBallAndOrCardSection.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/common/view/CAVKeyboardHelpMoveGrabbedBallAndOrCardSection.ts b/center-and-variability/js/common/view/CAVKeyboardHelpMoveGrabbedBallAndOrCardSection.ts --- a/center-and-variability/js/common/view/CAVKeyboardHelpMoveGrabbedBallAndOrCardSection.ts (revision 698de1a7c7ec748e6c055de38f5332504ffb79a1) +++ b/center-and-variability/js/common/view/CAVKeyboardHelpMoveGrabbedBallAndOrCardSection.ts (date 1696273432224) @@ -26,14 +26,12 @@ jumpEndMessage: LocalizedStringProperty ) { super( title, [ - KeyboardHelpSectionRow.labelWithIcon( moveMessage, - KeyboardHelpIconFactory.iconOrIcon( - KeyboardHelpIconFactory.iconOrIcon( - KeyboardHelpIconFactory.leftRightArrowKeysRowIcon(), - KeyboardHelpIconFactory.iconRow( [ LetterKeyNode.a(), LetterKeyNode.d() ], { spacing: 1.3 } ) ), - KeyboardHelpIconFactory.iconOrIcon( - KeyboardHelpIconFactory.upDownArrowKeysRowIcon(), - KeyboardHelpIconFactory.iconRow( [ LetterKeyNode.w(), LetterKeyNode.s() ] ), { spacing: 1.3 } ) ) ), + KeyboardHelpSectionRow.labelWithIconList( moveMessage, [ + KeyboardHelpIconFactory.leftRightArrowKeysRowIcon(), + KeyboardHelpIconFactory.iconRow( [ LetterKeyNode.a(), LetterKeyNode.d() ], { spacing: 1.3 } ), + KeyboardHelpIconFactory.upDownArrowKeysRowIcon(), + KeyboardHelpIconFactory.iconRow( [ LetterKeyNode.w(), LetterKeyNode.s() ], { spacing: 1.3 } ) + ] ), KeyboardHelpSectionRow.labelWithIcon( CenterAndVariabilityStrings.keyboardHelpDialog.moveInLargerStepsStringProperty, KeyboardHelpIconFactory.pageUpPageDownRowIcon() ), KeyboardHelpSectionRow.labelWithIcon( jumpStartMessage, TextKeyNode.home(), SECTION_LABEL_OPTIONS ), KeyboardHelpSectionRow.labelWithIcon( jumpEndMessage, TextKeyNode.end(), SECTION_LABEL_OPTIONS ), Index: center-and-variability/js/mean-and-median/view/MeanAndMedianKeyboardHelpNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/mean-and-median/view/MeanAndMedianKeyboardHelpNode.ts b/center-and-variability/js/mean-and-median/view/MeanAndMedianKeyboardHelpNode.ts --- a/center-and-variability/js/mean-and-median/view/MeanAndMedianKeyboardHelpNode.ts (revision 698de1a7c7ec748e6c055de38f5332504ffb79a1) +++ b/center-and-variability/js/mean-and-median/view/MeanAndMedianKeyboardHelpNode.ts (date 1696273432214) @@ -25,9 +25,8 @@ CenterAndVariabilityStrings.keyboardHelpDialog.moveGrabbedBallStringProperty, CenterAndVariabilityStrings.keyboardHelpDialog.jumpToStartOfNumberLineStringProperty, CenterAndVariabilityStrings.keyboardHelpDialog.jumpToEndOfNumberLineStringProperty - ), - new MeanAndMedianKeyboardHelpPredictSection() - ] ); + ) + ], new MeanAndMedianKeyboardHelpPredictSection() ); } } Index: center-and-variability/js/variability/view/VariabilityKeyboardHelpNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/variability/view/VariabilityKeyboardHelpNode.ts b/center-and-variability/js/variability/view/VariabilityKeyboardHelpNode.ts --- a/center-and-variability/js/variability/view/VariabilityKeyboardHelpNode.ts (revision 698de1a7c7ec748e6c055de38f5332504ffb79a1) +++ b/center-and-variability/js/variability/view/VariabilityKeyboardHelpNode.ts (date 1696273432219) @@ -23,9 +23,8 @@ CenterAndVariabilityStrings.keyboardHelpDialog.moveGrabbedBallStringProperty, CenterAndVariabilityStrings.keyboardHelpDialog.jumpToStartOfNumberLineStringProperty, CenterAndVariabilityStrings.keyboardHelpDialog.jumpToEndOfNumberLineStringProperty - ), - new VariabilityKeyboardHelpSection() - ] ); + ) + ], new VariabilityKeyboardHelpSection() ); } } Index: babel/_generated_development_strings/center-and-variability_all.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/babel/_generated_development_strings/center-and-variability_all.json b/babel/_generated_development_strings/center-and-variability_all.json --- a/babel/_generated_development_strings/center-and-variability_all.json (revision 4e5d57863bedc8191980866e8b46e35ae456592b) +++ b/babel/_generated_development_strings/center-and-variability_all.json (date 1696272122861) @@ -3056,6 +3056,15 @@ "pointer": { "value": "Pointer" }, + "keyboardHelpDialog.move": { + "value": "Move" + }, + "keyboardHelpDialog.endOfNumberLine": { + "value": "end of number line" + }, + "keyboardHelpDialog.startOfNumberLine": { + "value": "start of number line" + }, "keyboardHelpDialog.grabOrReleaseBall": { "value": "Grab or Release Ball" }, @@ -3083,6 +3092,12 @@ "keyboardHelpDialog.medianScreen.grabOrReleaseBallOrCard": { "value": "Grab or Release Ball or Card" }, + "keyboardHelpDialog.medianScreen.predictMedian": { + "value": "Predict Median" + }, + "keyboardHelpDialog.medianScreen.lowercasePredictMedian": { + "value": "predict median" + }, "keyboardHelpDialog.medianScreen.moveGrabbedBallOrCardTitle": { "value": "Move Grabbed Ball or Card" }, @@ -3095,32 +3110,14 @@ "keyboardHelpDialog.medianScreen.jumpToEndOfCardsOrNumberLine": { "value": "Jump to end of cards or number line" }, - "keyboardHelpDialog.medianScreen.movePredictMedian": { - "value": "Move predict median" - }, "keyboardHelpDialog.meanAndMedianScreen.predictMeanOrMedian": { "value": "Predict Mean or Median" }, - "keyboardHelpDialog.meanAndMedianScreen.movePredictionPointer": { - "value": "Move prediction pointer" - }, - "keyboardHelpDialog.meanAndMedianScreen.movePredictionPointerInLargerSteps": { - "value": "Move prediction pointer in larger steps" - }, - "keyboardHelpDialog.meanAndMedianScreen.movePredictionPointerInSmallerSteps": { - "value": "Move prediction pointer in smaller steps" + "keyboardHelpDialog.meanAndMedianScreen.predictionPointer": { + "value": "prediction pointer" }, "keyboardHelpDialog.variabilityScreen.movePointerIntervalHandleOrIntervalBlock": { "value": "Move Pointer, Interval Handles, or Interval Block" - }, - "keyboardHelpDialog.variabilityScreen.move": { - "value": "Move" - }, - "keyboardHelpDialog.variabilityScreen.moveInSmallerSteps": { - "value": "Move in smaller steps" - }, - "keyboardHelpDialog.variabilityScreen.moveInLargerSteps": { - "value": "Move in larger steps" } } } \ No newline at end of file ```

[catherinecarter]

• The spacing looks a little tight with the addition of the up/down keys and the w and s keys:

  

  I like your example 2 above better:

  

• I agree about the inconsistent verbs on screens 1 and 2. I think originally, @terracoda suggested to start the sentence with "Predict" which treated 'predict' as the verb, but seeing the together makes me think the "Move" verb should be first on both screens. So, "Move Predict Mean or Median" Thanks for noticing.

• In general, with the addition of the up/down/w/s keys, I like the layout of example 2 with the Basic Actions moved down

• In terms of the esc key, it is mentioned in the basic actions, but only to close a dialog. I wonder if the esc is a well-known key to exit something so it doesn't need mentioning, or if the action needs to be specifically called out in the other portions of the dialog. @terracoda, can you weigh in here?

  • p.s. I looked at other keyboard dialogs and don't see the esc key mentioned anywhere except to exit a dialog, so this would be a unique keyboard dialog if we did mention it

Thanks, @marlitas!

[terracoda]

The Example 2 layout is definitely the better layout. That's what we did in Molarity's Keyboard Shortcuts.

Regarding the ESC key, I do not immediately see any combo-boxes in this sim where the ESC key is used to exit without making a new selection. Is that what you are asking about?

In Molarity and Ratio and Proportion (screen one) ESC is part of the combo-box intstructions, but we did not explicitly provide instructions for the Range combo-box on screen 2 of RaP for a few reasons:

• lack of room
• combobox was not essential for screen use
• assumption that instructions of the combobox on screen 1 would help scaffold the same interaction on screen 2

[marlitas]

@terracoda the escape key can be used to drop a ball or a card when one is "grabbed". I can remove that behavior since the space and enter keys do essentially the same thing if we'd like.

[marlitas]

The keyboard dialog is now ready for review. Over to @catherinecarter and @Nancy-Salpepi.

There were several code changes, so a code review probably would be good too.

[catherinecarter]

Yes, I'd love to hear more about @terracoda's assessment of the escape key and whether we should remove the functionality from the code, or leave it but don't mention it in the dialog box.

I'm also curious if the size of the font should remain consistent:

Is it possible, or equally so, desirable, to have the "Move Pointer..." the same size as all the other titles, as shown in the "Basic Actions" title, but move what doesn't fit to a second line? Or is the most important aspect of the title to keep all words on the same line?

[Nancy-Salpepi]

"Move in Smaller Steps" currently only works on for the Interval tool, pointer and mean prediction arrow. It doesn't work for the median prediction arrows on the first two screens. I also can't move the median prediction arrows in smaller steps with the mouse.

[catherinecarter]

The predict median will snap to 1/2 integer values, but the predict mean should be able to move in smaller steps. The median value can only be either whole or 1/2 integers, so that's why it doesn't move in smaller steps.

That said, I agree that on the Median screen, the direction to "Move in smaller steps" should be removed.

On the Mean & Median screen, let's change the wording to say, "Move Predict Mean in smaller steps" (replacing the "Move in smaller steps") to be more clear this only applies to the Predict Mean slider and not the predict median slider.

Also a curiosity: Is it difficult to change the "move in larger steps" for the prediction sliders on all three screens to go by 2's rather than 1's?

[marlitas]

That said, I agree that on the Median screen, the direction to "Move in smaller steps" should be removed.

Okay. I would love to be able to do this while still using SliderControlsKeyboardHelpSection, but I would need to add an option to include or remove the "adjust in smaller steps" row. @jessegreenberg here's a patch for that. Does that seem okay to add into this class?

```diff Subject: [PATCH] adjustInSmallerSteps option --- Index: center-and-variability/js/median/view/MedianKeyboardHelpPredictMedianSection.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/view/MedianKeyboardHelpPredictMedianSection.ts b/center-and-variability/js/median/view/MedianKeyboardHelpPredictMedianSection.ts --- a/center-and-variability/js/median/view/MedianKeyboardHelpPredictMedianSection.ts (revision 61189db96aaee7fe2cf1c61b7a2c735d6685adad) +++ b/center-and-variability/js/median/view/MedianKeyboardHelpPredictMedianSection.ts (date 1696355645419) @@ -23,7 +23,8 @@ verbStringProperty: CenterAndVariabilityStrings.keyboardHelpDialog.moveStringProperty, sliderStringProperty: CenterAndVariabilityStrings.keyboardHelpDialog.medianScreen.predictMedianStringProperty, maximumStringProperty: CenterAndVariabilityStrings.keyboardHelpDialog.endOfNumberLineStringProperty, - minimumStringProperty: CenterAndVariabilityStrings.keyboardHelpDialog.startOfNumberLineStringProperty + minimumStringProperty: CenterAndVariabilityStrings.keyboardHelpDialog.startOfNumberLineStringProperty, + includeAdjustInSmallerSteps: false } ); } } Index: scenery-phet/js/keyboard/help/SliderControlsKeyboardHelpSection.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/keyboard/help/SliderControlsKeyboardHelpSection.ts b/scenery-phet/js/keyboard/help/SliderControlsKeyboardHelpSection.ts --- a/scenery-phet/js/keyboard/help/SliderControlsKeyboardHelpSection.ts (revision e422bf40bafa43adc5b4394304e0ff95a5d29cdb) +++ b/scenery-phet/js/keyboard/help/SliderControlsKeyboardHelpSection.ts (date 1696355645413) @@ -46,6 +46,8 @@ // Strings for extremities to support shortcuts like "jump to maximum" (renaming "maximum" if desired. maximumStringProperty?: TReadOnlyProperty; minimumStringProperty?: TReadOnlyProperty; + + includeAdjustInSmallerSteps?: boolean; }; export type SliderControlsKeyboardHelpSectionOptions = SelfOptions & KeyboardHelpSectionOptions; @@ -63,7 +65,8 @@ verbStringProperty: SceneryPhetStrings.keyboardHelpDialog.adjustStringProperty, sliderStringProperty: SceneryPhetStrings.keyboardHelpDialog.sliderStringProperty, maximumStringProperty: SceneryPhetStrings.keyboardHelpDialog.maximumStringProperty, - minimumStringProperty: SceneryPhetStrings.keyboardHelpDialog.minimumStringProperty + minimumStringProperty: SceneryPhetStrings.keyboardHelpDialog.minimumStringProperty, + includeAdjustInSmallerSteps: true }, providedOptions ); const keyboardHelpDialogVerbSliderStringProperty = new PatternStringProperty( SceneryPhetStrings.keyboardHelpDialog.verbSliderPatternStringProperty, { @@ -179,7 +182,7 @@ } ); // assemble final content for KeyboardHelpSection - const content = [ adjustSliderRow, adjustSliderInSmallerStepsRow, adjustInLargerStepsRow, jumpToMinimumRow, jumpToMaximumRow ]; + const content = [ adjustSliderRow, ...( options.includeAdjustInSmallerSteps ? [ adjustSliderInSmallerStepsRow ] : [] ), adjustInLargerStepsRow, jumpToMinimumRow, jumpToMaximumRow ]; super( options.headingStringProperty, content, options ); } ```

On the Mean & Median screen, let's change the wording to say, "Move Predict Mean in smaller steps"

Hmmm I would also love to still use SliderControlsKeyboardHelpSection with this change... It definitely seems a bit trickier. I guess I could add another option like above that allows me to insert "predict mean" into that row's string, but that doesn't seem as useful down the line as the other option... @jessegreenberg what are your thoughts here?

Also a curiosity: Is it difficult to change the "move in larger steps" for the prediction sliders on all three screens to go by 2's rather than 1's?

@catherinecarter this is really easy to do. Do you want me to do it?

[marlitas]

I'll remove the ready-for-review label until these latest changes are applied.

[catherinecarter]

Thanks for getting back quickly, @marlitas. Yes, please implement the 'move by 2' as the larger steps for the prediction sliders. Thank you!

[terracoda]

Oh. I forgot about that. You do not need to describe that in Keyboard Shortcuts. It’s just a natural thing that users do to cancel an interaction.

Please leave it in.

[marlitas]

Met with @jessegreenberg today and he said that we can move forward with adding an option to SliderControlsKeyboardHelpSection to remove the "Adjust in Smaller Steps" row. I will go ahead and apply the patch above to do so.

We will pair when his schedule clears up to do the larger API change for customizing the strings in each row of the SliderControlsKeyboardHelpSection.

[marlitas]

The option to remove the smallerSteps row has been added. Next up is customizing strings.

[jessegreenberg]

@marlitas and I worked together on the above common code change so that labels for each row can be customized. Next up is using the new options in the sim.

[marlitas]

Thanks for all the help on this @jessegreenberg! This is now ready for designer review. Over to @catherinecarter.

[catherinecarter]

Looks really good. Thanks for working so hard on this, @marlitas and @jessegreenberg, much appreciated!

My only question (I'm ok either way) is whether we want to add a noun to the Variability screen after the verb, "Move" in the top right. None of the other dialogs have just "Move," and instead describe at least one thing that's moveable.

I'm ok leaving as is since there are other sims that just say, "Move," but I'm curious if it would make sense to say, "Move object" or something?

What are your thoughts, @jessegreenberg and @marlitas?

[marlitas]

I think it's completely up to your preference, I have no preference either way. Code wise we can easily make that change. Would you like it to say "Move object"?

[catherinecarter]

I guess so. Seems strange to see just "Move" without any noun after it.

So yes, let's change it to say, "Move object." Thanks @marlitas.

[marlitas]

Okay this has been changed. @catherinecarter go ahead and close the issue if all looks wells.

[marlitas]

Oops I accidentally closed it! 😅

[catherinecarter]

Looks great. Thanks.

Closing.