phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

Use GridBox for LevelSelectionButtonGroup #121

Open marlitas opened 8 months ago

marlitas commented 8 months ago

I thought there was an issue tracking this work, but I can't seem to find it... perhaps it was only talked about in relation to https://github.com/phetsims/vegas/issues/120.

We would like to try to use GridBox as a way to lay out level selection buttons to simplify some of the logic.

marlitas commented 8 months ago

When comparing the published version of Fourier Making waves I noticed that the layout of how a 3 buttons per row max differs between the FlowBox implementation and the GridBox implementation.

GridBox:

image

FlowBox:

image

It is not possible to split buttons between columns like that with Gridbox, but I am unsure if that's a design constraint or not. I would like to check with @pixelzoom on this before moving forward. Patch below:

```diff Subject: [PATCH] Update credits, see: https://github.com/phetsims/qa/issues/1022 --- 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 b2861a7fafd33e7b07e9bb2870c2ecc7978fb5cb) +++ b/equality-explorer/js/solveit/view/SolveItLevelSelectionButtonGroup.ts (date 1704404580555) @@ -35,7 +35,7 @@ levelSelectionButtonOptions: { baseColor: 'rgb( 191, 239, 254 )' }, - flowBoxOptions: { + gridBoxOptions: { spacing: 20 }, gameLevels: EqualityExplorerQueryParameters.gameLevels Index: reactants-products-and-leftovers/js/game/view/RPALLevelSelectionButtonGroup.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/reactants-products-and-leftovers/js/game/view/RPALLevelSelectionButtonGroup.ts b/reactants-products-and-leftovers/js/game/view/RPALLevelSelectionButtonGroup.ts --- a/reactants-products-and-leftovers/js/game/view/RPALLevelSelectionButtonGroup.ts (revision 25faaf3f0e9545be68c5adddae5e4ac68dcba70f) +++ b/reactants-products-and-leftovers/js/game/view/RPALLevelSelectionButtonGroup.ts (date 1704404580528) @@ -66,7 +66,7 @@ }, groupButtonWidth: 150, groupButtonHeight: 150, - flowBoxOptions: { + gridBoxOptions: { spacing: 40 }, tandem: tandem Index: graphing-lines/js/linegame/view/LineGameLevelSelectionButtonGroup.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/graphing-lines/js/linegame/view/LineGameLevelSelectionButtonGroup.ts b/graphing-lines/js/linegame/view/LineGameLevelSelectionButtonGroup.ts --- a/graphing-lines/js/linegame/view/LineGameLevelSelectionButtonGroup.ts (revision 7a56881bbcb1485b111274754ece1f97e36544b3) +++ b/graphing-lines/js/linegame/view/LineGameLevelSelectionButtonGroup.ts (date 1704404580537) @@ -42,7 +42,7 @@ }, groupButtonWidth: 175, groupButtonHeight: 210, - flowBoxOptions: { + gridBoxOptions: { spacing: 50 } }, providedOptions ); 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 df6df186bc2eecbff9b63e13c20a6fd8bc35b956) +++ b/arithmetic/js/common/view/LevelSelectionNode.js (date 1704404580548) @@ -87,7 +87,7 @@ const levelSelectionButtonGroup = new LevelSelectionButtonGroup( levelSelectButtons, { groupButtonHeight: BUTTON_LENGTH, groupButtonWidth: BUTTON_LENGTH, - flowBoxOptions: { + gridBoxOptions: { spacing: 50 } } ); Index: graphing-lines/js/linegame/view/LineGameScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/graphing-lines/js/linegame/view/LineGameScreenView.ts b/graphing-lines/js/linegame/view/LineGameScreenView.ts --- a/graphing-lines/js/linegame/view/LineGameScreenView.ts (revision 7a56881bbcb1485b111274754ece1f97e36544b3) +++ b/graphing-lines/js/linegame/view/LineGameScreenView.ts (date 1704405549598) @@ -38,12 +38,10 @@ super( model, levelImages, rewardNodeFunctions, tandem, { settingsNodeOptions: { levelSelectionButtonGroupOptions: { - flowBoxOptions: { - spacing: 50, // x spacing - lineSpacing: 50, // y spacing - preferredWidth: 800, // set empirically to provide 3 buttons per row - wrap: true, - justify: 'center' + gridBoxOptions: { + spacing: 50, + autoColumns: 3, + xAlign: 'center' } } } 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 b0ddbd7cbb45985178ac85ad98709487b9275cb2) +++ b/vegas/js/demo/components/demoLevelSelectionButtonGroup.ts (date 1704404671631) @@ -81,7 +81,7 @@ levelSelectionButtonOptions: { baseColor: 'pink' }, - flowBoxOptions: { + gridBoxOptions: { spacing: 30 }, groupButtonHeight: 100, @@ -123,27 +123,24 @@ const buttonWidth = 100; const buttonHeight = 100; const buttonLineWidth = 3; - const xSpacing = 20; - const ySpacing = 20; + const spacing = 20; // preferredWidth is used to limit the width of the FlowBox, so that it displays a maximum number of buttons // per row. When combined with wrap:true, this causes buttons to wrap to a new line. // It would also be acceptable to set this value empirically. - const preferredWidth = ( buttonsPerRow * buttonWidth ) + // width of the buttons - ( ( buttonsPerRow - 1 ) * xSpacing ) + // space between the buttons - ( 2 * buttonsPerRow * buttonLineWidth ); // lineWidth of the buttons + // const preferredWidth = ( buttonsPerRow * buttonWidth ) + // width of the buttons + // ( ( buttonsPerRow - 1 ) * xSpacing ) + // space between the buttons + // ( 2 * buttonsPerRow * buttonLineWidth ); // lineWidth of the buttons super( items, { levelSelectionButtonOptions: { baseColor: 'lightGreen', lineWidth: buttonLineWidth }, - flowBoxOptions: { - spacing: xSpacing, // horizontal spacing - lineSpacing: ySpacing, // vertical spacing - preferredWidth: preferredWidth, - wrap: true, // start a new row when preferredWidth is reached - justify: 'center' // horizontal justification + gridBoxOptions: { + spacing: spacing, // horizontal spacing + autoColumns: buttonsPerRow, + xAlign: 'center' // horizontal justification }, groupButtonHeight: buttonHeight, groupButtonWidth: buttonWidth, @@ -186,7 +183,7 @@ groupButtonWidth: 75, groupButtonHeight: 75, - // Create a custom layout, not possible via the default FlowBox and flowBoxOptions. + // Create a custom layout, not possible via the default FlowBox and gridBoxOptions. createLayoutNode: ( buttons: LevelSelectionButton[] ) => { assert && assert( buttons.length === 5, 'rows option value is hardcoded for 5 levels' ); return new GridBox( { 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 10aa9a5b65092c257536e9a376d497b6fa1ab74f) +++ b/balancing-chemical-equations/js/game/view/LevelSelectionNode.ts (date 1704404580591) @@ -59,7 +59,7 @@ levelSelectionButtonOptions: { baseColor: '#d9ebff' }, - flowBoxOptions: { + gridBoxOptions: { spacing: 50, center: layoutBounds.center }, Index: balancing-act/js/game/view/StartGameLevelNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/balancing-act/js/game/view/StartGameLevelNode.js b/balancing-act/js/game/view/StartGameLevelNode.js --- a/balancing-act/js/game/view/StartGameLevelNode.js (revision 22d29f5d47de59a4e7374c315416a757316d8303) +++ b/balancing-act/js/game/view/StartGameLevelNode.js (date 1704404580597) @@ -82,7 +82,7 @@ }; } const levelSelectionButtonGroup = new LevelSelectionButtonGroup( buttonItems, { - flowBoxOptions: { + gridBoxOptions: { spacing: 30 } } ); 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 b0ddbd7cbb45985178ac85ad98709487b9275cb2) +++ b/vegas/js/LevelSelectionButtonGroup.ts (date 1704405138579) @@ -10,7 +10,7 @@ * - Support for the gameLevels query parameter, via LevelSelectionButtonGroupOptions.gameLevels. * * Layout: - * - The default layout is a single row of buttons, customizable via LevelSelectionButtonGroupOptions.flowBoxOptions. + * - The default layout is a single row of buttons, customizable via LevelSelectionButtonGroupOptions.gridBoxOptions. * - To create multiple rows of buttons, see example MultiRowButtonGroup in demoLevelSelectionButtonGroup.ts. * - To create a custom layout, see example XButtonGroup in demoLevelSelectionButtonGroup.ts. * @@ -20,7 +20,7 @@ import StrictOmit from '../../phet-core/js/types/StrictOmit.js'; import PickRequired from '../../phet-core/js/types/PickRequired.js'; import optionize, { combineOptions } from '../../phet-core/js/optionize.js'; -import { FlowBox, FlowBoxOptions, LayoutNode, Node, NodeLayoutConstraint, NodeOptions } from '../../scenery/js/imports.js'; +import { FlowBox, FlowBoxOptions, GridBox, GridBoxOptions, LayoutNode, Node, NodeLayoutConstraint, NodeOptions } from '../../scenery/js/imports.js'; import LevelSelectionButton, { DEFAULT_BUTTON_DIMENSION, LevelSelectionButtonOptions } from './LevelSelectionButton.js'; import TProperty from '../../axon/js/TProperty.js'; import Tandem from '../../tandem/js/Tandem.js'; @@ -50,7 +50,7 @@ levelSelectionButtonOptions?: StrictOmit; // Options for the default layout, which is a FlowBox. Ignored if createLayoutNode is provided. - flowBoxOptions?: StrictOmit; + gridBoxOptions?: StrictOmit; // Creates the Node that handles layout of the buttons. // Use this option if you have a custom layout that cannot be achieved using the default FlowBox. @@ -84,10 +84,9 @@ StrictOmit, NodeOptions>()( { groupButtonHeight: DEFAULT_BUTTON_DIMENSION, groupButtonWidth: DEFAULT_BUTTON_DIMENSION, - // The default layout is a single row of buttons. - flowBoxOptions: { - orientation: 'horizontal', - spacing: 10 + gridBoxOptions: { + spacing: 10, + autoRows: 1 // The default layout is a single row of buttons. }, // @ts-expect-error This default is provided for JavaScript simulations. tandem: Tandem.REQUIRED @@ -128,10 +127,15 @@ } else { - // The default layout is a FlowBox, customizable via options.flowBoxOptions. - layoutNode = new FlowBox( combineOptions( { + // autoRows is provided as a default, however if a client specifies autoColumns, then autoRows must be null. + if ( options.gridBoxOptions.autoColumns ) { + options.gridBoxOptions.autoRows = null; + } + + // The default layout is a FlowBox, customizable via options.gridBoxOptions. + layoutNode = new GridBox( combineOptions( { children: buttons - }, options.flowBoxOptions ) ); + }, options.gridBoxOptions ) ); } options.children = [ layoutNode ]; 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 74869fe927d3efad52c81362f2eb2fc9320767ef) +++ b/fourier-making-waves/js/waveGame/view/WaveGameLevelSelectionButtonGroup.ts (date 1704405549592) @@ -50,12 +50,11 @@ groupButtonHeight: BUTTON_HEIGHT, // A maximum number of buttons per row, wrapping to a new row - flowBoxOptions: { - spacing: X_SPACING, // horizontal spacing - lineSpacing: Y_SPACING, // vertical spacing - preferredWidth: BUTTONS_PER_ROW * ( BUTTON_WIDTH + X_SPACING ), - wrap: true, // start a new row when preferredWidth is reached - justify: 'center' // horizontal justification + gridBoxOptions: { + xSpacing: X_SPACING, // horizontal spacing + ySpacing: Y_SPACING, // vertical spacing, + autoColumns: BUTTONS_PER_ROW, + xAlign: 'center' }, gameLevels: FMWQueryParameters.gameLevels, tandem: tandem Index: area-model-common/js/game/view/GameAreaScreenView.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/area-model-common/js/game/view/GameAreaScreenView.js b/area-model-common/js/game/view/GameAreaScreenView.js --- a/area-model-common/js/game/view/GameAreaScreenView.js (revision ccf6c870a1ae7e05237c621d805e733fc04b4afe) +++ b/area-model-common/js/game/view/GameAreaScreenView.js (date 1704404832936) @@ -126,11 +126,9 @@ } ); this.levelSelectionLayer.addChild( new LevelSelectionButtonGroup( levelButtons, { - flowBoxOptions: { + gridBoxOptions: { spacing: buttonSpacing, - lineSpacing: buttonSpacing, - preferredWidth: 513, // empirically determined from default button width and lineWidth, and buttonSpacing - wrap: true + autoColumns: 3 }, center: this.layoutBounds.center } ) ); ```
pixelzoom commented 8 months ago

Ah... I had forgotten about these layouts that are not actually grids. A grid is not going to work here. Also ?gameLevels and LevelSelectionButton visibleProperty can be used to hide buttons. And it's not clear to me how to maintain a "visually pleasing" layout as buttons are added/removed. So I recommend that we table this issue, and add it to the "PhET-iO and Games" discussion in https://github.com/phetsims/vegas/issues/118.

marlitas commented 8 months ago

On-Hold until this is discussed and reviewed with a designer. The discussion may potentially happen with #118