Skip to content

Commit

Permalink
ComboBox work + Description Property value sets for #865
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonathanolson
jonathanolson committed Dec 20, 2023
1 parent 035ae07 commit 071966e
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 37 deletions.
9 changes: 7 additions & 2 deletions js/CarouselComboBox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -49,6 +49,9 @@ export type CarouselComboBoxOptions = SelfOptions & StrictOmit<ParentOptions, 'c

export default class CarouselComboBox<T> extends WidthSizable( Node ) {

// See ComboBox for a description of this property.
public readonly a11yNamePropertyMap: ComboBoxA11yNamePropertyMap<T>;

private readonly disposeCarouselComboBox: () => void;

/**
Expand Down Expand Up @@ -113,6 +116,8 @@ export default class CarouselComboBox<T> extends WidthSizable( Node ) {

super();

this.a11yNamePropertyMap = ComboBox.getA11yNamePropertyMap( comboBoxItems );

// Make items in the carousel have the same width and height.
const alignGroup = new AlignGroup();

Expand Down Expand Up @@ -148,7 +153,7 @@ export default class CarouselComboBox<T> extends WidthSizable( Node ) {
} );

// Pressing this button pops the carousel up and down
const button = new ComboBoxButton( property, comboBoxItems, contentNodes, combineOptions<ComboBoxButtonOptions>( {
const button = new ComboBoxButton( property, comboBoxItems, contentNodes, this.a11yNamePropertyMap, combineOptions<ComboBoxButtonOptions>( {
listener: () => {
carouselAndPageControl.visible = !carouselAndPageControl.visible;
},
Expand Down
38 changes: 35 additions & 3 deletions js/ComboBox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ 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';
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
Expand Down Expand Up @@ -72,6 +73,8 @@ export type ComboBoxItemNoNode<T> = StrictOmit<ComboBoxItem<T>, 'createNode'>;
export type ComboBoxListPosition = typeof LIST_POSITION_VALUES[number];
export type ComboBoxAlign = typeof ALIGN_VALUES[number];

export type ComboBoxA11yNamePropertyMap<T> = Map<T, TReadOnlyProperty<string | null>>;

// 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 ) => {
Expand Down Expand Up @@ -151,6 +154,11 @@ export default class ComboBox<T> 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<T>;

// button that shows the current selection (internal)
public button: ComboBoxButton<T>;

Expand Down Expand Up @@ -262,10 +270,11 @@ export default class ComboBox<T> 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,
Expand All @@ -290,7 +299,7 @@ export default class ComboBox<T> 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();
Expand Down Expand Up @@ -534,6 +543,29 @@ export default class ComboBox<T> extends WidthSizable( Node ) {
} );
}

public static getA11yNamePropertyMap<T>( items: ComboBoxItem<T>[] ): ComboBoxA11yNamePropertyMap<T> {
const map = new Map<T, TReadOnlyProperty<string | null>>();

// Connect a11yNamePropertyMap, creating Properties as needed.
items.forEach( item => {
let property: TReadOnlyProperty<string | null>;

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 ' +
Expand Down
51 changes: 35 additions & 16 deletions js/ComboBoxButton.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,7 +68,13 @@ export default class ComboBoxButton<T> extends RectangularPushButton {
private arrow: Path;
private separatorLine: Line;

public constructor( property: TProperty<T>, items: ComboBoxItemNoNode<T>[], nodes: Node[], providedOptions?: ComboBoxButtonOptions ) {
public constructor(
property: TProperty<T>,
items: ComboBoxItemNoNode<T>[],
nodes: Node[],
a11yNamePropertyMap: ComboBoxA11yNamePropertyMap<T>,
providedOptions?: ComboBoxButtonOptions
) {

const options = optionize<ComboBoxButtonOptions, SelfOptions, RectangularPushButtonOptions>()( {

Expand Down Expand Up @@ -234,34 +242,45 @@ export default class ComboBoxButton<T> extends RectangularPushButton {
// Keep track for disposal
let voicingPatternstringProperty: TReadOnlyProperty<string> | null = null;

// When Property's value changes, show the corresponding item's Node on the button.
let item: ComboBoxItemNoNode<T> | 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<string | null> = 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.
Expand All @@ -286,7 +305,7 @@ export default class ComboBoxButton<T> extends RectangularPushButton {
maxItemWidthProperty.dispose();
maxItemHeightProperty.dispose();

property.unlink( propertyObserver );
itemProperty.dispose();
options.localPreferredWidthProperty.unlink( preferredWidthListener );

voicingPatternstringProperty && voicingPatternstringProperty.dispose();
Expand Down
20 changes: 16 additions & 4 deletions js/ComboBoxListBox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {

Expand Down Expand Up @@ -64,8 +64,17 @@ export default class ComboBoxListBox<T> extends Panel {
* @param tandem
* @param providedOptions
*/
public constructor( property: TProperty<T>, items: ComboBoxItemNoNode<T>[], nodes: Node[], hideListBoxCallback: () => void,
focusButtonCallback: () => void, voiceOnSelectionNode: VoicingNode, tandem: Tandem, providedOptions?: ComboBoxListBoxOptions ) {
public constructor(
property: TProperty<T>,
items: ComboBoxItemNoNode<T>[],
nodes: Node[],
a11yNamePropertyMap: ComboBoxA11yNamePropertyMap<T>,
hideListBoxCallback: () => void,
focusButtonCallback: () => void,
voiceOnSelectionNode: VoicingNode,
tandem: Tandem,
providedOptions?: ComboBoxListBoxOptions
) {

assert && assert( items.length > 0, 'empty list box is not supported' );

Expand Down Expand Up @@ -162,8 +171,11 @@ export default class ComboBoxListBox<T> extends Panel {
const listItemNodes: ComboBoxListItemNode<T>[] = [];
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<ComboBoxListItemNodeOptions>( {
align: options.align,
highlightFill: options.highlightFill,
Expand Down
44 changes: 32 additions & 12 deletions js/ComboBoxListItemNode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -47,7 +48,14 @@ export default class ComboBoxListItemNode<T> extends Voicing( Node ) {

private readonly disposeComboBoxListItemNode: () => void;

public constructor( item: ComboBoxItemNoNode<T>, node: Node, highlightWidthProperty: TReadOnlyProperty<number>, highlightHeightProperty: TReadOnlyProperty<number>, providedOptions?: ComboBoxListItemNodeOptions ) {
public constructor(
item: ComboBoxItemNoNode<T>,
node: Node,
a11yNameProperty: TReadOnlyProperty<string | null>,
highlightWidthProperty: TReadOnlyProperty<number>,
highlightHeightProperty: TReadOnlyProperty<number>,
providedOptions?: ComboBoxListItemNodeOptions
) {

const options = optionize<ComboBoxListItemNodeOptions, SelfOptions, ParentOptions>()( {

Expand Down Expand Up @@ -93,17 +101,6 @@ export default class ComboBoxListItemNode<T> 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
Expand Down Expand Up @@ -148,6 +145,27 @@ export default class ComboBoxListItemNode<T> 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
Expand All @@ -164,6 +182,8 @@ export default class ComboBoxListItemNode<T> extends Voicing( Node ) {

this.disposeComboBoxListItemNode = () => {
patternStringProperty.dispose();
emptyA11yNameProperty.dispose();
a11yNameProperty.unlink( a11yNameListener );
highlightWidthProperty.unlink( highlightWidthListener );
highlightHeightProperty.unlink( highlightHeightListener );
};
Expand Down

0 comments on commit 071966e

Please sign in to comment.