phetsims / collision-lab

"Collision Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 4 forks source link

Translated strings should not be used for NumberProperty units #212

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

In BallNode.js:

    // Create the number display for the speed of the Ball, which appears above the ball. To be positioned later.
    const speedNumberDisplay = new PlayAreaNumberDisplay( ball.speedProperty, {
      visibleProperty: valuesVisibleProperty,
      valuePattern: StringUtils.fillIn( collisionLabStrings.pattern.vectorSymbolEqualsValueSpaceUnits, {
        symbol: collisionLabStrings.symbol.velocity,
        units: collisionLabStrings.units.metersPerSecond
      } )
    } );

    // Create the number display for the momentum of the Ball, which appears below the ball. To be positioned later.
    const momentumNumberDisplay = new PlayAreaNumberDisplay( ball.momentumMagnitudeProperty, {
      visibleProperty: valuesVisibleProperty,
      valuePattern: StringUtils.fillIn( collisionLabStrings.pattern.vectorSymbolEqualsValueSpaceUnits, {
        symbol: collisionLabStrings.symbol.momentum,
        units: collisionLabStrings.units.kilogramMetersPerSecond
      } )
    } );

Values for the units options to NumberDisplay must be in units.js. So the above will likely fail for anything other than English.

pixelzoom commented 3 years ago

Fixed in the above commit. @jonathanolson please review.

jonathanolson commented 3 years ago

Looks good to me, thanks!