Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RectangularRadioButtonGroup doesn't update layout when button widths change #852

Closed
jbphet opened this issue Aug 21, 2023 · 7 comments
Closed
Assignees

Comments

@jbphet
Copy link
Contributor

jbphet commented Aug 21, 2023

The widths of the buttons in a RectangularRadioButtonGroup don't appear to update when the size of one button changes in a different way from the others. This was first reported as part of phetsims/greenhouse-effect#349, and there is another issue that was created for updating the just the highlight.

I read through the code in RectangularRadioButtonGroup and found a portion of the code that looks like it is intended to make the widths of all the buttons the same. This code currently starts on line 181 of the source file and looks like this:

    // Calculate the maximum width and height of the content, so we can make all radio buttons the same size.
    const widestContentWidth = _.maxBy( nodes, node => node.width )!.width;
    const tallestContentHeight = _.maxBy( nodes, node => node.height )!.height;

This code probably needs to be re-run when the width changes for one or more of the buttons.

@jbphet
Copy link
Contributor Author

jbphet commented Aug 21, 2023

Assigning to the original author, he can pass it on to the dynamic layout experts if necessary.

@jbphet
Copy link
Contributor Author

jbphet commented Aug 21, 2023

I just realized a screenshot would be helpful, so here is an example that can be easily duplicated with the recent Greenhouse Effect dev test version. To duplicate:

  1. Go to https://phet-dev.colorado.edu/html/greenhouse-effect/1.2.0-dev.2/phet/greenhouse-effect_en_phet.html?stringTest=dynamic
  2. Select the first screen (Waves), and then select the "date mode" by pressing the button with the calendar icon "Greenhouse Gas Concentration" control panel
  3. Press the right arrow button to increase the string sizes

image

@pixelzoom
Copy link
Contributor

pixelzoom commented Aug 22, 2023

Aaron Davis was the original author of RectangularRadioButtonGroup. I made changes at various points to manage chaos as this class (and its supporting classes) evolved. I don't see any commits that indicate support for dynamic layout has been added. @jonathanolson has generally been handling dynamic layout for sun components, and it would be good to keep things consistent, so reassigning to him.

@marlitas
Copy link
Contributor

marlitas commented Sep 6, 2023

Just to confirm, because I believe I was initially confused when looking at this issue. We want the radio button widths to match across all the buttons? The group is updating is minimumWidths as I would expect, however visually the buttons should all be the same size and that is currently not happening?

I will assign and take a look at this as well.

@marlitas marlitas self-assigned this Sep 6, 2023
@marlitas marlitas removed their assignment Feb 23, 2024
@marlitas
Copy link
Contributor

@jonathanolson We would like you to prioritize this issue since it is now blocking Greenhouse: #851

@jonathanolson
Copy link
Contributor

Should be fixed in 3e55389.

@jbphet can you confirm?

@jbphet
Copy link
Contributor Author

jbphet commented Mar 20, 2024

Wow - nice one-line fix @jonathanolson! In fact, there was a significant reduction in code including all the related commits shown above.

I've verified that it's working correctly in Greenhouse Effect for the case described above. I think we're good to go here. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants