Setting generationSpinner.visibleProperty to false creates a layout problem.

[pixelzoom]

Reported by @arouinfar in #339 (comment). It's not related to that issue, becuase generationSpinner.visibleProperty has always been writeable. So creating this issue.

---

I noticed that toggling generationSpinner.visibleProperty can create layout issues.

I don't remember seeing this before, but I might of missed it. Not sure if this is related to the changes made to generationSpinner here or if this is entirely unrelated issue.

[pixelzoom]

The layout is handled in ProportionsGraphNode.ts. I reverted that file all the way back to 11/10/22, just prior to TypeScript conversion, and the problem is still present.

The problem is not present in the 1.4 release.

So either this problem was introduced a long time ago, or it's a recent problem in scenery layout.

[pixelzoom]

The layout is very simple, nothing clever going on, so I'm can't immediately identify the problem. Here's the relevant code in
ProportionsGraphNode.ts:

    // Layout the columns
    const hBox = new HBox( {
      spacing: COLUMN_SPACING,
      align: 'center',
      children: [ labelsColumn, ...geneColumns ]
    } );

    // Spinner for selecting which generation is displayed
    const generationSpinner = new ProportionsGenerationSpinner( proportionsModel.proportionsGenerationProperty, {
      tandem: options.tandem.createTandem( 'generationSpinner' )
    } );

    const content = new VBox( {
      align: 'center',
      spacing: 35,
      children: [ hBox, generationSpinner ]
    } );

[pixelzoom]

I replaced most of the graph with a rectangle:

    const rectangle = new Rectangle( 0, 0, 544, 173, {
      fill: 'red',
      stroke: 'black'
    } );

    // Spinner for selecting which generation is displayed
    const generationSpinner = new ProportionsGenerationSpinner( proportionsModel.proportionsGenerationProperty, {
      tandem: options.tandem.createTandem( 'generationSpinner' )
    } );

    const content = new VBox( {
      align: 'center',
      spacing: 35,
      children: [ rectangle, generationSpinner ]
    } );

... and I'm still seeing the reported layout problem when toggling generationSpinner.visibleProperty:

[pixelzoom]

This problem is present in all of the 1.5 dev versions (1.5.0-dev.1 through 1.5.0-dev.4) that have been published. 1.5.0-dev.1 was published over a year ago, on 7/6/2022. That was before support for dynamic locale was added (8/27/22) and before TypeScript conversion (11/10/22)

[pixelzoom]

Fixed in the above commit by listening to boundsProperty instead of localBoundsProperty. Relevant change in ProportionsGraphNode.ts:

-   content.localBoundsProperty.link( () => {
+   content.boundsProperty.link( () => {
      content.center = backgroundNode.center;
    } );

Listening to localBoundsProperty was an old pattern that prevented getting into an infinite loop. AFAIK it's supposed to still work. But something must have changed about the order that scenery updates localBoundsProperty. So mental note to avoid it in the future. And I don't see usages in the other sims that I'm responsible for.

@arouinfar please review. There were 3 other case (in PopulationGraphNode) were I also switched from localBoundsProperty to boundsProperty. I tested and didn't see any problems, but that should get a look too.

[arouinfar]

Looks good on master, thanks @pixelzoom.