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

radio button labels are not localized #110

Closed
pixelzoom opened this issue May 22, 2018 · 11 comments
Closed

radio button labels are not localized #110

pixelzoom opened this issue May 22, 2018 · 11 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented May 22, 2018

This was discovered while investigating #109.

The Intro screen has this control panel:

screenshot_637

The labels for the radio buttons are not localized. I expected to find a string for each label in the translations file. But instead, I find only the word "All", and code that is doing string concatenation to create the labels. I.e. in IntroPlayPanel:

    var oneBall = new HBox( {
      spacing: BALL_RADIUS / 2,
      children: [ new BallNode( BALL_RADIUS ), new Text( timesString + '1', fontOptions ) ]
    } );
    var tenBalls = new HBox( {
      spacing: BALL_RADIUS / 2,
      children: [ new BallNode( BALL_RADIUS ), new Text( timesString + '10', fontOptions ) ]
    } );
    var allBalls = new HBox( {
      spacing: BALL_RADIUS / 2,
      children: [ new BallNode( BALL_RADIUS ), new Text( timesString + allString, fontOptions ) ]
    } );

There's seems to be an assumption here that these are mathematical expression and therefore do not need to be localization. But in this case we're using mathematical expressions as a (possibly locale-specific) shortcut for describing the radio buttons, and that may not translate well. For example, a translation of "one ball" may work better than "×1" in some languages.

So I think the following strings should be in the translated strings file (where \u00D7 is Unicode for the times operator):

"oneBall": {
  "value": "\u00D7 1"
},
"tenBalls": {
  "values": "\u00D7 10"
},
"allBalls": {
  "values": "\u00D7 All"
}

@amanda-phet Do you agree?

@pixelzoom
Copy link
Contributor Author

@amanda-phet Also wondering why we're using "All" instead of "100". Why not just use the value so that students know how many balls they are adding?

@amanda-phet
Copy link
Contributor

amanda-phet commented May 23, 2018

Also wondering why we're using "All" instead of "100". Why not just use the value so that students know how many balls they are adding?

It's not necessarily 100.. if they dropped 3, then xAll would drop 97 more. We want students to be able to run trials of 100 over and over, and the bins can't really handle more than that, so if someone drops 50 then 100 more, we'd be in trouble.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented May 23, 2018

@amanda-phet I don't understand your logic here. The same argument applies to ×10. Drop 91 balls by doing ×10 nine times, then ×1. Then do ×10 and it will only release 9 balls. It's not at all obvious that the upper limit is 100, or when/why the "Play" button is disabled.

@amanda-phet
Copy link
Contributor

You're right, I forgot the same thing applies to x10.

As for the Play button, we are aware that the design choice was problematic and I'm always open to discussing alternatives. Since this sim is being designed for a11y and we are making some minor design changes anyway, we could use this opportunity to make some other changes.

@amanda-phet
Copy link
Contributor

amanda-phet commented May 23, 2018

Zoom discussion with @pixelzoom . Decided to change the third radio button to ×100. This is helpful for translations and is also consistent with how the ×10 radio button behaves, so it doesn't seem inconsistent.

pixelzoom added a commit that referenced this issue May 23, 2018
Signed-off-by: Chris Malley <[email protected]>
pixelzoom added a commit that referenced this issue May 23, 2018
Signed-off-by: Chris Malley <[email protected]>
@pixelzoom
Copy link
Contributor Author

pixelzoom commented May 23, 2018

Change '× All' to '× 100' in above commit. Also use MathSymbols.TIMES.

@pixelzoom
Copy link
Contributor Author

This change will be picked up the next time the sim is published from master. Leaving issues open and "ready for review" until then.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented May 23, 2018

One more thing that I noticed... There is a maxBallsIntro query parameter that can be used to change the max number of balls in the Intro screen. It's default values is 100. Rather than hardcoding the string "× 100", this value should be used to create the label. I made that change in the above commit. So if you run with ?maxBallsIntro=50, the radio button will be labeled with "× 50".

@pixelzoom
Copy link
Contributor Author

@amanda-phet please review 1.2.0-dev.4, then assign back to me. The only UI change is that "×All" is now "×100" by default. Or "×50" if you run with ?maxBallsIntro=50.

@amanda-phet
Copy link
Contributor

This looks like it's working well.

@amanda-phet amanda-phet assigned pixelzoom and unassigned amanda-phet Nov 13, 2020
@pixelzoom
Copy link
Contributor Author

👍🏻 Closing

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

2 participants