Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for dynamic strings to content of "Word" accordion box #199

Open
pixelzoom opened this issue Jan 12, 2023 · 0 comments
Open

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 12, 2023

The content of the "Word" accordion box does not currently support dynamic strings. So if you run the sim with ?stringTest=dynamic and use the left/right arrow keys, you will not see any change to the contents of the accordion box.

The main reason for this is that we do not have string Properties for the secondary locale. Instead we have this Property in NumberSuiteCommonPreferences.ts, whose value is an object literal consisting of keys and static strings:

  public readonly secondLocaleStringsProperty: TReadOnlyProperty<IntentionalAny>;

And in WordAccordionBox.ts, we call NumberSuiteCommonConstants.numberToString to map an integer number to the word for that integer:

    // The word shown in the accordion box
    const wordStringProperty = new DerivedProperty(
      [ currentNumberProperty, isPrimaryLocaleProperty, numberPlayPreferences.secondLocaleStringsProperty ],
      ( currentNumber, isPrimaryLocale, secondLocaleStrings ) =>
        NumberSuiteCommonConstants.numberToString( secondLocaleStrings, currentNumber, isPrimaryLocale )
    );

    const wordText = new Text( wordStringProperty, {
      font: options.font,
      maxWidth: this.contentBoundsProperty.value.width - options.textOffsetX - TEXT_MARGIN
    } );

What really needs to happen is something like:

    const wordText = new Text( '', {
      font: options.font,
      maxWidth: this.contentBoundsProperty.value.width - options.textOffsetX - TEXT_MARGIN
    } );

    Multilink.multilink(
      [ currentNumberProperty, isPrimaryLocaleProperty, numberPlayPreferences.secondLocaleStringsProperty ],
      ( currentNumber, isPrimaryLocale, secondLocaleStrings ) => {
        wordText.stringProperty = NumberSuiteCommonConstants.numberToStringProperty( secondLocaleStrings, currentNumber, isPrimaryLocale );
   } ); 

... where NumberSuiteCommonConstants.numberToStringProperty does not currently exist.

PhET-iO is not included for the 1.0 release, and the only way to change string Properties is via PhET-iO/Studio. So we decided to leave this as a known problem for the 1.0 release, and label this issue as deferred.

@pixelzoom pixelzoom changed the title Add support for dynamic locale to content of "Word" accordion box Add support for dynamic strings to content of "Word" accordion box Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant