Removing aspectRatio on ButtonNode, see phetsims/scenery#1482, phetsi…

…ms/scenery#1513
phetsims · Jun 21, 2024 · 998ffbc · 998ffbc
1 parent d19ca36
commit 998ffbc
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 35 deletions.
diff --git a/js/buttons/ButtonNode.ts b/js/buttons/ButtonNode.ts
@@ -73,11 +73,6 @@ type SelfOptions = {
 
   // Alter the appearance when changing the enabled of the button.
   enabledAppearanceStrategy?: EnabledAppearanceStrategy;
-
-  // If non-null, the aspect ratio of the button will be constrained to this value. It will check the minimum sizes,
-  // and will increase the minimum size if necessary to maintain the aspect ratio.
-  // Notably, this is used in RoundButton, so that the button is always a circle.
-  aspectRatio?: number | null;
 };
 type ParentOptions = SizableOptions & VoicingOptions & NodeOptions;
 
@@ -137,7 +132,6 @@ export default class ButtonNode extends Sizable( Voicing( Node ) ) {
         }
       },
       disabledColor: ColorConstants.LIGHT_GRAY,
-      aspectRatio: null,
 
       // pdom
       tagName: 'button',
@@ -156,9 +150,6 @@ export default class ButtonNode extends Sizable( Voicing( Node ) ) {
       'if options.enabledProperty is provided, it must === buttonModel.enabledProperty' );
     options.enabledProperty = buttonModel.enabledProperty;
 
-    assert && assert( options.aspectRatio === null || ( isFinite( options.aspectRatio ) && options.aspectRatio > 0 ),
-      `ButtonNode aspectRatio should be a positive finite value if non-null. Instead received ${options.aspectRatio}.` );
-
     super();
 
     this.content = options.content;

diff --git a/js/buttons/RectangularButton.ts b/js/buttons/RectangularButton.ts
@@ -157,7 +157,6 @@ export default class RectangularButton extends ButtonNode {
       xMargin: options.xMargin,
       yMargin: options.yMargin,
       maxLineWidth: this.maxLineWidth,
-      aspectRatio: options.aspectRatio,
       touchAreaXDilation: options.touchAreaXDilation,
       touchAreaYDilation: options.touchAreaYDilation,
       touchAreaXShift: options.touchAreaXShift,
@@ -390,7 +389,7 @@ type RectangularButtonNodeConstraintOptions = {
   buttonBackgroundOptions: ButtonShapeOptions;
   maxLineWidth: number;
 } & Required<Pick<RectangularButtonOptions,
-  'content' | 'size' | 'xMargin' | 'yMargin' | 'minWidth' | 'minHeight' | 'aspectRatio' |
+  'content' | 'size' | 'xMargin' | 'yMargin' | 'minWidth' | 'minHeight' |
   'touchAreaXDilation' | 'touchAreaYDilation' | 'touchAreaXShift' | 'touchAreaYShift' |
   'mouseAreaXDilation' | 'mouseAreaYDilation' | 'mouseAreaXShift' | 'mouseAreaYShift'
 >>;
@@ -431,8 +430,6 @@ class RectangularButtonNodeConstraint extends LayoutConstraint {
     const buttonNode = this.buttonNode;
     const content = this.options.content;
 
-    // TODO: add infinite loop protection with equalsEpsilon
-
     const widthSizable = buttonNode.widthSizable;
     const heightSizable = buttonNode.heightSizable;
     const contentWidthSizable = !!content && isWidthSizable( content );
@@ -452,26 +449,14 @@ class RectangularButtonNodeConstraint extends LayoutConstraint {
     contentMinimumHeightWithMargins = Math.max( this.options.minHeight, contentMinimumHeightWithMargins );
 
     // Only allow an initial update if we are not sizable in that dimension
-    let minimumWidth =
+    const minimumWidth =
       ( this.isFirstLayout || widthSizable )
       ? contentMinimumWidthWithMargins + this.options.maxLineWidth
       : buttonNode.localMinimumWidth!;
-    let minimumHeight = ( this.isFirstLayout || heightSizable )
+    const minimumHeight = ( this.isFirstLayout || heightSizable )
       ? contentMinimumHeightWithMargins + this.options.maxLineWidth
       : buttonNode.localMinimumHeight!;
 
-    // TODO: potentially ditch aspectRatio? Are we using it?
-    if ( this.options.aspectRatio !== null ) {
-      // TODO: for circular, check whether we are widthSizable/etc.
-
-      if ( minimumWidth < minimumHeight * this.options.aspectRatio ) {
-        minimumWidth = minimumHeight * this.options.aspectRatio;
-      }
-      if ( minimumHeight < minimumWidth / this.options.aspectRatio ) {
-        minimumHeight = minimumWidth / this.options.aspectRatio;
-      }
-    }
-
     // Our resulting sizes (allow setting preferred width/height on the buttonNode)
     this.lastLocalWidth = this.isFirstLayout || widthSizable
                                    ? Math.max( minimumWidth, widthSizable ? buttonNode.localPreferredWidth ?? 0 : 0 )

diff --git a/js/buttons/RoundButton.ts b/js/buttons/RoundButton.ts
@@ -70,8 +70,6 @@ export default class RoundButton extends ButtonNode {
       mouseAreaXShift: 0,
       mouseAreaYShift: 0,
 
-      aspectRatio: 1, // This will keep the minimum width and height the same, so our bounding box will be square
-
       // ButtonNodeOptions
       cursor: 'pointer',
 
@@ -118,7 +116,6 @@ export default class RoundButton extends ButtonNode {
       xMargin: options.xMargin,
       yMargin: options.yMargin,
       maxLineWidth: this.maxLineWidth,
-      aspectRatio: options.aspectRatio,
       touchAreaDilation: options.touchAreaDilation,
       touchAreaXShift: options.touchAreaXShift,
       touchAreaYShift: options.touchAreaYShift,
@@ -306,7 +303,7 @@ type RoundButtonNodeConstraintOptions = {
   buttonBackground: Circle;
   maxLineWidth: number;
 } & Required<Pick<RoundButtonOptions,
-  'content' | 'radius' | 'xMargin' | 'yMargin' | 'aspectRatio' |
+  'content' | 'radius' | 'xMargin' | 'yMargin' |
   'touchAreaDilation' | 'touchAreaXShift' | 'touchAreaYShift' | 'mouseAreaDilation' | 'mouseAreaXShift' | 'mouseAreaYShift'
 >>;
 
@@ -346,8 +343,6 @@ class RoundButtonNodeConstraint extends LayoutConstraint {
     const buttonNode = this.buttonNode;
     const content = this.options.content;
 
-    // TODO: add infinite loop protection with equalsEpsilon
-
     const widthSizable = buttonNode.widthSizable;
     const heightSizable = buttonNode.heightSizable;
     const contentWidthSizable = !!content && isWidthSizable( content );
@@ -372,8 +367,6 @@ class RoundButtonNodeConstraint extends LayoutConstraint {
       ? 2 * contentMinimumRadius + this.options.maxLineWidth
       : buttonNode.localMinimumHeight!;
 
-    assert && assert( this.options.aspectRatio === 1 );
-
     // Our resulting sizes (allow setting preferred width/height on the buttonNode)
     this.lastLocalWidth = this.isFirstLayout || widthSizable
       ? Math.max( minimumWidth, widthSizable ? buttonNode.localPreferredWidth ?? 0 : 0 )