search

phetsims / resistance-in-a-wire

"Resistance in a Wire" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/resistance-in-a-wire
GNU General Public License v3.0
1 stars 4 forks source link

Rho symbol does not translate #187

Closed rea-laura closed 5 years ago

rea-laura commented 5 years ago

Test Device

Pauling

Operating System

iOS 11.4.1

Browser

Safari 11.4.1

Problem Description

For https://github.com/phetsims/QA/issues/219: Rho symbols in equation and in slider bar panel do not translate during string test queries. This symbol does translate in the published version of the sim.

The issue appears on all operating systems and browsers tested thus far (will update if this changes.)

Steps to Reproduce

1) open the dev version of the sim: https://phet-dev.colorado.edu/html/resistance-in-a-wire/1.6.0-rc.1/phet/resistance-in-a-wire_en_phet.html 2) append URL with ?stringTest=double, ?stringTest=long, or any other string test. 3) rho character will not translate with the string tests.

Visuals image

zepumph commented 5 years ago

@ariel-phet when reworking this sim a while back, I think I remember talking to you about this. I thought we decided that this was okay. Can you confirm?

ariel-phet commented 5 years ago

@zepumph looking through old issues, I don't see anything related to the translation of rho. In fact, in an earlier round of dev testing on 1.5.0-rc.8 it seems this was translatable (https://github.com/phetsims/resistance-in-a-wire/issues/173). I know we have punted on certain things, but not sure why we wouldn't make this symbol translatable unless there was a compelling reason.

jessegreenberg commented 5 years ago

A decision on this may have come from https://github.com/phetsims/scenery-phet/issues/353, using MathSymbols.js.

jbphet commented 5 years ago

I read through the issue the @jessegreenberg linked above, and translatability was discussed for Trig Tour, but not for RIAW. I just looked through the RIAW translations, and there are two simulations in which the rho symbol is translated. One is Slovenian (locale=sl), and it looks legit to me:

image

The other is Tibetan (locale=bo), and it looks seriously messed up, so I don't think it's worth considering in this discussion.

Since there is at least one translation for this symbol, and since the work done to change it to a common math symbol makes the symbol untranslatable, it's my opinion that we should revert the change and keep the rho symbol translatable.

There are also several translations of the Ohms symbol Ω, which was converted to use our common math symbols at the same time. Here's an example from the Belarusian (locale=be) version:

Belarusian image

That too should be reverted in my opinion.

The commit in question is https://github.com/phetsims/resistance-in-a-wire/commit/8ef7861a780ee272ab39006268eedef7f13a4f56#diff-6348d9f4d7aa9bb88a51fab3adfe0d0f.

@ariel-phet - assigning to you for any final thoughts, and if you agree with me on this please assign it back and I'll make it happen. Tagging @zepumph in case he has any objections.

ariel-phet commented 5 years ago

@jbphet I agree with your assessment wholeheartedly

jbphet commented 5 years ago

I have restored the ability to translate rho (ρ) and omega (Ω) on master and on the 1.6 release branch.

zepumph commented 5 years ago

@jbphet instead of removing the MathSymbols support, can we add the translatable chars to that file, so that it is still consolidated?

So in MathSymbols.js something like:

    OHMS: ohmsSymbolString,
jbphet commented 5 years ago

@zepumph - I thought about this, but decided against because MathSymbols.js is common code, and I felt like this could lead to unintended consequences. To be honest, though, I didn't spend a lot of time considering the pros and cons. If you'd like to pursue it, and possibly discuss at dev meeting, that'd be okay with me.

jbphet commented 5 years ago

Having said that, I'm not going to hold off publication of RIAW waiting for a resolution on this. I don't think the consequences of publishing with this fix are too dire, and I can follow up with some adjustments to the common code file on master if necessary later.

zepumph commented 5 years ago

That sounds like a great plan. Please proceed with RIAW publication.

