search

phetsims / forces-and-motion-basics

"Forces and Motion: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/forces-and-motion-basics
GNU General Public License v3.0
7 stars 10 forks source link

Republish so that `.description` string keys are no longer present #298

Open liammulh opened 1 year ago

liammulh commented 1 year ago

It looks like FAMB has some old non-Kosher a11y description string keys:

Screen Shot 2023-01-04 at 3 36 08 PM

We don't want translators to translate these strings, but we can't really strip these string keys out programmatically because there are other valid string keys with .description in them.

It looks like on master, these string keys are no longer present, so one way to fix this would be to republish from master.

Assigning @jessegreenberg since he is the responsible dev for FAMB.

jessegreenberg commented 1 year ago

Thanks @liammulh, sorry about that. Would it work to do a maintenance release on the published sim somehow? Republishing from master would be very difficult at this point.

Would it break anything to just remove them and their usages from the published 2.3 branch?

jessegreenberg commented 1 year ago

A round of maintenance releases has completed and a new one will begin on Tuesday. We have a little window until then to do work in release branches.

liammulh commented 1 year ago

Would it break anything to just remove them and their usages from the published 2.3 branch?

It wouldn't break anything from the perspective of the translation utility. However, if you're going to change things on the published 2.3 branch, and you're worried about something breaking, why not prepend the .description string keys with a11y? For example, netForce.description would become a11y.netForce.description or a11y.netForce?

The translation utility prevents string keys with a11y in them from being translated, so these string keys would no longer show up in the translation utility. And then since the string keys would still be present in the sim, presumably nothing would break.

That being said, if you think removing these string keys wouldn't break anything in the sim and would be easier than what I suggested, then go ahead and remove them.

jessegreenberg commented 1 year ago

Thanks @liammulh, great. If it's safe to remove them I will go ahead with that.

liammulh commented 1 year ago

@jessegreenberg showed me that these strings have a visible key, which is set to false:

https://github.com/phetsims/forces-and-motion-basics/blob/cf9242a8863dfecd8eec669b5dd3bcca4bb77b5f/forces-and-motion-basics-strings_en.json#L75-L78

This is the old way of hiding string keys, and I think at some point the translation utility stopped supporting this. However, it would be (I think) easy to add a patch to the translation utility to hide these string keys.

liammulh commented 1 year ago

I am going to transfer this to Rosetta.

liammulh commented 1 year ago

Actually, I can't, lol. Never mind.

liammulh commented 1 year ago

I caused this regression by not implementing the following:

https://github.com/phetsims/rosetta/blob/5ec74679b3f1c1cdd30f617c47f5bb2c0c0a181c/js/routeHandlers.js#L274-L275

liammulh commented 1 year ago

@jessegreenberg, please close this issue and comment in https://github.com/phetsims/rosetta/issues/411 when you republish FAMB off of master.