diff --git a/js/CarouselComboBox.ts b/js/CarouselComboBox.ts index 513e2794..1fd2d632 100644 --- a/js/CarouselComboBox.ts +++ b/js/CarouselComboBox.ts @@ -32,7 +32,7 @@ import Carousel, { CarouselOptions } from './Carousel.js'; import ComboBoxButton, { ComboBoxButtonOptions } from './ComboBoxButton.js'; import PageControl, { PageControlOptions } from './PageControl.js'; import sun from './sun.js'; -import { ComboBoxItem } from './ComboBox.js'; +import ComboBox, { ComboBoxA11yNamePropertyMap, ComboBoxItem } from './ComboBox.js'; import { getGroupItemNodes } from './GroupItemOptions.js'; type SelfOptions = { @@ -49,6 +49,9 @@ export type CarouselComboBoxOptions = SelfOptions & StrictOmit extends WidthSizable( Node ) { + // See ComboBox for a description of this property. + public readonly a11yNamePropertyMap: ComboBoxA11yNamePropertyMap; + private readonly disposeCarouselComboBox: () => void; /** @@ -113,6 +116,8 @@ export default class CarouselComboBox extends WidthSizable( Node ) { super(); + this.a11yNamePropertyMap = ComboBox.getA11yNamePropertyMap( comboBoxItems ); + // Make items in the carousel have the same width and height. const alignGroup = new AlignGroup(); @@ -148,7 +153,7 @@ export default class CarouselComboBox extends WidthSizable( Node ) { } ); // Pressing this button pops the carousel up and down - const button = new ComboBoxButton( property, comboBoxItems, contentNodes, combineOptions( { + const button = new ComboBoxButton( property, comboBoxItems, contentNodes, this.a11yNamePropertyMap, combineOptions( { listener: () => { carouselAndPageControl.visible = !carouselAndPageControl.visible; }, diff --git a/js/ComboBox.ts b/js/ComboBox.ts index b80c84f1..79c30ad0 100644 --- a/js/ComboBox.ts +++ b/js/ComboBox.ts @@ -36,7 +36,7 @@ import sun from './sun.js'; import SunConstants from './SunConstants.js'; import DerivedProperty from '../../axon/js/DerivedProperty.js'; import IntentionalAny from '../../phet-core/js/types/IntentionalAny.js'; -import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js'; +import TReadOnlyProperty, { isTReadOnlyProperty } from '../../axon/js/TReadOnlyProperty.js'; import { SpeakableResolvedResponse } from '../../utterance-queue/js/ResponsePacket.js'; import GroupItemOptions, { getGroupItemNodes } from './GroupItemOptions.js'; import Multilink from '../../axon/js/Multilink.js'; @@ -44,6 +44,7 @@ import StrictOmit from '../../phet-core/js/types/StrictOmit.js'; import PhetioProperty from '../../axon/js/PhetioProperty.js'; import Matrix3 from '../../dot/js/Matrix3.js'; import { ComboBoxListItemNodeOptions } from './ComboBoxListItemNode.js'; +import TinyProperty from '../../axon/js/TinyProperty.js'; // const const LIST_POSITION_VALUES = [ 'above', 'below' ] as const; // where the list pops up relative to the button @@ -72,6 +73,8 @@ export type ComboBoxItemNoNode = StrictOmit, 'createNode'>; export type ComboBoxListPosition = typeof LIST_POSITION_VALUES[number]; export type ComboBoxAlign = typeof ALIGN_VALUES[number]; +export type ComboBoxA11yNamePropertyMap = Map>; + // The definition for how ComboBox sets its accessibleName and helpText in the PDOM. Forward it onto its button. See // ComboBox.md for further style guide and documentation on the pattern. const ACCESSIBLE_NAME_BEHAVIOR: PDOMBehaviorFunction = ( node, options, accessibleName, otherNodeCallbacks ) => { @@ -151,6 +154,11 @@ export default class ComboBox extends WidthSizable( Node ) { // List of nodes created from ComboBoxItems to be displayed with their corresponding value. See ComboBoxItem.createNode(). public readonly nodes: Node[]; + // A map from values to dynamic a11y names. This is required for correct operation, since we need to be able to + // modify a11y names dynamically (without requiring all ComboBox clients to do the wiring). Since we can't rely on + // Properties being passed in, we'll need to create Properties here. + public readonly a11yNamePropertyMap: ComboBoxA11yNamePropertyMap; + // button that shows the current selection (internal) public button: ComboBoxButton; @@ -262,10 +270,11 @@ export default class ComboBox extends WidthSizable( Node ) { super(); this.nodes = nodes; + this.a11yNamePropertyMap = ComboBox.getA11yNamePropertyMap( items ); this.listPosition = options.listPosition; - this.button = new ComboBoxButton( property, items, nodes, { + this.button = new ComboBoxButton( property, items, nodes, this.a11yNamePropertyMap, { align: options.align, arrowDirection: ( options.listPosition === 'below' ) ? 'down' : 'up', cornerRadius: options.cornerRadius, @@ -290,7 +299,7 @@ export default class ComboBox extends WidthSizable( Node ) { } ); this.addChild( this.button ); - this.listBox = new ComboBoxListBox( property, items, nodes, + this.listBox = new ComboBoxListBox( property, items, nodes, this.a11yNamePropertyMap, this.hideListBox.bind( this ), // callback to hide the list box () => { this.button.blockNextVoicingFocusListener(); @@ -534,6 +543,29 @@ export default class ComboBox extends WidthSizable( Node ) { } ); } + public static getA11yNamePropertyMap( items: ComboBoxItem[] ): ComboBoxA11yNamePropertyMap { + const map = new Map>(); + + // Connect a11yNamePropertyMap, creating Properties as needed. + items.forEach( item => { + let property: TReadOnlyProperty; + + if ( isTReadOnlyProperty( item.a11yName ) ) { + property = item.a11yName; + } + else if ( typeof item.a11yName === 'string' ) { + property = new TinyProperty( item.a11yName ); + } + else { + property = new TinyProperty( null ); + } + + map.set( item.value, property ); + } ); + + return map; + } + public static ComboBoxIO = new IOType( 'ComboBoxIO', { valueType: ComboBox, documentation: 'A combo box is composed of a push button and a listbox. The listbox contains items that represent ' + diff --git a/js/ComboBoxButton.ts b/js/ComboBoxButton.ts index 2bd18a17..198c6e1f 100644 --- a/js/ComboBoxButton.ts +++ b/js/ComboBoxButton.ts @@ -19,11 +19,13 @@ import TProperty from '../../axon/js/TProperty.js'; import nullSoundPlayer from '../../tambo/js/shared-sound-players/nullSoundPlayer.js'; import TinyProperty from '../../axon/js/TinyProperty.js'; import StrictOmit from '../../phet-core/js/types/StrictOmit.js'; -import ComboBox, { ComboBoxItemNoNode } from './ComboBox.js'; +import ComboBox, { ComboBoxA11yNamePropertyMap, ComboBoxItemNoNode } from './ComboBox.js'; import Multilink from '../../axon/js/Multilink.js'; import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js'; import PatternStringProperty from '../../axon/js/PatternStringProperty.js'; import Property from '../../axon/js/Property.js'; +import DerivedProperty from '../../axon/js/DerivedProperty.js'; +import DynamicProperty from '../../axon/js/DynamicProperty.js'; // constants const ALIGN_VALUES = [ 'left', 'center', 'right' ] as const; @@ -66,7 +68,13 @@ export default class ComboBoxButton extends RectangularPushButton { private arrow: Path; private separatorLine: Line; - public constructor( property: TProperty, items: ComboBoxItemNoNode[], nodes: Node[], providedOptions?: ComboBoxButtonOptions ) { + public constructor( + property: TProperty, + items: ComboBoxItemNoNode[], + nodes: Node[], + a11yNamePropertyMap: ComboBoxA11yNamePropertyMap, + providedOptions?: ComboBoxButtonOptions + ) { const options = optionize()( { @@ -234,34 +242,45 @@ export default class ComboBoxButton extends RectangularPushButton { // Keep track for disposal let voicingPatternstringProperty: TReadOnlyProperty | null = null; - // When Property's value changes, show the corresponding item's Node on the button. - let item: ComboBoxItemNoNode | null = null; - const propertyObserver = ( value: T ) => { + const itemProperty = new DerivedProperty( [ property ], value => { + const item = _.find( items, item => item.value === value )!; + assert && assert( item, `no item found for value: ${value}` ); + return item; + } ); + + const nodeProperty = new DerivedProperty( [ itemProperty ], item => { + return nodes[ items.indexOf( item ) ]; + } ); + + const a11yNameProperty: TReadOnlyProperty = new DynamicProperty( itemProperty, { + derive: item => a11yNamePropertyMap.get( item.value )! + } ); + + // Show the corresponding item's Node on the button. + nodeProperty.link( node => { // remove the node for the previous item itemNodeWrapper.removeAllChildren(); - // find the ComboBoxItem whose value matches the property's value - item = _.find( items, item => item.value === value )!; - assert && assert( item, `no item found for value: ${value}` ); - const index = items.indexOf( item ); - const node = nodes[ index ]; - // add the associated node itemNodeWrapper.addChild( node ); + } ); + // Update the button's accessible name when the item changes. + a11yNameProperty.link( a11yName => { // pdom - this.innerContent = ( item.a11yName || null ); + this.innerContent = a11yName; + // TODO: We should support this changing, see https://github.com/phetsims/sun/issues/865 const patternProperty = typeof options.comboBoxVoicingNameResponsePattern === 'string' ? new Property( options.comboBoxVoicingNameResponsePattern ) : options.comboBoxVoicingNameResponsePattern; voicingPatternstringProperty && voicingPatternstringProperty.dispose(); + // TODO: DO NOT have this getting recreated, we can simply create one up front, see https://github.com/phetsims/sun/issues/865 this.voicingNameResponse = voicingPatternstringProperty = new PatternStringProperty( patternProperty, { - value: item.a11yName! + value: a11yName || '' }, { tandem: Tandem.OPT_OUT } ); - }; - property.link( propertyObserver ); + } ); // Add aria-labelledby attribute to the button. // The button is aria-labelledby its own label sibling, and then (second) its primary sibling in the PDOM. @@ -286,7 +305,7 @@ export default class ComboBoxButton extends RectangularPushButton { maxItemWidthProperty.dispose(); maxItemHeightProperty.dispose(); - property.unlink( propertyObserver ); + itemProperty.dispose(); options.localPreferredWidthProperty.unlink( preferredWidthListener ); voicingPatternstringProperty && voicingPatternstringProperty.dispose(); diff --git a/js/ComboBoxListBox.ts b/js/ComboBoxListBox.ts index 1457a3f6..d227531b 100644 --- a/js/ComboBoxListBox.ts +++ b/js/ComboBoxListBox.ts @@ -20,7 +20,7 @@ import sun from './sun.js'; import TSoundPlayer from '../../tambo/js/TSoundPlayer.js'; import DerivedProperty from '../../axon/js/DerivedProperty.js'; import TProperty from '../../axon/js/TProperty.js'; -import ComboBox, { ComboBoxItemNoNode } from './ComboBox.js'; +import ComboBox, { ComboBoxA11yNamePropertyMap, ComboBoxItemNoNode } from './ComboBox.js'; type SelfOptions = { @@ -64,8 +64,17 @@ export default class ComboBoxListBox extends Panel { * @param tandem * @param providedOptions */ - public constructor( property: TProperty, items: ComboBoxItemNoNode[], nodes: Node[], hideListBoxCallback: () => void, - focusButtonCallback: () => void, voiceOnSelectionNode: VoicingNode, tandem: Tandem, providedOptions?: ComboBoxListBoxOptions ) { + public constructor( + property: TProperty, + items: ComboBoxItemNoNode[], + nodes: Node[], + a11yNamePropertyMap: ComboBoxA11yNamePropertyMap, + hideListBoxCallback: () => void, + focusButtonCallback: () => void, + voiceOnSelectionNode: VoicingNode, + tandem: Tandem, + providedOptions?: ComboBoxListBoxOptions + ) { assert && assert( items.length > 0, 'empty list box is not supported' ); @@ -162,8 +171,11 @@ export default class ComboBoxListBox extends Panel { const listItemNodes: ComboBoxListItemNode[] = []; items.forEach( ( item, index ) => { + const a11yNameProperty = a11yNamePropertyMap.get( item.value )!; + assert && assert( a11yNameProperty ); + // Create the list item node - const listItemNode = new ComboBoxListItemNode( item, nodes[ index ], highlightWidthProperty, highlightHeightProperty, + const listItemNode = new ComboBoxListItemNode( item, nodes[ index ], a11yNameProperty, highlightWidthProperty, highlightHeightProperty, combineOptions( { align: options.align, highlightFill: options.highlightFill, diff --git a/js/ComboBoxListItemNode.ts b/js/ComboBoxListItemNode.ts index b9ff410c..c7eeeb8e 100644 --- a/js/ComboBoxListItemNode.ts +++ b/js/ComboBoxListItemNode.ts @@ -19,6 +19,7 @@ import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js'; import Property from '../../axon/js/Property.js'; import PatternStringProperty from '../../axon/js/PatternStringProperty.js'; import { ComboBoxItemNoNode } from './ComboBox.js'; +import DerivedProperty from '../../axon/js/DerivedProperty.js'; type SelfOptions = { align?: 'left' | 'right' | 'center'; @@ -47,7 +48,14 @@ export default class ComboBoxListItemNode extends Voicing( Node ) { private readonly disposeComboBoxListItemNode: () => void; - public constructor( item: ComboBoxItemNoNode, node: Node, highlightWidthProperty: TReadOnlyProperty, highlightHeightProperty: TReadOnlyProperty, providedOptions?: ComboBoxListItemNodeOptions ) { + public constructor( + item: ComboBoxItemNoNode, + node: Node, + a11yNameProperty: TReadOnlyProperty, + highlightWidthProperty: TReadOnlyProperty, + highlightHeightProperty: TReadOnlyProperty, + providedOptions?: ComboBoxListItemNodeOptions + ) { const options = optionize()( { @@ -93,17 +101,6 @@ export default class ComboBoxListItemNode extends Voicing( Node ) { options.comboBoxVoicingNameResponsePattern.includes( '{{value}}' ), 'value needs to be filled in' ); - // pdom: get innerContent from the item - options.innerContent = ( item.a11yName || null ); - options.voicingObjectResponse = ( item.a11yName || null ); - const patternProperty = typeof options.comboBoxVoicingNameResponsePattern === 'string' ? - new Property( options.comboBoxVoicingNameResponsePattern ) : - options.comboBoxVoicingNameResponsePattern; - const patternStringProperty = new PatternStringProperty( patternProperty, { - value: item.a11yName! - }, { tandem: Tandem.OPT_OUT } ); - options.voicingNameResponse = patternStringProperty; - // Highlight that is shown when the pointer is over this item. This is not the a11y focus rectangle. const highlightRectangle = new Rectangle( { cornerRadius: options.highlightCornerRadius @@ -148,6 +145,27 @@ export default class ComboBoxListItemNode extends Voicing( Node ) { super( options ); this._supplyOpenResponseOnNextFocus = false; + const emptyA11yNameProperty = new DerivedProperty( [ a11yNameProperty ], ( a11yName: string | null ) => { + return a11yName ? a11yName : ''; + } ); + + // TODO: Allow comboBoxVoicingNameResponsePattern to change, see https://github.com/phetsims/sun/issues/865 + const patternProperty = typeof options.comboBoxVoicingNameResponsePattern === 'string' ? + new Property( options.comboBoxVoicingNameResponsePattern ) : + options.comboBoxVoicingNameResponsePattern; + // TODO: It seems unlinks are missing, and the existing code was broken, see https://github.com/phetsims/sun/issues/865 + const patternStringProperty = new PatternStringProperty( patternProperty, { + value: emptyA11yNameProperty + }, { tandem: Tandem.OPT_OUT } ); + this.voicingNameResponse = patternStringProperty; + + const a11yNameListener = ( a11yName: string | null ) => { + // pdom: get innerContent from the item + this.innerContent = a11yName; + this.voicingObjectResponse = a11yName; + }; + a11yNameProperty.link( a11yNameListener ); + this.item = item; // pdom focus highlight is fitted to this Node's bounds, so that it doesn't overlap other items in the list box @@ -164,6 +182,8 @@ export default class ComboBoxListItemNode extends Voicing( Node ) { this.disposeComboBoxListItemNode = () => { patternStringProperty.dispose(); + emptyA11yNameProperty.dispose(); + a11yNameProperty.unlink( a11yNameListener ); highlightWidthProperty.unlink( highlightWidthListener ); highlightHeightProperty.unlink( highlightHeightListener ); };