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

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

Open
Tracked by #407
liammulh opened this issue Jan 4, 2023 · 9 comments
Open
Tracked by #407

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

liammulh opened this issue Jan 4, 2023 · 9 comments

Comments

@liammulh
Copy link
Member

liammulh commented Jan 4, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member Author

liammulh commented Jan 5, 2023

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.

@liammulh liammulh removed their assignment Jan 5, 2023
@jessegreenberg
Copy link
Contributor

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

@liammulh
Copy link
Member Author

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

"netForce.description": {
"value": "Forces and Motion Basics. There is a heavily loaded cart on wheels sitting on a track. Attached to the left side of the cart is a thick rope with 4 knots spaced equally along the rope. Standing near this knotted rope is a group of 4 puller people. On the opposite side of the cart, a similar rope with 4 knots is attached to the right side of the cart. There is another group of 4 puller people standing near this rope. The centre position of the cart has been marked on the ground.",
"visible": false
},

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 liammulh assigned liammulh and unassigned jessegreenberg Feb 22, 2023
@liammulh
Copy link
Member Author

liammulh commented Mar 5, 2023

I am going to transfer this to Rosetta.

@liammulh
Copy link
Member Author

liammulh commented Mar 5, 2023

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

@liammulh
Copy link
Member Author

I caused this regression by not implementing the following:

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

@liammulh
Copy link
Member Author

@jessegreenberg, please close this issue and comment in phetsims/rosetta#411 when you republish FAMB off of master.

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

3 participants