TimeControlNode: description design and implementation does not consider options.

[pixelzoom]

Related to phetsims/models-of-the-hydrogen-atom#67 and phetsims/sun#901 ...

TimeControlNode has many options that affect the configuration of its user interface. The current description design and implementation seems to not have considered these options, and instead seems specific to some sim. Here's what it looks like in A11y View:

Problems:

• "Play sim on slow speed" does not consider that TimeControlNode may not have a "Slow" speed. Recommend changing this to "Play sim on slower speed".
• "Timing" is the wrong word here, it means something different. There is a reason why the UI componnt is named TimeControlNode, and "Time" should be used in the description. (Ditto for TimingControlsKeyboardHelpSection.)

[pixelzoom]

@jessegreenberg and I discussed this issue. While changing "slow" to "slower" and "Timing" to "Time" would be an improvement, this is not blocking for MOTHA publication. It can be addressed in a future release, there will be no sim-specific code changes needed to MOTHA, and MOTHA does have a "Slow" speed.

[marlitas]

Meeting 11/1/24

• Make the helpText default "smarter" by detecting which options have been passed through to then update accordingly.
• Any default helpText or accessibleName in TimeControlNode should still be able to be overwritten.
• We agreed that the overall design pattern throughout common code is to make accessibleName and helpText mutable.

[jessegreenberg]

In e168a23 I renamed "Timing Controls" to "Time Controls" in a11y and translatable strings, and I changed the description to use "slower", so it is more general for options. I did not touch string keys.

The other commits rename the class TimingControlsKeyboardHelpSection and references in code.

I discussed it with @terracoda and she is OK with these changes.

@pixelzoom would you like to review? Feel free to close, unless I missed something.

[pixelzoom]

... Feel free to close, unless I missed something.

There are definitely more ways to configure TimeControlNode that break the current description.

Examples:

• playPauseStepButtonOptions.includeStepForwardButton: false hides the step-forward button, making "step forward little by little" irrelevant.
• playPauseStepButtonOptions.includeStepBackwardButton: true adds a step-backward button, which the description says nothing about. And the step-backward button has no accessibleName.
• lots of things that can be configured dynamically via PhET-iO.

... and there may be others. So recommended to take a closer look at the TimeControlNode API. Either remove some options that are undesirable, or make description aware of those options.

PhET-iO configuration may be too much to tackle in this issue, but it's a problem. And it's going to become a bigger problem with PhET Premium.

[pixelzoom]

Also note that the step-backward button has no accessibleName.

[jessegreenberg]

OK, thanks for reviewing. Ill take a broader look at the options. Also wondering if we can find more general default help text that would be agnostic to the options.

[jessegreenberg]

After reviewing options, it isn't so bad (if we ignore phet-io customizations for now) -

• timeSpeeds?: TimeSpeed[]; // Controls which speed radio buttons are included
• PlayPauseStepButtonGroupOptions:
  • includeStepForwardButton
  • includeStepBackwardButton

It is possible that neither step button is included as well.

So we either need general help text for play/pause/step buttons that work for all options, or 4 different versions - One if both step buttons are visible, one if only forward is visible, one if only backward is visible, one if neither are visible.

[jessegreenberg]

Discussed with @terracoda. She recommended these variations in the help text.

• Pause to step through little by little.
• No help text if neither step button is included.

Another option was to be more explicit and describe each case.

• Pause to step forward little by little.
• Pause to step backward little by little.
• Pause to step forward or backward little by little.

We think the first option is reasonable. The buttons themselves will have the accessible names so it doesn't seem necessary to call out each case in the help text.

[jessegreenberg]

This was done in the above commit. The other commits rename description -> helpText for consistency.

@pixelzoom can you please review this?

[pixelzoom]

👍🏻 Closing.