Recenter NumberControl title when changed with PhET-iO?

[zepumph]

From phetsims/gravity-force-lab#76. It looks like GFL is using layoutFunction3 with the title align "center". @kathy-phet and I thought it would be nice if that recentered when the text changed. I couldn't figure out an easy way to do it, I tried something like

titleText.on( 'text', ()=> { titleText.centerX = wholeNode.centerX; } );

But it didn't work out of the box. I think this may have to do with the "resize" option which is set to false to "prevent slider from causing a resize when thumb is at min or max."

@pixelzoom added that commit (albeit 3 years ago). What do you think the best way to accomplish this is? Or do you think I should push back on this request?

UPDATE: tagging phetsims/gravity-force-lab#175 too

[pixelzoom]

My recommendation is to not make the change to NumberControl. Instead, set a maxWidth for the title, and require that any title set via PhET-iO to be limited to that width. If you don't do that, then you're asking for layout problems. That said...

Its more appropriate (and probably safer) to observe 'bounds', not 'text':

titleText.on( 'bounds', ()=> { titleText.centerX = wholeNode.centerX; } );

Re "the resize option".... You are presumably talking about the resize: false option to the VBox in layoutFunction3. You could propagate options to the VBox, so that you can change to resize: true in your client code. If you do that, please do so for all predefined layout functions.

[kathy-phet]

The way it was working did seem to be resizing the text correctly - it just wasn't centering it.

[zepumph]

The priority of this was raised because a client asked for it. In lieu of this I decided to only update this for NumberControls using layoutFunction3 and centering the title. I tested that this works as expected when changing the title in GFL studio, and I also regression tested a slider that with a centered title, and a slider with a left aligned title.

@pixelzoom please review.

[zepumph]

Here is the phetioID of the element in GFL studio that adjusts a number control title:
gravityForceLab.gravityForceLabScreen.view.massControl2.numberControl.titleNode.textProperty

[pixelzoom]

Imo, e1f6bb5 is totally unacceptable. Not only does it not address layout functions uniformly, it doesn’t even handle all values of options.alignTitle. It’s totally specific to createLayoutFunction3 and 'center' alignment. For something as important as NumberControl, I recommend taking the time to do this right. If that means adjusting "priority" to make that happen, then discuss with @ariel-phet.

[zepumph]

Thanks! I'll take another look.

[pixelzoom]

After discussing with @zepumph on Slack, here's what I recommend:

(1) Correctly handle all values of options.alignTitle in createLayoutFunction3, so that you're not creating buggy behavior for clients who use this layout with something other than 'center'. @zepumph suggested something like:

       titleAlignBox.on( 'bounds', () => {
          if ( options.alignTitle === 'center' ) {
            titleAlignBox.centerX = vBox.centerX;
          }
          else if( options.alignTitle === 'left'){
            titleAlignBox.left = vBox.left;
          }
          else if( options.alignTitle === 'right'){
            titleAlignBox.right = vBox.right;
          }
        } );

(2) If being able to change the title is truly "important for PhET-iO", then adding support to createLayoutFunction3 is the tip of the proverbial iceberg. Create an issue indicating that similar support needs to be added to all other predefined and custom layout functions, and decide how that work should be prioritized.

[zepumph]

I added (1) in the above commit. When testing with ISLCControlPanel (in GFL), the "left" option worked as expected, but for some reason, the "right" one did not. I'm going to have to investigate further (it may be ISLC implementation specific).

[zepumph]

After some more experimenting, I found a solution that I am more happy with. Instead of manually trying to recompute the alignment, I just passed that work up to the vbox. This is working perfectly save for really large strings. When the string is past the maxWidth, it begins scaling down, and then that pushed the numbercontrol content up, see below. I don't know an easy way around this. @pixelzoom can you think of how to "lock in" the height of the vBox on creation?

(2) If being able to change the title is truly "important for PhET-iO", then adding support to createLayoutFunction3 is the tip of the proverbial iceberg. Create an issue indicating that similar support needs to be added to all other predefined and custom layout functions, and decide how that work should be prioritized.

Yes I think that is a good point. Up to this point we have been tackling these as we go in PhET-iO design meetings, file by file. NumberControl is the first we have encountered that is big enough that I recommended not handling everything in it at once, but I could be convinced that that isn't the best approach. I think it is best when there is an easy way to test the changes (usually in studio), so ad-hoc is nice for that reason. I'll mark this for phet-io meeting to try to figure out the best approach for tackling our iceberg.

[pixelzoom]

Example of how you're using VBox to solve the problem?

[zepumph]

In NumberControl.createLayoutFunction3:

        // When the text of the title changes recompute the alignment between the title and content
        titleNode.on( 'bounds', () => {
          titleAndContentVBox.updateLayout();
        } );

To set this, open GFL in studio, and change the text on gravityForceLab.gravityForceLabScreen.view.massControl1.numberControl.titleNode.textProperty

[pixelzoom]

@pixelzoom can you think of how to "lock in" the height of the vBox on creation?

I think this problem is in the same category as blank spaces that appear in panels when UI components are hidden. The client will just have to live with it.

This is how text (and all Nodes) behave in general. If the maxWidth (or maxHeight) is exceeded, it gets scaled down. And if it's in a layout with other UI components, then things will move around. I don't recommend adding some title-specific hack (like a VStrut or AlignGroup) to createLayoutFunction3. If you don't want your title to scale down, then use a shorter title.

[zepumph]

I think this problem is in the same category as blank spaces that appear in panels when UI components are hidden. The client will just have to live with it.

This is how text (and all Nodes) behave in general. If the maxWidth (or maxHeight) is exceeded, it gets scaled down. And if it's in a layout with other UI components, then things will move around. I don't recommend adding some title-specific hack (like a VStrut or AlignGroup) to createLayoutFunction3. If you don't want your title to scale down, then use a shorter title.

I totally agree with you, and I'm very happy you said this! To me no further work is needed here. Thanks @pixelzoom for the review.

I created https://github.com/phetsims/phet-io/issues/1551 to investigate what further work is needed for PhET-iO dynamic layout both in NumberControl and in general. Closing