-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use GroupItemOptions for ComboBox #797
Comments
Apply this pattern to |
Progress so far: Index: main/sun/js/ComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ComboBox.ts b/main/sun/js/ComboBox.ts
--- a/main/sun/js/ComboBox.ts (revision 15be3e7ec7b6b9ad89936a0f6ecdbf2e23885ea9)
+++ b/main/sun/js/ComboBox.ts (date 1665189049304)
@@ -41,6 +41,7 @@
import LinkableProperty from '../../axon/js/LinkableProperty.js';
import TinyProperty from '../../axon/js/TinyProperty.js';
import Multilink, { UnknownMultilink } from '../../axon/js/Multilink.js';
+import GroupItemOptions, { getGroupItemNodes } from './GroupItemOptions.js';
// const
const LIST_POSITION_VALUES = [ 'above', 'below' ] as const; // where the list pops up relative to the button
@@ -51,19 +52,13 @@
// the value associated with the item
value: T;
- // the node displayed in the combo box for the item
- node: Node;
-
// Sound that will be played when this item is selected. If set to `null` a default sound will be used that is based
// on this item's position in the combo box list. A value of `nullSoundPlayer` can be used to disable.
soundPlayer?: TSoundPlayer | null;
- // phet-io - the tandem name for this item's associated Node in the combo box
- tandemName?: string | null;
-
// pdom - the label for this item's associated Node in the combo box
a11yLabel?: PDOMValueType | null;
-};
+} & GroupItemOptions;
export type ComboBoxListPosition = typeof LIST_POSITION_VALUES[number];
export type ComboBoxAlign = typeof ALIGN_VALUES[number];
@@ -184,9 +179,6 @@
assert && assert( _.uniqBy( items, ( item: ComboBoxItem<T> ) => item.value ).length === items.length,
'items must have unique values' );
assert && items.forEach( item => {
- assert && assert( !item.node.hasPDOMContent, 'Accessibility is provided by ComboBoxItemNode and ' +
- 'ComboBoxItem.a11yLabel. Additional PDOM content in the provided ' +
- 'Node could break accessibility.' );
assert && assert( !item.tandemName || item.tandemName.endsWith( ComboBox.ITEM_TANDEM_NAME_SUFFIX ),
`ComboBoxItem tandemName must end with '${ComboBox.ITEM_TANDEM_NAME_SUFFIX}': ${item.tandemName}` );
} );
@@ -242,6 +234,18 @@
phetioEnabledPropertyInstrumented: true // opt into default PhET-iO instrumented enabledProperty
}, providedOptions );
+ const nodes = getGroupItemNodes( items, options.tandem.createTandem( 'items' ) );
+
+ assert && nodes.forEach( node => {
+ assert && assert( !node.hasPDOMContent, 'Accessibility is provided by ComboBoxItemNode and ' +
+ 'ComboBoxItem.a11yLabel. Additional PDOM content in the provided ' +
+ 'Node could break accessibility.' );
+ } );
+ const elements = [];
+ for ( let i = 0; i < items.length; i++ ) {
+ elements[ i ] = { item: items[ i ], node: nodes[ i ] };
+ }
+
// validate option values
assert && assert( options.xMargin > 0 && options.yMargin > 0,
`margins must be > 0, xMargin=${options.xMargin}, yMargin=${options.yMargin}` );
@@ -260,8 +264,9 @@
}
// We'll need to adjust our button's preferred width if we have a label
- const buttonPreferredWidthProperty = options.labelNode
- ? new DerivedProperty( [
+ const buttonPreferredWidthProperty =
+ options.labelNode ?
+ new DerivedProperty( [
this.localPreferredWidthProperty,
options.labelNode.boundsProperty
], ( localPreferredWidth, labelBounds ) => {
@@ -269,8 +274,7 @@
return localPreferredWidth === null ? null : localPreferredWidth - labelBounds.width - options.labelXSpacing;
}, {
reentrant: true
- } )
- : this.localPreferredWidthProperty;
+ } ) : this.localPreferredWidthProperty;
// We'll need to adjust our (incoming, from the button) minimum width if we have a label.
let buttonMinimumWidthProperty = this.localMinimumWidthProperty;
@@ -288,7 +292,7 @@
} );
}
- this.button = new ComboBoxButton( property, items, {
+ this.button = new ComboBoxButton( property, items, nodes, {
align: options.align,
arrowDirection: ( options.listPosition === 'below' ) ? 'down' : 'up',
cornerRadius: options.cornerRadius,
@@ -321,7 +325,8 @@
} );
}
- this.listBox = new ComboBoxListBox( property, items,
+ // TODO: https://github.com/phetsims/sun/issues/797 don't forget dispose
+ this.listBox = new ComboBoxListBox( property, items, nodes,
this.hideListBox.bind( this ), // callback to hide the list box
() => {
this.button.blockNextVoicingFocusListener();
@@ -542,26 +547,26 @@
return this.listBox.isItemVisible( value );
}
- public static getMaxItemWidthProperty<T>( items: ComboBoxItem<T>[] ): TReadOnlyProperty<number> {
- const widthProperties = _.flatten( items.map( item => {
- const properties: TReadOnlyProperty<IntentionalAny>[] = [ item.node.boundsProperty ];
- if ( mixesWidthSizable( item.node ) ) {
- properties.push( item.node.isWidthResizableProperty );
- properties.push( item.node.minimumWidthProperty );
+ public static getMaxItemWidthProperty( nodes: Node[] ): TReadOnlyProperty<number> {
+ const widthProperties = _.flatten( nodes.map( node => {
+ const properties: TReadOnlyProperty<IntentionalAny>[] = [ node.boundsProperty ];
+ if ( mixesWidthSizable( node ) ) {
+ properties.push( node.isWidthResizableProperty );
+ properties.push( node.minimumWidthProperty );
}
return properties;
} ) );
// @ts-ignore DerivedProperty isn't understanding the Property[]
return new DerivedProperty( widthProperties, () => {
- return Math.max( ...items.map( item => isWidthSizable( item.node ) ? item.node.minimumWidth || 0 : item.node.width ) );
+ return Math.max( ...nodes.map( node => isWidthSizable( node ) ? node.minimumWidth || 0 : node.width ) );
} );
}
- public static getMaxItemHeightProperty<T>( items: ComboBoxItem<T>[] ): TReadOnlyProperty<number> {
- const heightProperties = items.map( item => item.node.boundsProperty );
+ public static getMaxItemHeightProperty( nodes: Node[] ): TReadOnlyProperty<number> {
+ const heightProperties = nodes.map( node => node.boundsProperty );
// @ts-ignore DerivedProperty isn't understanding the Property[]
return new DerivedProperty( heightProperties, () => {
- return Math.max( ...items.map( item => item.node.height ) );
+ return Math.max( ...nodes.map( node => node.height ) );
} );
}
Index: main/tambo/js/demo/ui-components/view/UIComponentsScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/tambo/js/demo/ui-components/view/UIComponentsScreenView.ts b/main/tambo/js/demo/ui-components/view/UIComponentsScreenView.ts
--- a/main/tambo/js/demo/ui-components/view/UIComponentsScreenView.ts (revision 5fa44dd369d0e620cf872d4438cae456cdb81ade)
+++ b/main/tambo/js/demo/ui-components/view/UIComponentsScreenView.ts (date 1665187567746)
@@ -167,9 +167,9 @@
{
label: 'ComboBox',
createNode: ( layoutBounds: Bounds2 ) => new ComboBox( new NumberProperty( 0 ), [
- { value: 0, node: new Text( 'Rainbows', { font: LABEL_FONT } ) },
- { value: 1, node: new Text( 'Unicorns', { font: LABEL_FONT } ) },
- { value: 2, node: new Text( 'Butterflies', { font: LABEL_FONT } ) }
+ { value: 0, createNode: tandem => new Text( 'Rainbows', { font: LABEL_FONT } ) },
+ { value: 1, createNode: tandem => new Text( 'Unicorns', { font: LABEL_FONT } ) },
+ { value: 2, createNode: tandem => new Text( 'Butterflies', { font: LABEL_FONT } ) }
], this, { center: layoutBounds.center } )
},
{
Index: main/joist/js/preferences/RegionAndCultureComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/preferences/RegionAndCultureComboBox.ts b/main/joist/js/preferences/RegionAndCultureComboBox.ts
--- a/main/joist/js/preferences/RegionAndCultureComboBox.ts (revision 6c6750f47fb15732ec31e239850606ac742ccf0f)
+++ b/main/joist/js/preferences/RegionAndCultureComboBox.ts (date 1665187507219)
@@ -52,7 +52,7 @@
return {
value: index,
- node: itemContent
+ createNode: ( tandem: Tandem ) => itemContent
};
} );
Index: main/joist/js/preferences/VoicingPanelSection.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/preferences/VoicingPanelSection.ts b/main/joist/js/preferences/VoicingPanelSection.ts
--- a/main/joist/js/preferences/VoicingPanelSection.ts (revision 6c6750f47fb15732ec31e239850606ac742ccf0f)
+++ b/main/joist/js/preferences/VoicingPanelSection.ts (date 1665187507206)
@@ -467,7 +467,7 @@
if ( voices.length === 0 ) {
items.push( {
value: null,
- node: new Text( noVoicesAvailableStringProperty, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS ),
+ createNode: ( tandem: Tandem ) => new Text( noVoicesAvailableStringProperty, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS ),
a11yLabel: noVoicesAvailableStringProperty
} );
}
@@ -475,7 +475,7 @@
voices.forEach( voice => {
items.push( {
value: voice,
- node: new Text( voice.name, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS ),
+ createNode: ( tandem: Tandem ) => new Text( voice.name, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS ),
a11yLabel: voice.name
} );
} );
Index: main/greenhouse-effect/js/common/view/ThermometerAndReadout.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/greenhouse-effect/js/common/view/ThermometerAndReadout.ts b/main/greenhouse-effect/js/common/view/ThermometerAndReadout.ts
--- a/main/greenhouse-effect/js/common/view/ThermometerAndReadout.ts (revision 11060b0634db2641b7f7524f42ef7b63ec07a7ff)
+++ b/main/greenhouse-effect/js/common/view/ThermometerAndReadout.ts (date 1665187405681)
@@ -221,7 +221,7 @@
return {
value: propertyValue,
- node: new NumberDisplay( property, propertyRange, numberDisplayOptions ),
+ createNode: tandem => new NumberDisplay( property, propertyRange, numberDisplayOptions ),
tandemName: tandemName,
a11yLabel: TemperatureDescriber.getTemperatureUnitsString( propertyValue )
};
Index: main/twixt/js/demo/EasingComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/twixt/js/demo/EasingComboBox.js b/main/twixt/js/demo/EasingComboBox.js
--- a/main/twixt/js/demo/EasingComboBox.js (revision a22ad38df670cf634f8155fce7ee8d89fb14a8dc)
+++ b/main/twixt/js/demo/EasingComboBox.js (date 1665188823136)
@@ -23,19 +23,19 @@
const comboTextOptions = { font: new PhetFont( 16 ) };
const items = [
- { value: Easing.LINEAR, node: new Text( 'Linear', comboTextOptions ) },
- { value: Easing.QUADRATIC_IN_OUT, node: new Text( 'Quadratic in-out', comboTextOptions ) },
- { value: Easing.QUADRATIC_IN, node: new Text( 'Quadratic in', comboTextOptions ) },
- { value: Easing.QUADRATIC_OUT, node: new Text( 'Quadratic out', comboTextOptions ) },
- { value: Easing.CUBIC_IN_OUT, node: new Text( 'Cubic in-out', comboTextOptions ) },
- { value: Easing.CUBIC_IN, node: new Text( 'Cubic in', comboTextOptions ) },
- { value: Easing.CUBIC_OUT, node: new Text( 'Cubic out', comboTextOptions ) },
- { value: Easing.QUARTIC_IN_OUT, node: new Text( 'Quartic in-out', comboTextOptions ) },
- { value: Easing.QUARTIC_IN, node: new Text( 'Quartic in', comboTextOptions ) },
- { value: Easing.QUARTIC_OUT, node: new Text( 'Quartic out', comboTextOptions ) },
- { value: Easing.QUINTIC_IN_OUT, node: new Text( 'Quintic in-out', comboTextOptions ) },
- { value: Easing.QUINTIC_IN, node: new Text( 'Quintic in', comboTextOptions ) },
- { value: Easing.QUINTIC_OUT, node: new Text( 'Quintic out', comboTextOptions ) }
+ { value: Easing.LINEAR, createNode: tandem => new Text( 'Linear', comboTextOptions ) },
+ { value: Easing.QUADRATIC_IN_OUT, createNode: tandem => new Text( 'Quadratic in-out', comboTextOptions ) },
+ { value: Easing.QUADRATIC_IN, createNode: tandem => new Text( 'Quadratic in', comboTextOptions ) },
+ { value: Easing.QUADRATIC_OUT, createNode: tandem => new Text( 'Quadratic out', comboTextOptions ) },
+ { value: Easing.CUBIC_IN_OUT, createNode: tandem => new Text( 'Cubic in-out', comboTextOptions ) },
+ { value: Easing.CUBIC_IN, createNode: tandem => new Text( 'Cubic in', comboTextOptions ) },
+ { value: Easing.CUBIC_OUT, createNode: tandem => new Text( 'Cubic out', comboTextOptions ) },
+ { value: Easing.QUARTIC_IN_OUT, createNode: tandem => new Text( 'Quartic in-out', comboTextOptions ) },
+ { value: Easing.QUARTIC_IN, createNode: tandem => new Text( 'Quartic in', comboTextOptions ) },
+ { value: Easing.QUARTIC_OUT, createNode: tandem => new Text( 'Quartic out', comboTextOptions ) },
+ { value: Easing.QUINTIC_IN_OUT, createNode: tandem => new Text( 'Quintic in-out', comboTextOptions ) },
+ { value: Easing.QUINTIC_IN, createNode: tandem => new Text( 'Quintic in', comboTextOptions ) },
+ { value: Easing.QUINTIC_OUT, createNode: tandem => new Text( 'Quintic out', comboTextOptions ) }
];
super( easingProperty, items, listParent, options );
Index: main/density-buoyancy-common/js/buoyancy/view/ShapeSizeControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/density-buoyancy-common/js/buoyancy/view/ShapeSizeControlNode.ts b/main/density-buoyancy-common/js/buoyancy/view/ShapeSizeControlNode.ts
--- a/main/density-buoyancy-common/js/buoyancy/view/ShapeSizeControlNode.ts (revision 329d94f227a28a0c893cd67642b4df0ac6bf71af)
+++ b/main/density-buoyancy-common/js/buoyancy/view/ShapeSizeControlNode.ts (date 1665187371696)
@@ -62,7 +62,7 @@
const comboBox = new ComboBox( massShapeProperty, MassShape.enumeration.values.map( massShape => {
return {
value: massShape,
- node: new Text( shapeStringMap[ massShape.name ], {
+ createNode: tandem => new Text( shapeStringMap[ massShape.name ], {
font: DensityBuoyancyCommonConstants.COMBO_BOX_ITEM_FONT,
maxWidth: 160
} ),
Index: main/tangible/js/mediaPipe/MediaPipe.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/tangible/js/mediaPipe/MediaPipe.ts b/main/tangible/js/mediaPipe/MediaPipe.ts
--- a/main/tangible/js/mediaPipe/MediaPipe.ts (revision 98dba401dffec1bdb5651a4e13983500ef79085f)
+++ b/main/tangible/js/mediaPipe/MediaPipe.ts (date 1665187567741)
@@ -300,7 +300,7 @@
const label = device.label || `Camera ${i + 1}`;
return {
value: device.deviceId,
- node: new Text( label ),
+ createNode: ( tandem: Tandem ) => new Text( label ),
a11yLabel: label
};
} );
Index: main/balancing-chemical-equations/js/introduction/view/ToolsComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/balancing-chemical-equations/js/introduction/view/ToolsComboBox.js b/main/balancing-chemical-equations/js/introduction/view/ToolsComboBox.js
--- a/main/balancing-chemical-equations/js/introduction/view/ToolsComboBox.js (revision 561fe846206c7d43dd9a2a13a929f2a6e921449f)
+++ b/main/balancing-chemical-equations/js/introduction/view/ToolsComboBox.js (date 1665188875068)
@@ -45,9 +45,9 @@
} );
const items = [
- { value: BalancedRepresentation.NONE, node: new Text( BalancingChemicalEquationsStrings.none, { font: FONT, maxWidth: 100 } ) },
- { value: BalancedRepresentation.BALANCE_SCALES, node: new Image( scales_png, { scale: 0.1875 } ) },
- { value: BalancedRepresentation.BAR_CHARTS, node: new Image( charts_png, { scale: 0.375 } ) }
+ { value: BalancedRepresentation.NONE, createNode: tandem => new Text( BalancingChemicalEquationsStrings.none, { font: FONT, maxWidth: 100 } ) },
+ { value: BalancedRepresentation.BALANCE_SCALES, createNode: tandem => new Image( scales_png, { scale: 0.1875 } ) },
+ { value: BalancedRepresentation.BAR_CHARTS, createNode: tandem => new Image( charts_png, { scale: 0.375 } ) }
];
super( balanceRepresentationProperty, items, parentNode, options );
Index: main/ph-scale/js/common/view/SoluteComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/ph-scale/js/common/view/SoluteComboBox.ts b/main/ph-scale/js/common/view/SoluteComboBox.ts
--- a/main/ph-scale/js/common/view/SoluteComboBox.ts (revision 03e802164a667ce6b77de8446e923ef2580663a8)
+++ b/main/ph-scale/js/common/view/SoluteComboBox.ts (date 1665187507208)
@@ -63,7 +63,7 @@
items.push( {
value: solute,
- node: hBox,
+ createNode: tandem => hBox,
tandemName: `${solute.tandemName}${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
} );
} );
Index: main/density-buoyancy-common/js/common/view/DensityControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/density-buoyancy-common/js/common/view/DensityControlNode.ts b/main/density-buoyancy-common/js/common/view/DensityControlNode.ts
--- a/main/density-buoyancy-common/js/common/view/DensityControlNode.ts (revision 329d94f227a28a0c893cd67642b4df0ac6bf71af)
+++ b/main/density-buoyancy-common/js/common/view/DensityControlNode.ts (date 1665187752283)
@@ -43,7 +43,7 @@
].map( material => {
return {
value: material,
- node: new Text( material.name, {
+ createNode: tandem => new Text( material.name, {
font: DensityBuoyancyCommonConstants.COMBO_BOX_ITEM_FONT,
maxWidth: 160
} ),
Index: main/quadrilateral/js/quadrilateral/view/sound/QuadrilateralSoundOptionsNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/quadrilateral/js/quadrilateral/view/sound/QuadrilateralSoundOptionsNode.ts b/main/quadrilateral/js/quadrilateral/view/sound/QuadrilateralSoundOptionsNode.ts
--- a/main/quadrilateral/js/quadrilateral/view/sound/QuadrilateralSoundOptionsNode.ts (revision e514aa56b3a68e19746965a040bb6802cf46d827)
+++ b/main/quadrilateral/js/quadrilateral/view/sound/QuadrilateralSoundOptionsNode.ts (date 1665187507215)
@@ -56,62 +56,62 @@
const designComboBox = new CarouselComboBox( model.soundDesignProperty, [
{
value: SoundDesign.MAINTENANCE_SOUNDS,
- node: new Text( 'Maintenance Sounds', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Maintenance Sounds', LABEL_TEXT_OPTIONS ),
tandemName: `successWithMaintenance${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.SUCCESS_SOUNDS,
- node: new Text( 'Success Sounds', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Success Sounds', LABEL_TEXT_OPTIONS ),
tandemName: `success${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.QUARTET,
- node: new Text( 'Quartet', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Quartet', LABEL_TEXT_OPTIONS ),
tandemName: `quartet${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.PARALLELS_VOLUME,
- node: new Text( 'Parallels Volume', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Parallels Volume', LABEL_TEXT_OPTIONS ),
tandemName: `parallelsVolume${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.PARALLELS_STACCATO,
- node: new Text( 'Parallels Staccato', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Parallels Staccato', LABEL_TEXT_OPTIONS ),
tandemName: `parallelsStaccato${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.TRACKS_BUILD_UP,
- node: new Text( 'Tracks - Build Up', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Tracks - Build Up', LABEL_TEXT_OPTIONS ),
tandemName: `tracksBuildUp${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.TRACKS_VOLUME_EMPHASIS,
- node: new Text( 'Tracks - Volume Emphasis', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Tracks - Volume Emphasis', LABEL_TEXT_OPTIONS ),
tandemName: `tracksVolume${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.TRACKS_MELODY,
- node: new Text( 'Tracks - Melody', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Tracks - Melody', LABEL_TEXT_OPTIONS ),
tandemName: `tracksMelody${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.TRACKS_ARPEGGIO,
- node: new Text( 'Tracks - Arpeggio', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Tracks - Arpeggio', LABEL_TEXT_OPTIONS ),
tandemName: `tracksArpeggio${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.TRACKS_BUILD_UP_SIMPLE,
- node: new Text( 'Tracks - Build Up - Simple', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Tracks - Build Up - Simple', LABEL_TEXT_OPTIONS ),
tandemName: `tracksBuildUpSimple${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.TRACKS_MELODY_SIMPLE,
- node: new Text( 'Tracks - Melody - Simple', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Tracks - Melody - Simple', LABEL_TEXT_OPTIONS ),
tandemName: `tracksMelodySimple${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
},
{
value: SoundDesign.TRACKS_MELODY_MAPPING,
- node: new Text( 'Tracks - Melody - Mapping', LABEL_TEXT_OPTIONS ),
+ createNode: tandem => new Text( 'Tracks - Melody - Mapping', LABEL_TEXT_OPTIONS ),
tandemName: `tracksMelodyMapping${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
}
], {
Index: main/geometric-optics/js/common/view/OpticalObjectChoiceComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/geometric-optics/js/common/view/OpticalObjectChoiceComboBox.ts b/main/geometric-optics/js/common/view/OpticalObjectChoiceComboBox.ts
--- a/main/geometric-optics/js/common/view/OpticalObjectChoiceComboBox.ts (revision 1c78c5492a945a5331049ff87c3e49714a561fbf)
+++ b/main/geometric-optics/js/common/view/OpticalObjectChoiceComboBox.ts (date 1665187405676)
@@ -76,7 +76,7 @@
// create and add combo box item to the array
items.push( {
value: opticalObjectChoice,
- node: hBox,
+ createNode: tandem => hBox,
tandemName: itemTandemName
} );
} );
Index: main/beers-law-lab/js/concentration/view/SoluteComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/beers-law-lab/js/concentration/view/SoluteComboBox.ts b/main/beers-law-lab/js/concentration/view/SoluteComboBox.ts
--- a/main/beers-law-lab/js/concentration/view/SoluteComboBox.ts (revision d7631720b8b880d82ed6b96d3180fe6cf553d9a2)
+++ b/main/beers-law-lab/js/concentration/view/SoluteComboBox.ts (date 1665187371693)
@@ -78,7 +78,7 @@
return {
value: solute,
- node: hBox,
+ createNode: tandem => hBox,
tandemName: `${solute.tandemName}${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
};
}
Index: main/density-buoyancy-common/js/common/view/MaterialMassVolumeControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/density-buoyancy-common/js/common/view/MaterialMassVolumeControlNode.ts b/main/density-buoyancy-common/js/common/view/MaterialMassVolumeControlNode.ts
--- a/main/density-buoyancy-common/js/common/view/MaterialMassVolumeControlNode.ts (revision 329d94f227a28a0c893cd67642b4df0ac6bf71af)
+++ b/main/density-buoyancy-common/js/common/view/MaterialMassVolumeControlNode.ts (date 1665187371682)
@@ -257,7 +257,7 @@
...materials.map( material => {
return {
value: MaterialEnumeration[ material.identifier! ],
- node: new Text( material.name, {
+ createNode: ( tandem: Tandem ) => new Text( material.name, {
font: DensityBuoyancyCommonConstants.COMBO_BOX_ITEM_FONT,
maxWidth: comboMaxWidth
} ),
@@ -266,7 +266,7 @@
} ),
{
value: MaterialEnumeration.CUSTOM,
- node: new Text( DensityBuoyancyCommonStrings.material.customStringProperty, {
+ createNode: tandem => new Text( DensityBuoyancyCommonStrings.material.customStringProperty, {
font: DensityBuoyancyCommonConstants.COMBO_BOX_ITEM_FONT,
maxWidth: comboMaxWidth
} ),
Index: main/density-buoyancy-common/js/common/view/GravityControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/density-buoyancy-common/js/common/view/GravityControlNode.ts b/main/density-buoyancy-common/js/common/view/GravityControlNode.ts
--- a/main/density-buoyancy-common/js/common/view/GravityControlNode.ts (revision 329d94f227a28a0c893cd67642b4df0ac6bf71af)
+++ b/main/density-buoyancy-common/js/common/view/GravityControlNode.ts (date 1665187752286)
@@ -44,7 +44,7 @@
].map( gravity => {
return {
value: gravity,
- node: new Text( gravity.name, {
+ createNode: tandem => new Text( gravity.name, {
font: DensityBuoyancyCommonConstants.COMBO_BOX_ITEM_FONT,
maxWidth: 160
} ),
Index: main/area-model-common/js/generic/view/GenericLayoutSelectionNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/area-model-common/js/generic/view/GenericLayoutSelectionNode.js b/main/area-model-common/js/generic/view/GenericLayoutSelectionNode.js
--- a/main/area-model-common/js/generic/view/GenericLayoutSelectionNode.js (revision 5f7ec8a298030f875aa747b1499c135f273cf05a)
+++ b/main/area-model-common/js/generic/view/GenericLayoutSelectionNode.js (date 1665188875013)
@@ -32,7 +32,7 @@
width -= 1;
const comboBoxItems = GenericLayout.VALUES.map( layout => ( {
- node: new HBox( {
+ createNode: tandem => new HBox( {
children: [
createLayoutIcon( layout.size, 0.7 ),
new Text( `${layout.size.height}x${layout.size.width}`, {
Index: main/models-of-the-hydrogen-atom/js/common/view/DeBroglieViewComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/models-of-the-hydrogen-atom/js/common/view/DeBroglieViewComboBox.ts b/main/models-of-the-hydrogen-atom/js/common/view/DeBroglieViewComboBox.ts
--- a/main/models-of-the-hydrogen-atom/js/common/view/DeBroglieViewComboBox.ts (revision a3b927c6583ff9d663b77a3f29f2b649e8fa83c4)
+++ b/main/models-of-the-hydrogen-atom/js/common/view/DeBroglieViewComboBox.ts (date 1665187507201)
@@ -55,9 +55,9 @@
}, textOptions ) );
const items: ComboBoxItem<DeBroglieView>[] = [
- { value: 'radial', node: radialViewText, tandemName: `radial${ComboBox.ITEM_TANDEM_NAME_SUFFIX}` },
- { value: '3D', node: threeDViewText, tandemName: `3D${ComboBox.ITEM_TANDEM_NAME_SUFFIX}` },
- { value: 'brightness', node: brightnessViewText, tandemName: `brightness${ComboBox.ITEM_TANDEM_NAME_SUFFIX}` }
+ { value: 'radial', createNode: tandem => radialViewText, tandemName: `radial${ComboBox.ITEM_TANDEM_NAME_SUFFIX}` },
+ { value: '3D', createNode: tandem => threeDViewText, tandemName: `3D${ComboBox.ITEM_TANDEM_NAME_SUFFIX}` },
+ { value: 'brightness', createNode: tandem => brightnessViewText, tandemName: `brightness${ComboBox.ITEM_TANDEM_NAME_SUFFIX}` }
];
super( deBroglieViewProperty, items, listboxParent, options );
Index: main/scenery-phet/js/ComboBoxDisplay.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery-phet/js/ComboBoxDisplay.ts b/main/scenery-phet/js/ComboBoxDisplay.ts
--- a/main/scenery-phet/js/ComboBoxDisplay.ts (revision ce185329efd1931c873ba7b91e3c298b8e70d343)
+++ b/main/scenery-phet/js/ComboBoxDisplay.ts (date 1665187790227)
@@ -108,8 +108,8 @@
comboBoxItems.push( {
value: item.choice,
- node: itemNode,
- tandemName: item.tandemName || null
+ createNode: tandem => itemNode,
+ tandemName: item.tandemName
} );
} );
Index: main/sun/js/ComboBoxListBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ComboBoxListBox.ts b/main/sun/js/ComboBoxListBox.ts
--- a/main/sun/js/ComboBoxListBox.ts (revision 15be3e7ec7b6b9ad89936a0f6ecdbf2e23885ea9)
+++ b/main/sun/js/ComboBoxListBox.ts (date 1665189248787)
@@ -8,7 +8,7 @@
import PhetioAction from '../../tandem/js/PhetioAction.js';
import optionize, { combineOptions } from '../../phet-core/js/optionize.js';
-import { KeyboardUtils, SceneryEvent, SpeakingOptions, TInputListener, TPaint, VBox, VoicingNode } from '../../scenery/js/imports.js';
+import { KeyboardUtils, SceneryEvent, SpeakingOptions, TInputListener, TPaint, VBox, VoicingNode, Node } from '../../scenery/js/imports.js';
import multiSelectionSoundPlayerFactory from '../../tambo/js/multiSelectionSoundPlayerFactory.js';
import generalCloseSoundPlayer from '../../tambo/js/shared-sound-players/generalCloseSoundPlayer.js';
import generalOpenSoundPlayer from '../../tambo/js/shared-sound-players/generalOpenSoundPlayer.js';
@@ -52,13 +52,14 @@
/**
* @param property
* @param items
+ * @param nodes
* @param hideListBoxCallback - called to hide the list box
* @param focusButtonCallback - called to transfer focus to the combo box's button
* @param voiceOnSelectionNode - Node to voice the response when selecting a combo box item.
* @param tandem
* @param providedOptions
*/
- public constructor( property: TProperty<T>, items: ComboBoxItem<T>[], hideListBoxCallback: () => void,
+ public constructor( property: TProperty<T>, items: ComboBoxItem<T>[], nodes: Node[], hideListBoxCallback: () => void,
focusButtonCallback: () => void, voiceOnSelectionNode: VoicingNode, tandem: Tandem, providedOptions?: ComboBoxListBoxOptions ) {
const options = optionize<ComboBoxListBoxOptions, SelfOptions, PanelOptions>()( {
@@ -140,8 +141,8 @@
};
// Compute max item size
- const maxItemWidthProperty = ComboBox.getMaxItemWidthProperty( items );
- const maxItemHeightProperty = ComboBox.getMaxItemHeightProperty( items );
+ const maxItemWidthProperty = ComboBox.getMaxItemWidthProperty( nodes );
+ const maxItemHeightProperty = ComboBox.getMaxItemHeightProperty( nodes );
// Uniform dimensions for all highlighted items in the list, highlight overlaps margin by 50%
const highlightWidthProperty = new DerivedProperty( [ maxItemWidthProperty ], width => width + options.xMargin );
@@ -152,7 +153,7 @@
items.forEach( ( item, index ) => {
// Create the list item node
- const listItemNode = new ComboBoxListItemNode( item, highlightWidthProperty, highlightHeightProperty,
+ const listItemNode = new ComboBoxListItemNode( item, nodes[ index ], highlightWidthProperty, highlightHeightProperty,
combineOptions<ComboBoxListItemNodeOptions>( {
align: options.align,
highlightFill: options.highlightFill,
Index: main/sun/js/demo/DemosScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/demo/DemosScreenView.ts b/main/sun/js/demo/DemosScreenView.ts
--- a/main/sun/js/demo/DemosScreenView.ts (revision 15be3e7ec7b6b9ad89936a0f6ecdbf2e23885ea9)
+++ b/main/sun/js/demo/DemosScreenView.ts (date 1665187541029)
@@ -110,7 +110,7 @@
const items = demos.map( ( demo: SunDemo ) => {
return {
value: demo,
- node: new Text( demo.label, { font: COMBO_BOX_ITEM_FONT } )
+ createNode: ( tandem: Tandem ) => new Text( demo.label, { font: COMBO_BOX_ITEM_FONT } )
};
} );
Index: main/molecule-polarity/js/realmolecules/view/RealMoleculesComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/molecule-polarity/js/realmolecules/view/RealMoleculesComboBox.ts b/main/molecule-polarity/js/realmolecules/view/RealMoleculesComboBox.ts
--- a/main/molecule-polarity/js/realmolecules/view/RealMoleculesComboBox.ts (revision ed6cabedc7b4577fd21e25f7c038ca4267ebe3eb)
+++ b/main/molecule-polarity/js/realmolecules/view/RealMoleculesComboBox.ts (date 1665187507192)
@@ -70,7 +70,7 @@
return {
value: molecule,
- node: node,
+ createNode: tandem => node,
tandemName: `${molecule.tandem.name}${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
};
}
Index: main/sun/js/ComboBoxListItemNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ComboBoxListItemNode.ts b/main/sun/js/ComboBoxListItemNode.ts
--- a/main/sun/js/ComboBoxListItemNode.ts (revision 15be3e7ec7b6b9ad89936a0f6ecdbf2e23885ea9)
+++ b/main/sun/js/ComboBoxListItemNode.ts (date 1665189232674)
@@ -44,7 +44,7 @@
public readonly item: ComboBoxItem<T>;
- public constructor( item: ComboBoxItem<T>, highlightWidthProperty: TReadOnlyProperty<number>, highlightHeightProperty: TReadOnlyProperty<number>, providedOptions?: ComboBoxListItemNodeOptions ) {
+ public constructor( item: ComboBoxItem<T>, node: Node, highlightWidthProperty: TReadOnlyProperty<number>, highlightHeightProperty: TReadOnlyProperty<number>, providedOptions?: ComboBoxListItemNodeOptions ) {
const options = optionize<ComboBoxListItemNodeOptions, SelfOptions, ParentOptions>()( {
@@ -98,7 +98,7 @@
// Wrapper for the item's Node. Do not transform item.node because it is shared with ComboBoxButton!
const itemNodeWrapper = new Node( {
- children: [ item.node ]
+ children: [ node ]
} );
highlightWidthProperty.link( width => {
Index: main/beers-law-lab/js/beerslaw/view/SolutionComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/beers-law-lab/js/beerslaw/view/SolutionComboBox.ts b/main/beers-law-lab/js/beerslaw/view/SolutionComboBox.ts
--- a/main/beers-law-lab/js/beerslaw/view/SolutionComboBox.ts (revision d7631720b8b880d82ed6b96d3180fe6cf553d9a2)
+++ b/main/beers-law-lab/js/beerslaw/view/SolutionComboBox.ts (date 1665187371690)
@@ -85,7 +85,7 @@
return {
value: solution,
- node: hBox,
+ createNode: tandem => hBox,
tandemName: `${solution.tandemName}${ComboBox.ITEM_TANDEM_NAME_SUFFIX}`
};
}
Index: main/sun/js/ComboBoxButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ComboBoxButton.ts b/main/sun/js/ComboBoxButton.ts
--- a/main/sun/js/ComboBoxButton.ts (revision 15be3e7ec7b6b9ad89936a0f6ecdbf2e23885ea9)
+++ b/main/sun/js/ComboBoxButton.ts (date 1665189159345)
@@ -66,7 +66,7 @@
private arrow: Path;
private vSeparator: VSeparatorDeprecated;
- public constructor( property: TProperty<T>, items: ComboBoxItem<T>[], providedOptions?: ComboBoxButtonOptions ) {
+ public constructor( property: TProperty<T>, items: ComboBoxItem<T>[], nodes: Node[], providedOptions?: ComboBoxButtonOptions ) {
const options = optionize<ComboBoxButtonOptions, SelfOptions, RectangularPushButtonOptions>()( {
@@ -113,8 +113,8 @@
const itemYMargin = options.yMargin;
// Compute max item size
- const maxItemWidthProperty = ComboBox.getMaxItemWidthProperty( items );
- const maxItemHeightProperty = ComboBox.getMaxItemHeightProperty( items );
+ const maxItemWidthProperty = ComboBox.getMaxItemWidthProperty( nodes );
+ const maxItemHeightProperty = ComboBox.getMaxItemHeightProperty( nodes );
const arrow = new Path( null, {
fill: options.arrowFill
@@ -122,6 +122,10 @@
// Wrapper for the selected item's Node.
// Do not transform ComboBoxItem.node because it is shared with ComboBoxListItemNode.
+
+
+ const matchingItem = _.find( items, item => item.value === property.value )!;
+ const index = items.indexOf( matchingItem );
const itemNodeWrapper = new Node( {
layoutOptions: {
yMargin: itemYMargin,
@@ -129,7 +133,7 @@
xAlign: options.align
},
children: [
- _.find( items, item => item.value === property.value )!.node
+ nodes[ index ]
]
} );
@@ -236,9 +240,11 @@
// 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( item.node );
+ itemNodeWrapper.addChild( node );
// pdom
this.innerContent = ( item.a11yLabel || null );
Index: main/ratio-and-proportion/js/create/view/TickMarkRangeComboBoxNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/ratio-and-proportion/js/create/view/TickMarkRangeComboBoxNode.ts b/main/ratio-and-proportion/js/create/view/TickMarkRangeComboBoxNode.ts
--- a/main/ratio-and-proportion/js/create/view/TickMarkRangeComboBoxNode.ts (revision 27b938fa83e1ed93c75233316dca102f5e97fe93)
+++ b/main/ratio-and-proportion/js/create/view/TickMarkRangeComboBoxNode.ts (date 1665187915471)
@@ -35,7 +35,7 @@
private tickMarkRangeProperty: Property<number>;
public constructor( tickMarkRangeProperty: Property<number>, comboBoxParent: Node,
- tickMarkViewProperty: EnumerationProperty<TickMarkView> ) {
+ tickMarkViewProperty: EnumerationProperty<TickMarkView> ) {
super();
this.tickMarkRangeMap = {
@@ -46,12 +46,13 @@
this.tickMarkRangeProperty = tickMarkRangeProperty;
const items = [
- { value: 10, node: new RichText( this.tickMarkRangeMap[ 10 ], RANGE_TEXT_OPTIONS ), a11yLabel: RatioAndProportionStrings.zeroToTen },
- { value: 20, node: new RichText( this.tickMarkRangeMap[ 20 ], RANGE_TEXT_OPTIONS ), a11yLabel: RatioAndProportionStrings.zeroToTwenty },
- { value: 30, node: new RichText( this.tickMarkRangeMap[ 30 ], RANGE_TEXT_OPTIONS ), a11yLabel: RatioAndProportionStrings.zeroToThirty }
+ { value: 10, createNode: ( tandem: Tandem ) => new RichText( this.tickMarkRangeMap[ 10 ], RANGE_TEXT_OPTIONS ), a11yLabel: RatioAndProportionStrings.zeroToTen },
+ { value: 20, createNode: ( tandem: Tandem ) => new RichText( this.tickMarkRangeMap[ 20 ], RANGE_TEXT_OPTIONS ), a11yLabel: RatioAndProportionStrings.zeroToTwenty },
+ { value: 30, createNode: ( tandem: Tandem ) => new RichText( this.tickMarkRangeMap[ 30 ], RANGE_TEXT_OPTIONS ), a11yLabel: RatioAndProportionStrings.zeroToThirty }
];
- const widestItem = Math.max( ...items.map( item => item.node.width ) );
+ // TODO: https://github.com/phetsims/sun/issues/797 is this OK?
+ const widestItem = Math.max( ...items.map( item => item.createNode( Tandem.OPT_OUT ).width ) );
const comboBoxOptions = {
labelNode: new RichText( RatioAndProportionStrings.rangeStringProperty, RANGE_TEXT_OPTIONS ),
@@ -73,7 +74,7 @@
// NOTE: The values are [ 10, true ]... so it's typed interestingly.
this.disabledComboBox = new ComboBox<true | number>( new BooleanProperty( value ) as Property<true | number>, [
- { value: value, node: new HSeparatorDeprecated( widestItem, { centerY: -5 } ), a11yLabel: RatioAndProportionStrings.a11y.tickMark.tickMarksHidden },
+ { value: value, createNode: tandem => new HSeparatorDeprecated( widestItem, { centerY: -5 } ), a11yLabel: RatioAndProportionStrings.a11y.tickMark.tickMarksHidden },
items[ 0 ] // add this one to get the proper height of the text.
], new Node(), comboBoxOptions );
Index: main/tambo/js/demo/testing/view/RemoveAndDisposeSoundGeneratorsTestPanel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/tambo/js/demo/testing/view/RemoveAndDisposeSoundGeneratorsTestPanel.ts b/main/tambo/js/demo/testing/view/RemoveAndDisposeSoundGeneratorsTestPanel.ts
--- a/main/tambo/js/demo/testing/view/RemoveAndDisposeSoundGeneratorsTestPanel.ts (revision 5fa44dd369d0e620cf872d4438cae456cdb81ade)
+++ b/main/tambo/js/demo/testing/view/RemoveAndDisposeSoundGeneratorsTestPanel.ts (date 1665187567732)
@@ -89,7 +89,7 @@
SOUND_GENERATOR_INFO.forEach( ( soundGenerator, soundGeneratorKey ) => {
comboBoxItems.push( {
value: soundGeneratorKey,
- node: new Text( soundGenerator.comboBoxItemName, { font: COMBO_BOX_FONT } )
+ createNode: tandem => new Text( soundGenerator.comboBoxItemName, { font: COMBO_BOX_FONT } )
} );
} );
const selectedSoundGeneratorTypeProperty = new Property( comboBoxItems[ 0 ].value );
Index: main/sun/js/CarouselComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/CarouselComboBox.ts b/main/sun/js/CarouselComboBox.ts
--- a/main/sun/js/CarouselComboBox.ts (revision 15be3e7ec7b6b9ad89936a0f6ecdbf2e23885ea9)
+++ b/main/sun/js/CarouselComboBox.ts (date 1665188968090)
@@ -33,6 +33,7 @@
import PageControl, { PageControlOptions } from './PageControl.js';
import sun from './sun.js';
import { ComboBoxItem } from './ComboBox.js';
+import { getGroupItemNodes } from './GroupItemOptions.js';
type SelfOptions = {
@@ -113,9 +114,12 @@
// Make items in the carousel have the same width and height.
const alignGroup = new AlignGroup();
+ const nodes = getGroupItemNodes( comboBoxItems, options.tandem.createTandem( 'items' ) );
+
// Create items for the carousel, whose API for 'items' is different than ComboBox.
+ let i = 0;
const carouselItemNodes = _.map( comboBoxItems,
- comboBoxItem => new CarouselItemNode( property, comboBoxItem, alignGroup, options.itemNodeOptions )
+ comboBoxItem => new CarouselItemNode( property, comboBoxItem, nodes[ i++ ], alignGroup, options.itemNodeOptions )
);
assert && assert( carouselItemNodes.length === comboBoxItems.length, 'expected a carouselItem for each comboBoxItem' );
@@ -141,7 +145,7 @@
} );
// Pressing this button pops the carousel up and down
- const button = new ComboBoxButton( property, comboBoxItems, combineOptions<ComboBoxButtonOptions>( {
+ const button = new ComboBoxButton( property, comboBoxItems, nodes, combineOptions<ComboBoxButtonOptions>( {
listener: () => {
carouselAndPageControl.visible = !carouselAndPageControl.visible;
},
@@ -239,7 +243,7 @@
private readonly disposeCarouselItemNode: () => void;
- public constructor( property: TProperty<T>, comboBoxItem: ComboBoxItem<T>, alignGroup: AlignGroup, providedOptions?: CarouselItemNodeOptions ) {
+ public constructor( property: TProperty<T>, comboBoxItem: ComboBoxItem<T>, node: Node, alignGroup: AlignGroup, providedOptions?: CarouselItemNodeOptions ) {
const options = optionize<CarouselItemNodeOptions, CarouselItemNodeSelfOptions, NodeOptions>()( {
@@ -254,7 +258,7 @@
tandem: Tandem.OPTIONAL
}, providedOptions );
- const uniformNode = new AlignBox( comboBoxItem.node, {
+ const uniformNode = new AlignBox( node, {
xAlign: options.align,
group: alignGroup
} );
Index: main/sun/js/demo/components/demoComboBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/demo/components/demoComboBox.ts b/main/sun/js/demo/components/demoComboBox.ts
--- a/main/sun/js/demo/components/demoComboBox.ts (revision 15be3e7ec7b6b9ad89936a0f6ecdbf2e23885ea9)
+++ b/main/sun/js/demo/components/demoComboBox.ts (date 1665187752290)
@@ -20,7 +20,7 @@
values.forEach( value => {
items.push( {
value: value,
- node: new Text( value, { font: FONT } )
+ createNode: tandem => new Text( value, { font: FONT } )
} );
} );
Index: main/my-solar-system/js/common/view/MySolarSystemControls.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/my-solar-system/js/common/view/MySolarSystemControls.ts b/main/my-solar-system/js/common/view/MySolarSystemControls.ts
--- a/main/my-solar-system/js/common/view/MySolarSystemControls.ts (revision a19f21b3cdbb060cc351967190427f2d3baa0085)
+++ b/main/my-solar-system/js/common/view/MySolarSystemControls.ts (date 1665187507188)
@@ -66,18 +66,18 @@
model.isLab
? [
new ComboBox( model.labModeProperty, [
- { value: LabModes.SUN_PLANET, node: new Text( MySolarSystemStrings.mode.sunAndPlanetStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.SUN_PLANET_MOON, node: new Text( MySolarSystemStrings.mode.sunPlanetAndMoonStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.SUN_PLANET_COMET, node: new Text( MySolarSystemStrings.mode.sunPlanetAndCometStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.TROJAN_ASTEROIDS, node: new Text( MySolarSystemStrings.mode.trojanAsteroidsStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.ELLIPSES, node: new Text( MySolarSystemStrings.mode.ellipsesStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.HYPERBOLIC, node: new Text( MySolarSystemStrings.mode.hyperbolicStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.SLINGSHOT, node: new Text( MySolarSystemStrings.mode.slingshotStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.DOUBLE_SLINGSHOT, node: new Text( MySolarSystemStrings.mode.doubleSlingshotStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.BINARY_STAR_PLANET, node: new Text( MySolarSystemStrings.mode.binaryStarPlanetStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.FOUR_STAR_BALLET, node: new Text( MySolarSystemStrings.mode.fourStarBalletStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.DOUBLE_DOUBLE, node: new Text( MySolarSystemStrings.mode.doubleDoubleStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
- { value: LabModes.CUSTOM, node: new Text( MySolarSystemStrings.mode.customStringProperty, COMBO_BOX_TEXT_OPTIONS ) }
+ { value: LabModes.SUN_PLANET, createNode: tandem => new Text( MySolarSystemStrings.mode.sunAndPlanetStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.SUN_PLANET_MOON, createNode: tandem => new Text( MySolarSystemStrings.mode.sunPlanetAndMoonStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.SUN_PLANET_COMET, createNode: tandem => new Text( MySolarSystemStrings.mode.sunPlanetAndCometStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.TROJAN_ASTEROIDS, createNode: tandem => new Text( MySolarSystemStrings.mode.trojanAsteroidsStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.ELLIPSES, createNode: tandem => new Text( MySolarSystemStrings.mode.ellipsesStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.HYPERBOLIC, createNode: tandem => new Text( MySolarSystemStrings.mode.hyperbolicStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.SLINGSHOT, createNode: tandem => new Text( MySolarSystemStrings.mode.slingshotStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.DOUBLE_SLINGSHOT, createNode: tandem => new Text( MySolarSystemStrings.mode.doubleSlingshotStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.BINARY_STAR_PLANET, createNode: tandem => new Text( MySolarSystemStrings.mode.binaryStarPlanetStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.FOUR_STAR_BALLET, createNode: tandem => new Text( MySolarSystemStrings.mode.fourStarBalletStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.DOUBLE_DOUBLE, createNode: tandem => new Text( MySolarSystemStrings.mode.doubleDoubleStringProperty, COMBO_BOX_TEXT_OPTIONS ) },
+ { value: LabModes.CUSTOM, createNode: tandem => new Text( MySolarSystemStrings.mode.customStringProperty, COMBO_BOX_TEXT_OPTIONS ) }
], topLayer, {
tandem: providedOptions.tandem.createTandem( 'labModeComboBox' )
} )
Index: main/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts b/main/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
--- a/main/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts (revision 27b938fa83e1ed93c75233316dca102f5e97fe93)
+++ b/main/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts (date 1665187541033)
@@ -149,7 +149,7 @@
return {
value: targetRatio,
- node: node,
+ createNode: tandem => node,
soundPlayer: challengeInfo.soundClip,
a11yLabel: challengeInfo.a11yLabel,
tandemName: challengeInfo.tandemName
|
I'd like to work on #798 first, going on hold until then. |
Today I implicated myself in this issue. @samreid is likely going to be converting Carousel to use GroupItemOptions, and we may want to do this work as part of that work and/or BLL which uses combo box. This would be especially nice if we felt like PhET-iO api would change, but @samreid and I don't really think that it would. |
Addressed in #814. UPDATE: I thought this was a carousel issue. No progress on this committed yet. |
I'm taking a stab at continuing this from our momentum in Carousel. I have a good local copy and I'm interested in keeping backwards compatibility with item.node on master so we can phase out the old pattern instead of doing things all in one commit. It only took 2 lines of code to make that happen, and now I'm running pixel comparisons to ensure regressionless behavior. I'll keep you posted! |
Good progress here, I was able to get all of the |
Oops, I had stashed the ESP changes for a snapshot comparison, here they are now. |
CT is looking mighty clean today. @samreid will you please take a look at the usages of group item options in ComboBox, and the commits in CarouselComboBox, as well as spot checking some of the usages. I can't think of much else here. |
I reviewed the code in ComboBox and CarouselComboBox, and checked some usages. I tested // assert && assert( !item.hasOwnProperty( 'node' ), 'we use createNode now' ); |
Ahh yes thanks. This is redundant to Line 31 in cb5784e
Closing |
After working on #787, I'm thinking we should apply the same pattern (createNode + tandemName) for ComboBoxItems. @pixelzoom does that sound good to you? If so, please assign to me and I can take the lead.
The text was updated successfully, but these errors were encountered: