phetsims / sun

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

ComboBox should use a line instead of a separator #810

Closed marlitas closed 1 year ago

marlitas commented 1 year ago

From Dev Meeting 11/17/22:

@jonathanolson: I'm not convinced ComboBoxButton should be using a separator. That seems fragile and should be done with something else.

This will remove the deprecated separator and use Line or a similar component to achieve the proper look and layout for comboBox.

marlitas commented 1 year ago

This work is completed in the above commits. Assigning to @jbphet for review as responsible dev.

jbphet commented 1 year ago

The changes look reasonable to me. I also tested the behavior in the sun demo and in Molarity, which has a large and prominent combo box, and the behavior and appearance seem correct. Closing.

pixelzoom commented 1 year ago

Reopening.

Was this tested with dynamic layout? I'm seeing buggy layout when the combo box button grows. For example, here's the control panel in the Discrete screen of Fourier, after pressing RIGHT arrow once with stringTest=dynamic. The ▼ is outside of the button bounds, and should be centered in the space to the right of the vertical separator.

screenshot_2044

Blocks publication of any sim that uses ComboBox and supports dynamic layout.

pixelzoom commented 1 year ago

I do not see this problem with the combo boxes in pH Scale or Beer's Law Lab. Perhaps there's some layout assumption that's different for Fourier?

marlitas commented 1 year ago

It looks like AlignBox is not receiving the correct minimumWidth from it's GridBox content in Fourier.

AlignBox minimumWidth = 135 Screenshot 2022-12-12 at 11 07 18 AM

GridBox minimumWidth = 148 Screenshot 2022-12-12 at 11 07 33 AM

I confirmed this was happening prior to switching out VSeparatorDeprecated with Line in ComboBoxButton. Since this seems unrelated to this conversion I created a new issue (https://github.com/phetsims/sun/issues/816), and will close this one as completed.