pH Meter value text is not aligned properly on the initial set state

[phet-steele]

1. In the State wrapper (also happens in the Studio wrapper when previewing the sim), enter the Macro or Micro screens.
2. If you entered the Macro screen, you can already see in the lower sim that the readout on the pH meter has the incorrect alignment. Dragging the probe over the solution will display text like this:

3. If you entered the Micro screen, you will see this text start off incorrectly in the lower sim:

Some notes!

• The issue is corrected once you swap to a different solute.
• The issue can easily return by pressing Reset All.
• I have not seen buggy behaviour on the My Solution screen. EDIT: Yea I do in Studio if I Preview a sim while the pH is negative.

Seen on Win 10 Chrome and FF. For phetsims/qa/issues/648.

[pixelzoom]

Thanks @phet-steele, I'll investigate. Sounds like code that's changing the displayed value, but forgetting to center it in the meter.

[pixelzoom]

PHMeterNode.js is the place to start.

[pixelzoom]

Here's a screenshot of the State wrapper, showing the downstream alignment problem for the pH meter in the Macro screen:

It's also a problem with the pH meter in the Micro screen:

I'm beginning to suspect a general problem with NumberDisplay. These 2 meters were recently changed to use NumberDisplay in #96.

But... I don't see this problem in other sims that use NumberDisplay (e.g. beers-law-lab, gas-properties, natural-selection).

[pixelzoom]

I made these changes in NumberDisplay, so that I can see the background with a red stroke:

-      stroke: options.backgroundStroke,
-      lineWidth: options.backgroundLineWidth,
+      stroke: 'red',
+      lineWidth: 3,

Below is a screenshot. NumberDisplay's background is in the correct position, and does not move. What is NOT correct is the alignment of the value on the background, which is the responsibility of NumberDisplay.

[pixelzoom]

Here's the NumberDisplay instantiation in MacroPHMeterNode.js:

    // pH display
    const numberDisplay = new NumberDisplay( pHProperty, PHScaleConstants.PH_RANGE, {
      decimalPlaces: PHScaleConstants.PH_METER_DECIMAL_PLACES,
      align: 'right',
      noValueAlign: 'center',
      cornerRadius: CORNER_RADIUS,
      xMargin: 8,
      yMargin: 5,
      textOptions: {
        font: new PhetFont( 28 ),
        textPropertyOptions: { phetioHighFrequency: true }
      },
      tandem: options.tandem.createTandem( 'numberDisplay' )
    } );

After moving the probe out of the solution, the "No value" (—) character is also misaligned in the downstream sim. It is supposed to be noValueAlign: 'center', but appears to be noValueAlign: 'left'.

[pixelzoom]

This is indeed a general problem with NumberDisplay. Here's the problem:

    const valueText = new Constructor( longestString, merge( {
      tandem: options.tandem.createTandem( 'valueText' )
    }, options.textOptions, {
      maxWidth: null // we are handling maxWidth manually, so we don't want to provide it initially.
    } ) );
...
    // display the value
    const numberObserver = value => {
...
      // horizontally align value in background
      const align = ( value === null ) ? options.noValueAlign : options.align;
      if ( align === 'center' ) {
        valueText.centerX = backgroundNode.centerX;
      }
      else if ( align === 'left' ) {
        valueText.left = backgroundNode.left + options.xMargin;
      }
      else { // right
        valueText.right = backgroundNode.right - options.xMargin;
      }
      valueText.centerY = backgroundNode.centerY;
    };
    numberProperty.link( numberObserver );

valueText is fully instrumented, including its textProperty. The State wrapper is saving and restoring that textProperty. But the alignment of valueText is handled in the numberProperty observer. So when the State wrapper restore textProperty, the alignment code is not called.

This can be fixed by moving the alignment code into an observer of valueText.boundsProperty, and I'll do that.

[pixelzoom]

Fixed in the above commit, tested in State Wrapper.

@phet-steele please verify in master. If it's OK, you can simply close this issue. Since this problem was introduced recently by #96, it was never published in a production version.

[phet-steele]

This looks fixed in master on my Win 10 Chrome.

[pixelzoom]

Thanks @phet-steele.