phetsims / unit-rates

"Unit Rates" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 2 forks source link

numeratorOptions and denominatorOptions are a mess #221

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

numeratorOptions and denominatorOptions are confused throughout. They are also the only case where @ts-expect-error was needed for TypeScript, in https://github.com/phetsims/unit-rates/issues/219. They should be cleaned up when there's time.

In DoubleNumberLine.ts:

export type AxisOptions = {
  axisLabel?: string; // label for the axis
  maxDecimals?: number; // maximum number of decimal places, integer >= 0
  maxDigits?: number; // maximum number of digits on the keypad
  trimZeros?: boolean; // whether to trim trailing zeros from decimal places
  pickerColor?: Color | string; // color of NumberPicker associated with the axis
  valueFormat?: string; // pattern used to display value for the axis
};
...
  numeratorOptions?: AxisOptions; // options specific to the rate's numerator
  denominatorOptions?: AxisOptions; // options specific to the rate's denominator

In ShoppingQuestionsFactory.ts

export type NumeratorOptions = WithRequired<AxisOptions, 'maxDecimals' | 'trimZeros'>;
export type DenominatorOptions = NumeratorOptions;
...
                          numeratorOptions: NumeratorOptions,
                          denominatorOptions: DenominatorOptions ): ShoppingQuestion {

In ShoppingScene.ts:

    this.numeratorOptions = combineOptions<NumeratorOptions>( {
...
    this.denominatorOptions = combineOptions<DenominatorOptions>( {

It might be nice to define an Axis class in the model, with instances numeratorAxis and denominatorAxis.

pixelzoom commented 1 year ago

Here's my first attempt at Axis.ts, not committed.

Axis.ts ```ts // Copyright 2023, University of Colorado Boulder /** * Axis is a data structure that describes on axis of a double number line. * * @author Chris Malley (PixelZoom, Inc.) */ import unitRates from '../../unitRates.js'; import { Color } from '../../../../scenery/js/imports.js'; import optionize from '../../../../phet-core/js/optionize.js'; import SunConstants from '../../../../sun/js/SunConstants.js'; type SelfOptions = { axisLabel?: string; // label for the axis maxDigits?: number; // maximum number of digits on the keypad maxDecimals?: number; // maximum number of decimal places, integer >= 0 trimZeros?: boolean; // whether to trim trailing zeros from decimal places pickerColor?: Color | string; // color of NumberPicker associated with the axis valueFormat?: string; // pattern used to display value for the axis }; export type AxisOptions = SelfOptions; export default class Axis { public readonly axisLabel: string; // label for the axis public readonly maxDecimals: number; // maximum number of decimal places, integer >= 0 public readonly maxDigits: number; // maximum number of digits on the keypad public readonly trimZeros: boolean; // whether to trim trailing zeros from decimal places public readonly pickerColor: Color | string; // color of NumberPicker associated with the axis public readonly valueFormat: string; // pattern used to display value for the axis public constructor( providedOptions?: AxisOptions ) { const options = optionize()( { // SelfOptions axisLabel: '', maxDigits: 4, maxDecimals: 2, trimZeros: false, pickerColor: 'black', valueFormat: SunConstants.VALUE_NUMBERED_PLACEHOLDER }, providedOptions ); this.axisLabel = options.axisLabel; this.maxDecimals = options.maxDecimals; this.maxDigits = options.maxDigits; this.trimZeros = options.trimZeros; this.pickerColor = options.pickerColor; this.valueFormat = options.valueFormat; } } unitRates.register( 'Axis', Axis ); ```
pixelzoom commented 1 year ago

Looking at published version 1.0.26, here is the info for the axes.

=== Fruit ===

Numerator axisLabel: UnitRatesStrings.dollars = 'dollars' maxDigits: 4 maxDecimals: 2 trimZeros: false pickerColor: 'black' valueFormat: UnitRatesStrings.pattern_0cost = '${0}'

Denominator axisLabel: itemData.pluralName maxDigits: 4 maxDecimals: 2 trimZeros: true pickerColor: 'red' valueFormat: SunConstants.VALUE_NUMBERED_PLACEHOLDER = '{0}'

=== Vegetables ===

Numerator axisLabel: UnitRatesStrings.dollars = 'dollars' maxDigits: 4 maxDecimals: 2 trimZeros: false pickerColor: 'black' valueFormat: UnitRatesStrings.pattern_0cost = '${0}'

Denominator axisLabel: itemData.pluralName maxDigits: 4 maxDecimals: 2 trimZeros: true pickerColor: 'orange' valueFormat: SunConstants.VALUE_NUMBERED_PLACEHOLDER = '{0}'

=== Candy ===

Numerator axisLabel: UnitRatesStrings.dollars = 'dollars' maxDigits: 4 maxDecimals: 2 trimZeros: false pickerColor: 'black' valueFormat: UnitRatesStrings.pattern_0cost = '${0}'

Denominator axisLabel: UnitRatesStrings.pounds = 'pounds' maxDigits: 4 maxDecimals: 2 trimZeros: true pickerColor: 'purple' valueFormat: SunConstants.VALUE_NUMBERED_PLACEHOLDER = '{0}'

=== Racing Lab ===

Numerator axisLabel: UnitRatesStrings.miles = 'miles' maxDigits: 5 maxDecimals: 1 trimZeros: true pickerColor: car.color = URColors.car1 = 'red' valueFormat: SunConstants.VALUE_NUMBERED_PLACEHOLDER = '{0}'

Denominator axisLabel: itemData.pluralName = 'hours' maxDigits: 4 maxDecimals: 2 trimZeros: true pickerColor: car.color = URColors.car2 = 'red' valueFormat: SunConstants.VALUE_NUMBERED_PLACEHOLDER = '{0}'

pixelzoom commented 1 year ago

Done in the above commits. The various numeratorOptions and denominatorOptions have been replaced by numeratorAxis: Axis and denominatorAxis: Axis throughout.

Tested by manually comparing to published 1.0.26 and the specification in https://github.com/phetsims/unit-rates/issues/221.

Closing.