Skip to content

Commit

Permalink
better long term solution for setting a min width for displays in com…
Browse files Browse the repository at this point in the history
…bo box, see #256
  • Loading branch information
zepumph committed Jan 13, 2023
1 parent 4d5da29 commit fbff459
Showing 1 changed file with 12 additions and 13 deletions.
25 changes: 12 additions & 13 deletions js/common/view/ThermometerAndReadout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ class ThermometerAndReadout extends Node {
tandem: options.tandem.createTandem( 'comboBox' )
} );
this.addChild( comboBox );

// Make sure the NumberDisplays are all the same width, https://github.com/phetsims/greenhouse-effect/issues/256
const maxItemWidth = Math.max( ..._.map( comboBox.nodes, 'width' ) );
comboBox.nodes.forEach( ( node: Node ) => {
if ( node instanceof NumberDisplay ) {
node.setBackgroundWidth( maxItemWidth );
}
} );
}
else {

Expand Down Expand Up @@ -212,14 +220,6 @@ class ThermometerAndReadout extends Node {
propertyValue: TemperatureUnits,
tandemName: string ): ComboBoxItem<TemperatureUnits> {

// Set a minimum background width for the number display. This is necessary because the nodes that go into the
// combo box must be the same size, or things can look weird, see
// https://github.com/phetsims/greenhouse-effect/issues/256. The approach of using a hard-coded constant is
// admittedly quite brittle, but because of the constraints of ComboBox, it's hard to figure out the sizes of the
// displays before creating them. So, hard coded it is. Be wary if reusing this. The value was empirically
// determined.
const minNumberDisplayBackgroundWidth = 68;

const numberDisplayOptions = {
backgroundStroke: null,
decimalPlaces: DECIMAL_PLACES_IN_READOUT,
Expand All @@ -228,15 +228,14 @@ class ThermometerAndReadout extends Node {
maxWidth: 120
},
valuePattern: new PatternStringProperty(
GreenhouseEffectStrings.temperature.units.valueUnitsPatternStringProperty,
{ units: unitsStringProperty }
),
minBackgroundWidth: minNumberDisplayBackgroundWidth
GreenhouseEffectStrings.temperature.units.valueUnitsPatternStringProperty, {
units: unitsStringProperty
} )
};

return {
value: propertyValue,
createNode: tandem => new NumberDisplay( property, propertyRange, numberDisplayOptions ),
createNode: () => new NumberDisplay( property, propertyRange, numberDisplayOptions ),
tandemName: tandemName,
a11yName: TemperatureDescriber.getTemperatureUnitsString( propertyValue )
};
Expand Down

0 comments on commit fbff459

Please sign in to comment.