phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
http://scenerystack.org/
MIT License
9 stars 6 forks source link

AudioPreferencesPanel does not dynamically change its layout #903

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

By default, the Audio Preferences for Number Play looks like this. It's a really strange layout, but that's not the probem I'm reporting.

screenshot_2228

If we run Number Play with screens=3,4, then "Hear Total" toggle is irrelevant, and it's hidden by setting its visible: false. The layout then looks like this, which is really wrong. The left column is empty, so the right column should have moved to the center.

screenshot_2227

The relevant code is in AudioPreferencesPanel.ts. excludeInvisibleChildrenFromBounds: false is probably the culprit here - it's default is true for FlowBox/VBox/HBox. But note the comment.

    // Some contents of this Dialog will be dynamically removed. Dont resize when this happens because we don't want
    // to shift contents of the entire Preferences dialog.
    const contentOptions: VBoxOptions = {
      align: 'left',
      spacing: PreferencesDialog.CONTENT_SPACING,
      excludeInvisibleChildrenFromBounds: false
    };
marlitas commented 1 year ago

The culprit for this was the contentNode being created in PreferencesPanelSection. I addressed this in the above commit and made sure there were no reverberations to other sims, most notably I checked: John Travoltage, Ratio and Proportion, and Quadrilateral.

@pixelzoom can you review and close if all looks good? I'm mostly curious if I used combineOptions appropriately here.

Also if any collateral effects are seen by other devs from this change in other sims please re-open and ping me.

pixelzoom commented 1 year ago

Verified that layout looks as expected with and without screens=3,4 for NumberPlay.

Use of combineOptions looks fine.

Closing.