LevelSelectionButton Layout bug with AlignBox and Decorator Pattern

[marlitas]

While working on phetsims/arithmetic#196 @Luisav1 and I were having issues with the level selection icons and layouts matching up. Upon inspecting the scenery helper we saw that an alignBox was pushing down our leftmost icon and causing other shifting behavior in the buttons.

We found that LevelSelectionButtonGroup was using an alignGroup to match the bounds of icons across all buttons. However, this was conflicting with the scaling logic in LevelSelectionButton

  public static createSizedImageNode( icon: Node, size: Dimension2 ): Node {
    icon.scale( Math.min( size.width / icon.bounds.width, size.height / icon.bounds.height ) );

This scaling logic accurately downsized icons to fit within the button sizes provided in options, however when the bounds of the alignBox were scaled down, that triggered an updateLayout in AlignGroup, that would then reset the bounds to match the largest height of the alignBoxes in the group. Since no other AlignBoxes had been visited yet the scaling was essentially reversed in this situation.

[marlitas]

@Luisav1 and I worked on this patch together that seems to solve the problem. We checked the before and after on arithmetic, BCE, area-model-algebra/multiplication, and vegas demos. We saw no changes here. There is still work to be done to address all uses of LevelSelectionButtonGroup to make sure options are updated.

Subject: [PATCH] remove align box from LevelSelectionButtonGroup.ts
---
Index: vegas/js/LevelSelectionButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vegas/js/LevelSelectionButton.ts b/vegas/js/LevelSelectionButton.ts
--- a/vegas/js/LevelSelectionButton.ts	(revision 775277a8a4b9afc30ee19529773fc6ab6ea34a5c)
+++ b/vegas/js/LevelSelectionButton.ts	(date 1701210571704)
@@ -58,6 +58,7 @@
 
 export type LevelSelectionButtonOptions = SelfOptions & StrictOmit<RectangularPushButtonOptions, 'content'>;
 
+export const DEFAULT_BUTTON_DIMENSION = 150;
 export default class LevelSelectionButton extends RectangularPushButton {
 
   private readonly disposeLevelSelectionButton: () => void;
Index: arithmetic/js/common/view/LevelSelectionNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/arithmetic/js/common/view/LevelSelectionNode.js b/arithmetic/js/common/view/LevelSelectionNode.js
--- a/arithmetic/js/common/view/LevelSelectionNode.js	(revision b5c7020c5225747a6d86f8071243fb3ee2d33687)
+++ b/arithmetic/js/common/view/LevelSelectionNode.js	(date 1701210571709)
@@ -11,8 +11,8 @@
 import ResetAllButton from '../../../../scenery-phet/js/buttons/ResetAllButton.js';
 import TimerToggleButton from '../../../../scenery-phet/js/buttons/TimerToggleButton.js';
 import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
-import { HBox, Node, Text, VBox } from '../../../../scenery/js/imports.js';
-import LevelSelectionButton from '../../../../vegas/js/LevelSelectionButton.js';
+import { Node, Text, VBox } from '../../../../scenery/js/imports.js';
+import LevelSelectionButtonGroup from '../../../../vegas/js/LevelSelectionButtonGroup.js';
 import ScoreDisplayStars from '../../../../vegas/js/ScoreDisplayStars.js';
 import arithmetic from '../../arithmetic.js';
 import ArithmeticStrings from '../../ArithmeticStrings.js';
@@ -69,29 +69,35 @@
 
     // add select level buttons
     assert && assert( model.levelModels.length === iconSets[ options.iconSet ].length, 'Number of icons doesn\'t match number of levels' );
-    const levelSelectButtons = model.levelModels.map( ( level, levelIndex ) => new LevelSelectionButton(
-      iconSets[ options.iconSet ][ levelIndex ],
-      model.levelModels[ levelIndex ].displayScoreProperty,
-      {
-        buttonWidth: BUTTON_LENGTH,
-        buttonHeight: BUTTON_LENGTH,
-        baseColor: options.buttonBaseColor,
-        bestTimeProperty: model.levelModels[ levelIndex ].bestTimeProperty,
-        bestTimeVisibleProperty: ArithmeticGlobals.timerEnabledProperty,
-        listener: () => {
-          callback( levelIndex );
-        },
-        createScoreDisplay: scoreProperty => new ScoreDisplayStars( scoreProperty, {
-          numberOfStars: ArithmeticConstants.NUM_STARS,
-          perfectScore: level.perfectScore
-        } ),
-        soundPlayerIndex: levelIndex
+    const levelSelectButtons = model.levelModels.map( ( level, levelIndex ) => {
+      return {
+        icon: iconSets[ options.iconSet ][ levelIndex ],
+        scoreProperty: model.levelModels[ levelIndex ].displayScoreProperty,
+        options: {
+          baseColor: options.buttonBaseColor,
+          bestTimeVisibleProperty: ArithmeticGlobals.timerEnabledProperty,
+          bestTimeProperty: model.levelModels[ levelIndex ].bestTimeProperty,
+          listener: () => {
+            callback( levelIndex );
+          },
+          createScoreDisplay: scoreProperty => new ScoreDisplayStars( scoreProperty, {
+            numberOfStars: ArithmeticConstants.NUM_STARS,
+            perfectScore: level.perfectScore
+          } ),
+          soundPlayerIndex: levelIndex
+        }
+      };
+    } );
+    const levelSelectionButtonGroup = new LevelSelectionButtonGroup( levelSelectButtons, {
+      groupButtonHeight: BUTTON_LENGTH,
+      groupButtonWidth: BUTTON_LENGTH,
+      flowBoxOptions: {
+        spacing: 50,
+        top: chooseLevelTitle.bottom + 15,
+        centerX: chooseLevelTitle.centerX
       }
-    ) );
-    const selectLevelButtonsHBox = new HBox( { spacing: 50, children: levelSelectButtons } );
-    selectLevelButtonsHBox.top = chooseLevelTitle.bottom + 15;
-    selectLevelButtonsHBox.centerX = chooseLevelTitle.centerX;
-    this.addChild( selectLevelButtonsHBox );
+    } );
+    this.addChild( levelSelectionButtonGroup );
 
     // add timer and sound buttons
     const soundAndTimerButtons = new VBox( {
Index: vegas/js/demo/components/demoLevelSelectionButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vegas/js/demo/components/demoLevelSelectionButtonGroup.ts b/vegas/js/demo/components/demoLevelSelectionButtonGroup.ts
--- a/vegas/js/demo/components/demoLevelSelectionButtonGroup.ts	(revision 775277a8a4b9afc30ee19529773fc6ab6ea34a5c)
+++ b/vegas/js/demo/components/demoLevelSelectionButtonGroup.ts	(date 1701210652340)
@@ -85,13 +85,13 @@
     super( items, {
       levelSelectionButtonOptions: {
         baseColor: 'pink',
-        buttonWidth: 100,
-        buttonHeight: 100,
         bestTimeFont: new PhetFont( 18 )
       },
       flowBoxOptions: {
         spacing: 30
       },
+      groupButtonHeight: 100,
+      groupButtonWidth: 100,
       tandem: Tandem.OPT_OUT
     } );
   }
@@ -142,8 +142,6 @@
     super( items, {
       levelSelectionButtonOptions: {
         baseColor: 'lightGreen',
-        buttonWidth: buttonWidth,
-        buttonHeight: buttonHeight,
         lineWidth: buttonLineWidth,
         bestTimeFont: new PhetFont( 18 )
       },
@@ -154,6 +152,8 @@
         wrap: true, // start a new row when preferredWidth is reached
         justify: 'center' // horizontal justification
       },
+      groupButtonHeight: buttonHeight,
+      groupButtonWidth: buttonWidth,
       tandem: Tandem.OPT_OUT
     } );
   }
@@ -188,10 +188,10 @@
 
     super( items, {
       levelSelectionButtonOptions: {
-        baseColor: 'orange',
-        buttonWidth: 75,
-        buttonHeight: 75
+        baseColor: 'orange'
       },
+      groupButtonWidth: 75,
+      groupButtonHeight: 75,
 
       // Create a custom layout, not possible via the default FlowBox and flowBoxOptions.
       createLayoutNode: ( buttons: LevelSelectionButton[] ) => {
Index: vegas/js/LevelSelectionButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vegas/js/LevelSelectionButtonGroup.ts b/vegas/js/LevelSelectionButtonGroup.ts
--- a/vegas/js/LevelSelectionButtonGroup.ts	(revision 775277a8a4b9afc30ee19529773fc6ab6ea34a5c)
+++ b/vegas/js/LevelSelectionButtonGroup.ts	(date 1701210571721)
@@ -21,10 +21,11 @@
 import PickRequired from '../../phet-core/js/types/PickRequired.js';
 import optionize, { combineOptions } from '../../phet-core/js/optionize.js';
 import { AlignBox, AlignGroup, FlowBox, FlowBoxOptions, LayoutNode, Node, NodeLayoutConstraint, NodeOptions } from '../../scenery/js/imports.js';
-import LevelSelectionButton, { LevelSelectionButtonOptions } from './LevelSelectionButton.js';
+import LevelSelectionButton, { DEFAULT_BUTTON_DIMENSION, LevelSelectionButtonOptions } from './LevelSelectionButton.js';
 import TProperty from '../../axon/js/TProperty.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import vegas from './vegas.js';
+import tandem from '../../tandem/js/Tandem.js';
 
 // Describes one LevelSelectionButton
 export type LevelSelectionButtonGroupItem = {
@@ -40,14 +41,14 @@
 
   // Options for the button. These will override LevelSelectionButtonGroupOptions.levelSelectionButtonOptions.
   // Setting tandem is the responsibility of the group, so it is omitted here.
-  options?: StrictOmit<LevelSelectionButtonOptions, 'tandem'>;
+  options?: StrictOmit<LevelSelectionButtonOptions, 'tandem' | 'buttonHeight' | 'buttonWidth'>;
 };
 
 type SelfOptions = {
 
   // Options for all LevelSelectionButton instances in the group.
   // These can be overridden for specific button(s) via LevelSelectionButtonGroupItem.options.
-  levelSelectionButtonOptions?: StrictOmit<LevelSelectionButtonOptions, 'tandem'>;
+  levelSelectionButtonOptions?: StrictOmit<LevelSelectionButtonOptions, 'tandem' | 'buttonHeight' | 'buttonWidth'>;
 
   // Options for the default layout, which is a FlowBox. Ignored if createLayoutNode is provided.
   flowBoxOptions?: StrictOmit<FlowBoxOptions, 'children'>;
@@ -60,6 +61,9 @@
   // query parameter. Set this to the value of the gameLevels query parameter, if supported by your sim.
   // See getGameLevelsSchema.ts.
   gameLevels?: number[];
+
+  groupButtonHeight?: number;
+  groupButtonWidth?: number;
 };
 
 export type LevelSelectionButtonGroupOptions = SelfOptions & StrictOmit<NodeOptions, 'children'> & PickRequired<NodeOptions, 'tandem'>;
@@ -79,7 +83,8 @@
 
     const options = optionize<LevelSelectionButtonGroupOptions,
       StrictOmit<SelfOptions, 'createLayoutNode' | 'gameLevels' | 'levelSelectionButtonOptions'>, NodeOptions>()( {
-
+      groupButtonHeight: DEFAULT_BUTTON_DIMENSION,
+      groupButtonWidth: DEFAULT_BUTTON_DIMENSION,
       // The default layout is a single row of buttons.
       flowBoxOptions: {
         orientation: 'horizontal',
@@ -89,11 +94,6 @@
       tandem: Tandem.REQUIRED
     }, providedOptions );
 
-    // All icons will have the same effective size.
-    const alignBoxOptions = {
-      group: new AlignGroup()
-    };
-
     // Create the LevelSelectionButton instances.
     const buttons = items.map( ( item, index ) => {
 
@@ -103,10 +103,13 @@
         tandem = options.tandem.createTandem( tandemName );
       }
 
-      return new LevelSelectionButton( new AlignBox( item.icon, alignBoxOptions ), item.scoreProperty,
-        combineOptions<LevelSelectionButtonOptions>( {
-          tandem: tandem
-        }, options.levelSelectionButtonOptions, item.options ) );
+      const buttonOptions = combineOptions<LevelSelectionButtonOptions>( {
+        buttonHeight: options.groupButtonHeight,
+        buttonWidth: options.groupButtonWidth,
+        tandem: tandem
+      }, options.levelSelectionButtonOptions, item.options );
+
+      return new LevelSelectionButton( item.icon, item.scoreProperty, buttonOptions );
     } );
 
     // Hide buttons for levels that are not included in gameLevels.
Index: balancing-chemical-equations/js/game/view/LevelSelectionNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts b/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts
--- a/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts	(revision a537d661a427843dd77ee8f6eb3872218dbad003)
+++ b/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts	(date 1701210571729)
@@ -59,14 +59,14 @@
     const buttonGroup = new LevelSelectionButtonGroup( buttonItems, {
       levelSelectionButtonOptions: {
         baseColor: '#d9ebff',
-        buttonWidth: 155,
-        buttonHeight: 155,
         bestTimeVisibleProperty: viewProperties.timerEnabledProperty
       },
       flowBoxOptions: {
         spacing: 50,
         center: layoutBounds.center
       },
+      groupButtonHeight: 155,
+      groupButtonWidth: 155,
       tandem: tandem.createTandem( 'buttonGroup' )
     } );
 

@Luisav1 mentioned she can keep working on using the new options to size buttons and make sure all buttons remain a consistent size.

@pixelzoom your review and input here would be greatly appreciated. Let me know if you would like to connect synchronously.

[Luisav1]

The commits above include the changes in the patch and also implement the new options in:

• Graphing Lines
• Fourier Making Waves
• Reactants Products and Leftovers

I checked their before and after to ensure the button sizes remained the same. I also checked Equality Explorer and Number Play as they use LevelSelectionButtonGroup too but nothing changed because they use the default button size.

[pixelzoom]

@marlitas said:

@pixelzoom your review and input here would be greatly appreciated. Let me know if you would like to connect synchronously.

I have a pile of high priority things I'm wading through. So it would be best to assign this to me, and/or schedule a time to chat on Zoom.

[marlitas]

Now that this is committed, I'll go ahead and assign to you as ready for review. This is currently not blocking for us and I imagine won't become a higher priority until next week at the earliest.

[pixelzoom]

@marlitas and I discussed this on Zoom.

The changes made so far are OK.

The remaining problems seem due to the fact that ButtonNodeConstraint (in ButtonNode) assume that content is all that it needs to be concerned about, and LevelSelectionButton decorates RectangularPushButton vis this.addChild( bestTimeNode ). This is a problem that I see in other UI components that have "content" - Panel, AccordionBox,... If addChild is not going to be supported by layout, then addChild should not be allowed.

To address LevelSelectionButton specifically, I would consider adding bestTimeNode as a child of content, rather than as a child of LevelSelectionButton. I would also clean up LevelSelectionButton to use VBox, etc. for layout, rather than the current implementation that relies on specifically handling translations for all of the subcomponents.

[marlitas]

I started to do some work here to address this issue, but the more I got into it the more it felt like I need to wait to design decisions are made in the design meeting next week. Below is a partial patch for if we decide to go the VBox route that keeps the current design intact, but I'm very much hoping we put the bestTimeNode inside the button instead.

Subject: [PATCH] remove decorator pattern
---
Index: vegas/js/LevelSelectionButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vegas/js/LevelSelectionButton.ts b/vegas/js/LevelSelectionButton.ts
--- a/vegas/js/LevelSelectionButton.ts	(revision bda60624e913f2ccd0c6ebdc377ba943bd986f7b)
+++ b/vegas/js/LevelSelectionButton.ts	(date 1702065199346)
@@ -13,10 +13,10 @@
 
 import TProperty from '../../axon/js/TProperty.js';
 import Dimension2 from '../../dot/js/Dimension2.js';
-import optionize from '../../phet-core/js/optionize.js';
+import optionize, { combineOptions } from '../../phet-core/js/optionize.js';
 import StrictOmit from '../../phet-core/js/types/StrictOmit.js';
 import PhetFont from '../../scenery-phet/js/PhetFont.js';
-import { Font, TColor, Node, Rectangle, Text } from '../../scenery/js/imports.js';
+import { Font, TColor, Node, Rectangle, Text, VBox, ManualConstraint, VBoxOptions } from '../../scenery/js/imports.js';
 import RectangularPushButton, { RectangularPushButtonOptions } from '../../sun/js/buttons/RectangularPushButton.js';
 import SoundClip from '../../tambo/js/sound-generators/SoundClip.js';
 import soundConstants from '../../tambo/js/soundConstants.js';
@@ -49,17 +49,18 @@
   bestTimeVisibleProperty?: TProperty<boolean> | null; // controls visibility of best time, null if no best time
   bestTimeFill?: TColor;
   bestTimeFont?: Font;
-  bestTimeYSpacing?: number;  // vertical space between drop shadow and best time
+  bestTimeYMargin?: number;  // vertical space between drop shadow and best time
 
   // Configures the soundPlayer for a specific game level. Note that this assumes zero-based indexing for game level,
   // which is often not the case. This option is ignored if RectangularPushButtonOptions.soundPlayer is provided.
   soundPlayerIndex?: number;
+  rectangularPushButtonOptions?: StrictOmit<RectangularPushButtonOptions, 'content'>;
 };
 
-export type LevelSelectionButtonOptions = SelfOptions & StrictOmit<RectangularPushButtonOptions, 'content'>;
+export type LevelSelectionButtonOptions = SelfOptions & StrictOmit<VBoxOptions, 'children'>;
 
 export const DEFAULT_BUTTON_DIMENSION = 150;
-export default class LevelSelectionButton extends RectangularPushButton {
+export default class LevelSelectionButton extends VBox {
 
   private readonly disposeLevelSelectionButton: () => void;
 
@@ -70,7 +71,7 @@
    */
   public constructor( icon: Node, scoreProperty: TProperty<number>, providedOptions?: LevelSelectionButtonOptions ) {
 
-    const options = optionize<LevelSelectionButtonOptions, SelfOptions, RectangularPushButtonOptions>()( {
+    const options = optionize<LevelSelectionButtonOptions, SelfOptions, VBoxOptions>()( {
 
       // SelfOptions
       buttonWidth: 150,
@@ -84,14 +85,16 @@
       bestTimeVisibleProperty: null,
       bestTimeFill: 'black',
       bestTimeFont: DEFAULT_BEST_TIME_FONT,
-      bestTimeYSpacing: 10,
+      bestTimeYMargin: 0,
+      spacing: 10,
+      soundPlayerIndex: 0,
 
-      // RectangularPushButton options
-      cornerRadius: 10,
-      baseColor: 'rgb( 242, 255, 204 )',
-      xMargin: 10,
-      yMargin: 10,
-      soundPlayerIndex: 0,
+      rectangularPushButtonOptions: {
+        cornerRadius: 10,
+        baseColor: 'rgb( 242, 255, 204 )',
+        xMargin: 10,
+        yMargin: 10
+      },
 
       // phet-io
       tandem: Tandem.REQUIRED
@@ -106,7 +109,7 @@
     // Background behind scoreDisplay
     const scoreDisplayBackgroundHeight = options.buttonHeight * options.scoreDisplayProportion;
     const scoreDisplayBackground = new Rectangle( 0, 0, maxContentWidth, scoreDisplayBackgroundHeight, {
-      cornerRadius: options.cornerRadius,
+      cornerRadius: options.rectangularPushButtonOptions.cornerRadius,
       fill: 'white',
       stroke: 'black',
       pickable: false
@@ -120,28 +123,33 @@
     const iconHeight = options.buttonHeight - scoreDisplayBackground.height - 2 * options.yMargin - options.iconToScoreDisplayYSpace;
     const iconSize = new Dimension2( maxContentWidth, iconHeight );
     const adjustedIcon = LevelSelectionButton.createSizedImageNode( icon, iconSize );
-    adjustedIcon.centerX = scoreDisplayBackground.centerX;
-    adjustedIcon.bottom = scoreDisplayBackground.top - options.iconToScoreDisplayYSpace;
+
+    const scoreDisplayNode = new Node( { children: [ scoreDisplayBackground, scoreDisplay ] } );
 
     // Keep scoreDisplay centered in its background when its bounds change
-    const scoreDisplayUpdateLayout = () => {
-      scoreDisplay.center = scoreDisplayBackground.center;
-    };
-    scoreDisplay.boundsProperty.lazyLink( scoreDisplayUpdateLayout );
-    scoreDisplayUpdateLayout();
-
-    options.content = new Node( {
-      children: [ adjustedIcon, scoreDisplayBackground, scoreDisplay ]
+    ManualConstraint.create( scoreDisplayNode, [ scoreDisplayBackground, scoreDisplay ],
+      ( backgroundProxy, displayProxy ) => {
+      displayProxy.center = backgroundProxy.center;
     } );
+
+    const rectangularPushButtonOptions = combineOptions<RectangularPushButtonOptions>( {
+      content: new VBox( {
+        children: [ adjustedIcon, scoreDisplayNode ]
+      } ),
+      layoutOptions: {
+        topMargin: options.bestTimeYMargin
+      }
+    }, options.rectangularPushButtonOptions );
+    const levelSelectionButton = new RectangularPushButton( rectangularPushButtonOptions );
 
     // If no sound player was provided, create the default.
-    if ( options.soundPlayer === undefined ) {
+    if ( options.rectangularPushButtonOptions.soundPlayer === undefined ) {
       const soundClip = new SoundClip( levelSelectionButton_mp3, {
         initialOutputLevel: 0.5,
         rateChangesAffectPlayingSounds: false
       } );
       soundManager.addSoundGenerator( soundClip, { categoryName: 'user-interface' } );
-      options.soundPlayer = {
+      options.rectangularPushButtonOptions.soundPlayer = {
         play() {
           soundClip.setPlaybackRate( Math.pow( soundConstants.TWELFTH_ROOT_OF_TWO, options.soundPlayerIndex ), 0 );
           soundClip.play();
@@ -152,7 +160,7 @@
       };
     }
 
-    super( options );
+    super( combineOptions<VBoxOptions>( { children: [ levelSelectionButton ] }, options ) );
 
     // Variables that are set if options.bestTimeProperty is specified.
     let bestTimeListener: ( ( bestTime: number | null ) => void ) | null = null;
@@ -164,16 +172,13 @@
       const bestTimeNode = new Text( '', {
         font: options.bestTimeFont,
         fill: options.bestTimeFill,
-        maxWidth: this.width // constrain to width of the push button
+        maxWidth: levelSelectionButton.width // constrain to width of the push button
       } );
-      const centerX = this.centerX;
-      bestTimeNode.top = this.bottom + options.bestTimeYSpacing;
       this.addChild( bestTimeNode );
 
       bestTimeListener = ( bestTime: number | null ) => {
         if ( bestTime !== null ) {
           bestTimeNode.string = ( bestTime ? GameTimer.formatTime( bestTime ) : '' );
-          bestTimeNode.centerX = centerX;
         }
       };
       options.bestTimeProperty.link( bestTimeListener );
Index: equality-explorer/js/solveit/view/SolveItLevelSelectionButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/equality-explorer/js/solveit/view/SolveItLevelSelectionButtonGroup.ts b/equality-explorer/js/solveit/view/SolveItLevelSelectionButtonGroup.ts
--- a/equality-explorer/js/solveit/view/SolveItLevelSelectionButtonGroup.ts	(revision 2e42b59ab8ff90cf3a0c026e1a4484c8ded6e6e7)
+++ b/equality-explorer/js/solveit/view/SolveItLevelSelectionButtonGroup.ts	(date 1702066239804)
@@ -33,7 +33,9 @@
       // LevelSelectionButtonGroupOptions
       isDisposable: false,
       levelSelectionButtonOptions: {
-        baseColor: 'rgb( 191, 239, 254 )'
+        rectangularPushButtonOptions: {
+          baseColor: 'rgb( 191, 239, 254 )'
+        }
       },
       flowBoxOptions: {
         spacing: 20
Index: balancing-chemical-equations/js/game/view/LevelSelectionNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts b/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts
--- a/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts	(revision d63ae9c6a3e2962c10fd0b4cff4cc243dd95893a)
+++ b/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts	(date 1702066239798)
@@ -50,7 +50,9 @@
             numberOfStars: model.getNumberOfEquations( level ),
             perfectScore: model.getPerfectScore( level )
           } ),
-          listener: () => startGame( level ),
+          rectangularPushButtonOptions: {
+            listener: () => startGame( level )
+          },
           soundPlayerIndex: level
         }
       } );
@@ -58,7 +60,9 @@
 
     const buttonGroup = new LevelSelectionButtonGroup( buttonItems, {
       levelSelectionButtonOptions: {
-        baseColor: '#d9ebff',
+        rectangularPushButtonOptions: {
+          baseColor: '#d9ebff'
+        },
         bestTimeVisibleProperty: viewProperties.timerEnabledProperty
       },
       flowBoxOptions: {
Index: fourier-making-waves/js/waveGame/view/WaveGameLevelSelectionButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/fourier-making-waves/js/waveGame/view/WaveGameLevelSelectionButtonGroup.ts b/fourier-making-waves/js/waveGame/view/WaveGameLevelSelectionButtonGroup.ts
--- a/fourier-making-waves/js/waveGame/view/WaveGameLevelSelectionButtonGroup.ts	(revision ab292fe1e300553c8ed20545011e0a01a1c555cf)
+++ b/fourier-making-waves/js/waveGame/view/WaveGameLevelSelectionButtonGroup.ts	(date 1702066239729)
@@ -44,7 +44,9 @@
 
     super( items, {
       levelSelectionButtonOptions: {
-        baseColor: FMWColors.levelSelectionButtonFillProperty
+        rectangularPushButtonOptions: {
+          baseColor: FMWColors.levelSelectionButtonFillProperty
+        }
       },
       groupButtonWidth: BUTTON_WIDTH,
       groupButtonHeight: BUTTON_HEIGHT,
Index: arithmetic/js/common/view/LevelSelectionNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/arithmetic/js/common/view/LevelSelectionNode.js b/arithmetic/js/common/view/LevelSelectionNode.js
--- a/arithmetic/js/common/view/LevelSelectionNode.js	(revision 81b4137f9da141c31e4cfe8c160c7d4331190bb5)
+++ b/arithmetic/js/common/view/LevelSelectionNode.js	(date 1702062337334)
@@ -86,7 +86,6 @@
       };
     } );
 
-    // TODO: Why is this not respecting invisible child bounds? https://github.com/phetsims/arithmetic/issues/199
     const levelSelectionButtonGroup = new LevelSelectionButtonGroup( levelSelectButtons, {
       groupButtonHeight: BUTTON_LENGTH,
       groupButtonWidth: BUTTON_LENGTH,

[pixelzoom]

@marlitas While you're dealing with layout of LevelSelectionButtonGroup ...

It would be preferrable to use GridBox instead of the current FlowBox. Sims that want need than one row of buttons currently need to resort to a hack, like this example in LineGameScreenView.ts for graphing-lines:

flowBoxOptions: {
  ...
  preferredWidth: 800, // set empirically to provide 3 buttons per row
  wrap: true,
  ...
}

It would be so much nicer (and more robust!) to do:

gridBoxOptions: {
  columns: 3
}

[marlitas]

It would be preferrable to use GridBox instead of the current FlowBox.

Completely agree. I'll add that into the work being done here. Should be easy to implement.

[marlitas]

Meeting 12/15/23

Time button layout:

• Inside the button
• A gray 0:00 should appear until the time is recorded
• 16 font size as the default (will need to check specific sim usages)
• Add the time below the stars and add clock icon to the right.
• If time gets turned off the display adjusts

In the end we decided to remove any notion of time from the LevelSelectionScreen. Therefore we no longer need to worry about this in terms of layout.

@pixelzoom edit for #130: According to Google calendar, the 12/15/23 meeting organizer was @marlitas, and the invitees were @amanda-phet @arouinfar @kathy-phet @ariel-phet @pixelzoom.

[pixelzoom]

In the end we decided to remove any notion of time from the LevelSelectionScreen. Therefore we no longer need to worry about this in terms of layout.

Clarification -- we decided to delete the time associated with each LevelSelectionButton. We did not decide to remove the optional time (clock) toggle button.

[pixelzoom]

Changes to LevelSelectionButton look good.

demoLevelSelectionButton and demoLevelSelectionButtonGroup were left with a lot of vestigial stuff related to best time. I cleaned things up in df37c81. Back to @marlitas to review, close if OK.

[marlitas]

Thanks for catching that @pixelzoom. Looks good to me. Closing!

[phet-dev]

Reopening because there is a TODO marked for this issue.

[pixelzoom]

@marlitas This issue was automatically reopened because there is a TODO referencing this issue in arithmetic LevelSelectionNode.js.

[marlitas]

Ah yes. Thank you phet-dev bot. Removed above.

[jonathanolson]

Reopening. The commit above (https://github.com/phetsims/phet-io-client-guides/commit/fb58a5cfb61bda5fcdd60aa696df1007e86a0543) put the energy-forms-and-changes 1.4 release branch into a broken state.

Looks like energy-forms-and-changes had a maintenance update on December 11th, but then December 15th or some time later, it looks like the commit was added into the energy-forms-and-changes-1.4 branch (https://github.com/phetsims/phet-io-client-guides/commits/energy-forms-and-changes-1.4/), but the dependencies.json was not updated in energy-forms-and-changes 1.4 branch (https://github.com/phetsims/energy-forms-and-changes/blob/1.4/dependencies.json).

Did the commit mean to hit the release branch? It's blocking my pending maintenance release.

[pixelzoom]

Sorry, I have no insight into why that commit was picked to energy-forms-and-changes 1.4.

[marlitas]

I also have no insight, and have no clue how that even happened in the first place without me explicitly doing so. I reverted the guilty commit. Sorry about the disruption here.

[jonathanolson]

Patched above! No worries.