phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

x-axis label is missing on "Fourier Components" chart. #233

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

After conversion of WavePacketComponentsChartNode to TypeScript, the x-axis label is missing:

screenshot_2518

Compare with https://phet-dev.colorado.edu/html/fourier-making-waves/1.0.13/phet/fourier-making-waves_en_phet.html:

screenshot_2519
pixelzoom commented 1 year ago

Relevant files: DomainChartNode.ts, WavePacketComponentsChartNode.ts.

Relevant commits:

pixelzoom commented 1 year ago

If you change the Domain (Function of) combobox, then the x-axis label appears. So it's either initially invisible, positioned incorrectly, or its text is not initially set.

I'm not 100% certain that this problem was introduced recently, or even by TypeScript conversion. I tried to bisect, and failed. Unfortunately I did not publish a baseline before starting TypeScript conversion. And the most recent dev version deployed from master is 1.1.0-dev.1 on 12/21/2022.

pixelzoom commented 1 year ago

Using grunt checkout-timestamp, I was able to identify when the problem appeared.

No problem on Nov 03 2022: grunt checkout-timestamp --repo="fourier-making-waves" --timestamp="Nov 03 2022"

Problem on Nov 04 2022: grunt checkout-timestamp --repo="fourier-making-waves" --timestamp="Nov 04 2022"

But I'm not sure that I believe or trust this, because the format of --timestamp is undocumented.

pixelzoom commented 1 year ago

In DomainChartNode.ts, I hid xZoomButtonGroup, and discovered that the initial position of xAxisLabel is incorrect. It should be vertically centered on the x-axis. Instead, it's bottom aligned with chartRectangle.

screenshot_2523

Related code in DomainChartNode.ts:

    // Position the x-axis label at the right of the chart. The y position depends on the y range. If the y range
    // is non-negative, then the y-axis label is aligned with the bottom of the chart. Otherwise it's vertically
    // centered on the chart.
    xAxisLabel.boundsProperty.link( bounds => {
      xAxisLabel.left = chartRectangle.right + FMWConstants.X_AXIS_LABEL_SPACING;
      if ( chartTransform.modelYRange.min >= 0 ) {
        xAxisLabel.bottom = chartRectangle.bottom;
      }
      else {
        xAxisLabel.centerY = chartRectangle.centerY;
      }
    } );

So my guess is that the value of chartTransform.modelYRange.min is incorrect when new WavePacketComponentsChartNode is called. And that means that options.chartTransformOptions to new WavePacketComponentsChartNode may be incorrect.

pixelzoom commented 1 year ago

Here's the default for modelXRange in DomainChartNode.ts:

      chartTransformOptions: {
        modelXRange: xAxisDescriptionProperty.value.createRangeForDomain( domainProperty.value, spaceMultiplier, timeMultiplier ),
pixelzoom commented 1 year ago

Fixed in the above commit. I have no idea how/when this was broken. But the fix suggests that it's been broken for a long time, before TypeScript conversion.

Closing.