Buttons with no content don't lay out correctly

[jbphet]

Saw this in the sun demo just now while testing changes for https://github.com/phetsims/sun/issues/292.

[jbphet]

I got some help from @marlitas on this, and we discovered that the preferred height and width are zero for these buttons. Here's a screen shot:

We also found that adding any content to the buttons makes them lay out correctly, though they seem to size themselves a little oddly. Here is a screenshot of a case where I added the option content: new Text( 'X' ) to each of the momentary buttons in demoMomentaryButtons.ts:

This led @marlitas and I to conclude that this is a general problem with WidthSizable, specifically its handling of the case where a button has no content. With this in mind, I'm going to move this issue to the scenery repo, change the name, and assign it to @marlitas and @jonathanolson.

[marlitas]

I ran into a buttonConstraint layout issue in Number Line Integers as well. I don't think they're exactly related, but seem to be in the same area. In this case ButtonNodeConstraint is not setting the correct minimumHeight for a RoundButton if the radius option is not passed in by the client.

The below patch fixes the bug I encountered, but also feels like it might be potentially mis-using minUnstrokedHeight and minUnstrokedWidth.

Would like to discuss with @jonathanolson

```diff Subject: [PATCH] Center text dynamically, see: https://github.com/phetsims/number-line-integers/issues/108 --- Index: js/buttons/RoundButton.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/buttons/RoundButton.ts b/js/buttons/RoundButton.ts --- a/js/buttons/RoundButton.ts (revision 26851caba268b49f4abbd8a50e4ce4372cc625b6) +++ b/js/buttons/RoundButton.ts (date 1699573275871) @@ -78,14 +78,6 @@ assert && assert( typeof options.radius === 'number', `invalid radius: ${options.radius}` ); } - if ( options.radius ) { - assert && assert( options.xMargin < options.radius, 'xMargin cannot be larger than radius' ); - assert && assert( options.yMargin < options.radius, 'yMargin cannot be larger than radius' ); - - options.minUnstrokedWidth = options.radius * 2; - options.minUnstrokedHeight = options.radius * 2; - } - // If no options were explicitly passed in for the button appearance strategy, pass through the general appearance // options. if ( !options.buttonAppearanceStrategyOptions ) { @@ -99,6 +91,18 @@ const buttonRadius = options.radius || Math.max( options.content!.width + options.xMargin * 2, options.content!.height + options.yMargin * 2 ) / 2; + if ( options.radius ) { + assert && assert( options.xMargin < options.radius, 'xMargin cannot be larger than radius' ); + assert && assert( options.yMargin < options.radius, 'yMargin cannot be larger than radius' ); + + options.minUnstrokedWidth = options.radius * 2; + options.minUnstrokedHeight = options.radius * 2; + } + else { + options.minUnstrokedWidth = buttonRadius * 2; + options.minUnstrokedHeight = buttonRadius * 2; + } + if ( options.content && options.radius ) { const previousContent = options.content; const minScale = Math.min( ```

[jonathanolson]

Tracking progress in button-layout branch

[jonathanolson]

Sandbox link initial: http://localhost/scenery/tests/sandbox.html?script=%2F%2F%20import%20sun%2Fjs%2Fbuttons%2FRoundPushButton%0A%2F%2F%20import%20sun%2Fjs%2Fbuttons%2FRectangularPushButton%0A%0Aconst%20roundPushButton%20%3D%20new%20RoundPushButton(%20%7B%20radius%3A%2050%20%7D%20)%3B%0Aconst%20rectangularPushButton%20%3D%20new%20RectangularPushButton(%20%7B%20size%3A%20new%20Dimension2(%20150%2C%20150%20)%20%7D%20)%3B%0A%0AroundPushButton.heightSizable%20%3D%20true%3B%0AroundPushButton.layoutOptions%20%3D%20%7B%20stretch%3A%20true%20%7D%3B%0A%0ArectangularPushButton.layoutOptions%20%3D%20%7B%20grow%3A%201%20%7D%3B%0A%0Ascene.addChild(%20new%20HBox(%20%7B%0A%20%20spacing%3A%2010%2C%0A%20%20children%3A%20%5B%20roundPushButton%2C%20rectangularPushButton%20%5D%2C%0A%20%20preferredWidth%3A%20500%2C%0A%20%20%0A%7D%20)%20)%3B%0A%0A%2F%2Fconsole.log(%20roundPushButton.width%2C%20rectangularPushButton.width%20)%0A%2F%2Fconsole.log(%20roundPushButton.parent.width%20)%0Aconsole.log(%20roundPushButton.minimumWidth%20)%0A

[jonathanolson]

@jbphet I believe this is resolved. Do you mind checking briefly?

[jbphet]

Sun demo (where the problem was initially noticed) is lookin' good:

Also, the values shown by the Scenery helper seem more reasonable:

I also checked the case where text content is added to the buttons as a sort of a regression test, and it looked fine.

@jonathanolson - Hopefully this is what you meant by "checking briefly". Let me know if you need any additional review. I haven't review the code, just the behavior.

[jonathanolson]

Yup, thanks! Closing.