NumberControl: support for `accessibleName` and `helpText`, display units in A11y View

[pixelzoom]

Related to https://github.com/phetsims/models-of-the-hydrogen-atom/issues/67 and https://github.com/phetsims/sun/issues/901 ...

NumberControl does not support accessibleName and helpText. You can provide these options, but they are ignored.

Developers have been using this undesirable workaround in sim-specific code:

const numberControl = new NumberControl( ..., {
  ...
  sliderOptions: {
    ...
    accessibleName: 'The accessible name.',
    helpText: 'The help text.'
  },
} );

Besides the obvious problems of reaching inside NumberControl to access its Slider, this workaround does not address the units associated with the NumberControl's value. NumberControl is responsible for units, Slider has no knowledge of units. So for example in MOTHA, we end up with this in the A11y View, where we want to see "380 nm".

[jessegreenberg]

With the above commits, NumberControl supports accessibleName and helpText, forwarding them to the Slider in its implementation.

This workaround does not address the units associated with the NumberControl's value. NumberControl is responsible for units, Slider has no knowledge of units.

The units that appear in the a11y view would come from an AccessibleValueHandler option pdomCreateAriaValueText. Here is what it looks like right now:

const myNumberControl = new NumberControl( {
  accessibleName: 'My NumberControl',
  numberDisplayOptions: {
    valuePattern: '{0} lbs'
  },
  sliderOptions: {
    pdomCreateAriaValueText: value => {
      return StringUtils.format( '{0} lbs', value );
    }
  }

One option is NumberControl could take a top level valuePattern that is used for both the NumberDisplay and the aria-valueText. Would that be better?

const myNumberControl = new NumberControl( {
  accessibleName: 'My NumberControl',
  valuePattern: '{0} lbs'
} );

[pixelzoom]

I reviewed the changes for accessibleName and helpText. Should accessibleNameBehavior and helpTextBehavior also be excluded from NumberControlSliderOptions?

I'll need to get back to you on the issue of units - it requires a little more digging and ruminating.

[jessegreenberg]

Yes, thanks. We are now excluding all ParallelDOMOptions from NumberControlSliderOptions in https://github.com/phetsims/scenery-phet/commit/c3d402b93428aa6db895e1c7433ccd07353024ed.

[pixelzoom]

@jessegreenberg asked:

One option is NumberControl could take a top level valuePattern that is used for both the NumberDisplay and the aria-valueText. Would that be better?

Yes, I think this would be a better approach than having to duplicate the pattern in a new option (like sliderOptions. pdomCreateAriaValueText). But unfortunately NumberDisplay has multiple APIs that control the format of what it displays. From NumberDisplay.ts SelfOptions:

  // Pattern used to format the value.
  // Must contain SunConstants.VALUE_NAMED_PLACEHOLDER or SunConstants.VALUE_NUMBERED_PLACEHOLDER.
  valuePattern?: string | TReadOnlyProperty<string>;

  // The number of decimal places to show. If null, the full value is displayed.
  // We attempted to change the default to null, but there were too many usages that relied on the 0 default.
  // See https://github.com/phetsims/scenery-phet/issues/511
  decimalPlaces?: number | null;

  // Takes a {number} and returns a {string} for full control. Mutually exclusive with valuePattern and
  // decimalPlaces.  Named "numberFormatter" instead of "formatter" to help clarify that it is separate from the
  // noValueString/Align/Pattern defined below. Please see also numberFormatterDependencies
  numberFormatter?: ( ( n: number ) => string ) | null;

  // If your numberFormatter depends on other Properties, you must specify them so that the text will update when those
  // dependencies change.
  numberFormatterDependencies?: TReadOnlyProperty<unknown>[];

numberFormatter provides more control (as the doc says). And I think you'll find that not all NumberControls are using valuePattern.

[pixelzoom]

Below is a patch that I don't think is too awful. It involves adding this to NumberControl:

    this.slider.ariaValueText = numberDisplay.valueStringProperty;

... and making adding valueStringProperty to NumberDisplay's public API:

public readonly valueStringProperty: TReadOnlyProperty<string>;

So the Slider's ariaValueText will be exactly what the NumberDisplay is showing.

It looks like ariaValueText is already excluded from sliderOptions?: NumberControlSliderOptions, but you should verify.

patch

```diff Subject: [PATCH] TODOs for https://github.com/phetsims/joist/issues/989 --- Index: js/NumberDisplay.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/NumberDisplay.ts b/js/NumberDisplay.ts --- a/js/NumberDisplay.ts (revision 2440ab76884e9d8ac047702e99bd6dab301d4079) +++ b/js/NumberDisplay.ts (date 1730842298402) @@ -89,6 +89,7 @@ private readonly numberFormatterProperty: TProperty; private readonly valueText: RichText | Text; private readonly backgroundNode: Rectangle; + public readonly valueStringProperty: TReadOnlyProperty; private readonly disposeNumberDisplay: () => void; // called by dispose /** @@ -280,6 +281,7 @@ this.numberFormatterProperty = numberFormatterProperty; this.valueText = valueText; this.backgroundNode = backgroundNode; + this.valueStringProperty = valueStringProperty; // Align the value in the background. ManualConstraint.create( this, [ valueTextContainer, backgroundNode ], ( valueTextContainerProxy, backgroundNodeProxy ) => { Index: js/NumberControl.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/NumberControl.ts b/js/NumberControl.ts --- a/js/NumberControl.ts (revision 2440ab76884e9d8ac047702e99bd6dab301d4079) +++ b/js/NumberControl.ts (date 1730842298380) @@ -424,6 +424,7 @@ const numberDisplay = new NumberDisplay( numberProperty, numberRange, options.numberDisplayOptions ); this.slider = new Slider( numberProperty, numberRange, options.sliderOptions ); + this.slider.ariaValueText = numberDisplay.valueStringProperty; // pdom - forward the accessibleName and help text set on this component to the slider ParallelDOM.forwardAccessibleName( this, this.slider ); ```

[pixelzoom]

@jessegreenberg and I discussed on Zoom. Setting this.slider.ariaValueText does not dynamically update, so it's probably some other option that needs to be set. But we like the approach of using NumberDisplay's valueStringProperty.

@jessegreenberg will take it from here. Let me know if you'd like me to review.

[jessegreenberg]

That was a great idea, committed above. Can you please review @pixelzoom?