phetsims / plinko-probability

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

radio button labels are not localized #110

Closed pixelzoom closed 3 years ago

pixelzoom commented 6 years ago

This was discovered while investigating https://github.com/phetsims/plinko-probability/issues/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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

pixelzoom commented 6 years ago

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 commented 6 years ago

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 commented 5 years ago

@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 commented 3 years ago

This looks like it's working well.

pixelzoom commented 3 years ago

👍🏻 Closing