I will mark for developer meeting with one main question to discuss: "should characters in MathSymbols.js be translatable?"

We decided before that MathSymbols.js was a good place consolidating certain symbols, but in this issue we found that doing that for "ohms" disabled translation.

I personally think it would be best to make symbols in that file translatable, but I don't necessarily know if there are drawbacks to having translatables like that in common code, and/or if there will be complications if the same symbol, used in multiple sims, is translated into different things depending on context.

pixelzoom commented 5 years ago

I will mark for developer meeting with one main question to discuss: "should characters in MathSymbols.js be translatable?"

If you mean should all characters in MathSymbols.js, the answer is "no". There are some universal symbols that should not be translatable.

If you mean is it OK to add translatable characters to MathSymbols.js... I think that would be OK. The English strings should live in scenery-phet-strings_en.json, and should be imported to MathSymbols.js using the string! plugin.

jessegreenberg commented 5 years ago

We went through something similar in trig-tour around the time MathSymbols was discussed. I found this issue https://github.com/phetsims/trig-tour/issues/88 where we decided to use MathSymbols instead of the translatable strings. In https://github.com/phetsims/trig-tour/issues/88#issuecomment-373266130 we found that no translators were changing the math symbols in trig tour.

pixelzoom commented 5 years ago

@jessegreenberg I guess it depends on what you mean by "math symbols". In Graphing Lines, Graphing Quadratics and similar sims, we decided to localize x,y,a,b,c,m, etc. And we've seen that a couple of translators have actually changed these.

jessegreenberg commented 5 years ago

Looks like symbols removed from translatable strings in that issue were infinity, pi, plus/minus, and theta, and it makes sense that those are not translatable. x and y were left for i18n.

pixelzoom commented 5 years ago

@zepumph Is there still something here that we need to discuss at developer meeting?

zepumph commented 5 years ago

@zepumph Is there still something here that we need to discuss at developer meeting?

Yes the main question is if we, as a team, allow translatable strings in MathSymbols.js. I didn't know if it went against what people expected that file to look like. I think this issue should be solved by adding ohms back to MathSymbols.js as a translatable string.

Looks like symbols removed from translatable strings in that [trig tour] issue were infinity, pi, plus/minus, and theta, and it makes sense that those are not translatable. x and y were left for i18n.

We found that ohms were translatable, I would question why pi or theta would never be translatable and ohms would.

pixelzoom commented 5 years ago

@zepumph said:

We found that ohms were translatable, I would question why pi or theta would never be translatable and ohms would.

This seems like an argument for allowing translatable symbols in MathSymbols, so that we could make a symbol translatable without having to change client code.

pixelzoom commented 5 years ago

12/13/18 dev meeting:

pixelzoom commented 5 years ago

I'll handle fixing up MathSymbols.js. Put the translatable symbols in a section of MathSymbols with a nice comment about what should be translatable and why.

Then assign to @jbphet to work into 1.6 release of RIAW.

High priority.

pixelzoom commented 5 years ago

MathSymbols.js has been "fixed up" in the above commit.

I added MathSymbols .RESISTIVITY, for use in RIAW. The convention I choose for string keys is (e.g.) "symbol.resistivity. @jbphet feel free to change this convention if you prefer something else.

One note: Things like resistivity and ohms are not technically math symbols, so there's a little dissonance with them being in MathSymbols.js. But it doesn't bother me enough to want to address it. @jbphet if you feel otherwise, let me know.

KatieWoe commented 5 years ago

Confirmed fixed for 1.6.0-rc.2. It looks like discussion is still going, so I'll let you close if ready @jbphet

jbphet commented 5 years ago

Some work has been done on this to incorporate the new, translatable, common code symbols. As a result, this should be re-verified on the next RC.

Also, do not close this issue until it has been verified that the translations are correct after publication. It will likely fall to me to do this.

jbphet commented 5 years ago

I've just verified the translations on the newly published 1.6.0 version, and they look good. Closing.