phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

ToggleNode should support dynamic layout #885

Closed samreid closed 1 month ago

samreid commented 2 months ago

During https://github.com/phetsims/density-buoyancy-common/issues/154 @zepumph and I observed that ToggleNode and BooleanToggleNode do not support dynamic layout. When the children are resized, the layout function does not relayout. Thus during translations, things can end up misaligned. We discussed this patch:

```diff Subject: [PATCH] Use BooleanToggleNode.ts, see https://github.com/phetsims/density-buoyancy-common/issues/154 --- Index: js/ToggleNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/ToggleNode.ts b/js/ToggleNode.ts --- a/js/ToggleNode.ts (revision f4b4c6064617a45c29e1fb503e9e47e7ee68a754) +++ b/js/ToggleNode.ts (date 1717533183227) @@ -11,7 +11,7 @@ import StrictOmit from '../../phet-core/js/types/StrictOmit.js'; import optionize from '../../phet-core/js/optionize.js'; -import { Node, NodeOptions } from '../../scenery/js/imports.js'; +import { LayoutProxy, ManualConstraint, Node, NodeOptions } from '../../scenery/js/imports.js'; import sun from './sun.js'; import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js'; import GroupItemOptions, { getGroupItemNodes } from './GroupItemOptions.js'; @@ -20,10 +20,12 @@ value: T; // a value } & GroupItemOptions; +type Nodelike = Node | LayoutProxy; + type SelfOptions = { // {function} determines the relative layout of element Nodes. See below for pre-defined layout. - alignChildren?: ( children: Node[] ) => void; + alignChildren?: ( children: Nodelike[] ) => void; // Determine whether unselected children (the ones not displayed) are in the scene graph. // - If included (the default), unselected children are in the scene graph and hidden via setVisible(false). In this case @@ -59,6 +61,10 @@ super( options ); + ManualConstraint.create( this, options.children, ( ...x: Nodelike[] ) => { + options.alignChildren( x ); + } ); + const valueListener = ( value: T ) => { const matches: Node[] = []; for ( let i = 0; i < elements.length; i++ ) { @@ -98,7 +104,7 @@ * A value for the alignChildren option. * Centers the latter nodes on the x,y center of the first node. */ - public static CENTER( children: Node[] ): void { + public static CENTER( children: Nodelike[] ): void { for ( let i = 1; i < children.length; i++ ) { children[ i ].center = children[ 0 ].center; } @@ -108,7 +114,7 @@ * A value for the alignChildren option. * Centers the latter nodes on the x center of the first node. */ - public static CENTER_X( children: Node[] ): void { + public static CENTER_X( children: Nodelike[] ): void { for ( let i = 1; i < children.length; i++ ) { children[ i ].centerX = children[ 0 ].centerX; } @@ -118,7 +124,7 @@ * A value for the alignChildren option. * Centers the latter nodes on the y center of the first node. */ - public static CENTER_Y( children: Node[] ): void { + public static CENTER_Y( children: Nodelike[] ): void { for ( let i = 1; i < children.length; i++ ) { children[ i ].centerY = children[ 0 ].centerY; } @@ -128,7 +134,7 @@ * A value for the alignChildren option. * Left aligns nodes on the left of the first node. */ - public static LEFT( children: Node[] ): void { + public static LEFT( children: Nodelike[] ): void { for ( let i = 1; i < children.length; i++ ) { children[ i ].left = children[ 0 ].left; } @@ -138,7 +144,7 @@ * A value for the alignChildren option. * Aligns nodes on the bottom of the first node. */ - public static BOTTOM( children: Node[] ): void { + public static BOTTOM( children: Nodelike[] ): void { for ( let i = 1; i < children.length; i++ ) { children[ i ].bottom = children[ 0 ].bottom; } @@ -148,7 +154,7 @@ * A value for the alignChildren option. * Aligns nodes on the bottom of the first node. */ - public static CENTER_BOTTOM( children: Node[] ): void { + public static CENTER_BOTTOM( children: Nodelike[] ): void { for ( let i = 1; i < children.length; i++ ) { children[ i ].centerBottom = children[ 0 ].centerBottom; } @@ -158,7 +164,7 @@ * A value for the alignChildren option. * Right aligns nodes on the right of the first node. */ - public static RIGHT( children: Node[] ): void { + public static RIGHT( children: Nodelike[] ): void { for ( let i = 1; i < children.length; i++ ) { children[ i ].right = children[ 0 ].right; } @@ -168,7 +174,7 @@ * A value for the alignChildren option. * No alignment is performed. */ - public static NONE( children: Node[] ): void { + public static NONE( children: Nodelike[] ): void { // do nothing } }
zepumph commented 1 month ago

Snapshot comparison tests didn't show any issues, so I was able to commit and remove workarounds. Closing.