phetsims / build-a-nucleus

"Build a Nucleus" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 5 forks source link

Why do `<span>` tags have inline styling? #76

Closed liammulh closed 1 year ago

liammulh commented 1 year ago

From Slack:

Liam Mulhall 4 minutes ago Is it okay if a sim has <span> tags in its strings? I saw this when I went to the translation form for BAN:

ban-strings

Jonathan Olson 3 minutes ago Seems fine in general, however I’m not sure why the style is in the translated string. I’d expect that to be hardcoded in the sim :+1: 1

Liam Mulhall 3 minutes ago It's a prototype, so they might change.

John Blanco :house_with_garden: 3 minutes ago Probably worth a question issue in GitHub for whoever set the strings up that way. :+1: 1

Liam Mulhall 2 minutes ago Okay, I'll open up an issue in the BAN repo.

It looks like @Luisav1 and @marlitas are the responsible devs, so assigning them.

marlitas commented 1 year ago

After a short chat with @chrisklus it looks like this was done to create even spacing among the labels in the Half-Life Timescale Dialog.

Screenshot 2023-02-17 at 9 22 06 AM

I looked into changing this, and I do believe we can hardcode it through the sim by setting the monospace font when the Text element is created. This would require some code restructuring (the css is overwriting Text font options in a couple of places, which I don't love).

Before I continue moving forward with this:

  1. @liammulh / @jbphet What are the repercussions of changing string values when there is an already published prototype? Specifically when this sim goes to full publication?
  2. I want to check-in with @Luisav1 to also confirm the motivation/need and what I'm seeing in the code.
liammulh commented 1 year ago

What are the repercussions of changing string values when there is an already published prototype? Specifically when this sim goes to full publication?

There is a big red warning in the translation utility that strings in prototype sims might change, so it's preferable to change the strings before publication rather than after.

liammulh commented 1 year ago

What are the repercussions of changing string values when there is an already published prototype? Specifically when this sim goes to full publication?

To answer the original question, the translators will have to re-translate the string if they've already translated it. But assuming they've read the warning, they are okay with that.

jbphet commented 1 year ago

What @liammulh says about the translators re-translating is true, but if you use the same string keys, this will look pretty bad in the translated versions. I had a similar issue come up for Greenhouse Effect, and I did a manual fix of all of the translations before re-publishing it. It was essentially just a search and replace of the strings, so it wasn't hard, but it had to be timed to be done just before the sim was re-published. You could log an issue and plan to do the same thing.

Alternatively, you could change the English strings, then fix the translations, and then publish a maintenance release of the prototype. That would solve the problem and there would be no risk of missing this step with the next publication.

marlitas commented 1 year ago

@Luisav1 and I decided that since her time on this sim is limited we will put this issue on hold until the sim is ready for publishing.

zepumph commented 1 year ago

We are getting ready to sprint on this. Marking off hold.

zepumph commented 1 year ago

@Luisav1 and I discussed this today, and would like to experiment with making the first letter its own Node. Then we can use an AlignBox to ensure the correct spacing.

zepumph commented 1 year ago

This is blocking me testing stringTest=dynamic because of the html formatting. I'm going to work on this in correlation with https://github.com/phetsims/build-a-nucleus/issues/90. @marlitas recommended grid box with 3 columns: the letter, the hyphen, and the i18n sentence.

zepumph commented 1 year ago
```diff Subject: [PATCH] dynamic locale adjustments to the "Available Decays" panel, https://github.com/phetsims/build-a-nucleus/issues/90 --- Index: build-a-nucleus-strings_en.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/build-a-nucleus-strings_en.json b/build-a-nucleus-strings_en.json --- a/build-a-nucleus-strings_en.json (revision 5bf96458622725cddb04daebfb801452771b14ff) +++ b/build-a-nucleus-strings_en.json (date 1689962899555) @@ -36,64 +36,64 @@ "value": "Half-Life Timescale" }, "timeForLightToCrossANucleus": { - "value": " - Time for light to cross a nucleus" + "value": "Time for light to cross a nucleus" }, "timeForLightToCrossAnAtom": { - "value": " - Time for light to cross an atom" + "value": "Time for light to cross an atom" }, "timeForLightToCrossOneThousandAtoms": { - "value": " - Time for light to cross 1000 atoms" + "value": "Time for light to cross 1000 atoms" }, "timeForSoundToTravelOneMillimeter": { - "value": " - Time for sound to travel 1 mm" + "value": "Time for sound to travel 1 mm" }, "aBlinkOfAnEye": { - "value": " - A blink of an eye" + "value": "A blink of an eye" }, "oneMinute": { - "value": " - One minute" + "value": "One minute" }, "oneYear": { - "value": " - One year" + "value": "One year" }, "averageHumanLifespan": { - "value": " - Average human lifespan" + "value": "Average human lifespan" }, "ageOfTheUniverse": { - "value": " - Age of the Universe" + "value": "Age of the Universe" }, "lifetimeOfLongestLivedStars": { - "value": " - Lifetime of longest lived stars" + "value": "Lifetime of longest lived stars" }, "A": { - "value": "A" + "value": "A" }, "B": { - "value": "B" + "value": "B" }, "C": { - "value": "C" + "value": "C" }, "D": { - "value": "D" + "value": "D" }, "E": { - "value": "E" + "value": "E" }, "F": { - "value": "F" + "value": "F" }, "G": { - "value": "G" + "value": "G" }, "H": { - "value": "H" + "value": "H" }, "I": { - "value": "I" + "value": "I" }, "J": { - "value": "J" + "value": "J" }, "availableDecays": { "value": "Available Decays" @@ -158,7 +158,6 @@ "availableDecaysInfoPanelText": { "value": "While other decays may exist, the available decays shown are the most common. In cases where there is no known decay, no available decays are shown." }, - "chartIntro": { "value": "Chart Intro" }, Index: js/decay/view/HalfLifeInfoDialog.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/decay/view/HalfLifeInfoDialog.ts b/js/decay/view/HalfLifeInfoDialog.ts --- a/js/decay/view/HalfLifeInfoDialog.ts (revision 5bf96458622725cddb04daebfb801452771b14ff) +++ b/js/decay/view/HalfLifeInfoDialog.ts (date 1689966316468) @@ -9,7 +9,7 @@ import Dialog from '../../../../sun/js/Dialog.js'; import buildANucleus from '../../buildANucleus.js'; -import { HBox, Node, Rectangle, RichText, Text, VBox } from '../../../../scenery/js/imports.js'; +import { Font, GridBox, HBox, Node, Rectangle, RichText, Text, VBox } from '../../../../scenery/js/imports.js'; import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; import BuildANucleusStrings from '../../BuildANucleusStrings.js'; import BANColors from '../../common/BANColors.js'; @@ -32,19 +32,19 @@ neutronCountProperty: TReadOnlyProperty, doesNuclideExistBooleanProperty: TReadOnlyProperty ) { - const leftSideStringProperties = [ - BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_A_NUCLEUS.timescaleStringProperty, - BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_AN_ATOM.timescaleStringProperty, - BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_ONE_THOUSAND_ATOMS.timescaleStringProperty, - BANTimescalePoints.TIME_FOR_SOUND_TO_TRAVEL_ONE_MILLIMETER.timescaleStringProperty, - BANTimescalePoints.A_BLINK_OF_AN_EYE.timescaleStringProperty + const leftSideTimescalePoints = [ + BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_A_NUCLEUS, + BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_AN_ATOM, + BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_ONE_THOUSAND_ATOMS, + BANTimescalePoints.TIME_FOR_SOUND_TO_TRAVEL_ONE_MILLIMETER, + BANTimescalePoints.A_BLINK_OF_AN_EYE ]; - const rightSideStringProperties = [ - BANTimescalePoints.ONE_MINUTE.timescaleStringProperty, - BANTimescalePoints.ONE_YEAR.timescaleStringProperty, - BANTimescalePoints.AVERAGE_HUMAN_LIFESPAN.timescaleStringProperty, - BANTimescalePoints.AGE_OF_THE_UNIVERSE.timescaleStringProperty, - BANTimescalePoints.LIFETIME_OF_LONGEST_LIVED_STARS.timescaleStringProperty + const rightSideTimescalePoints = [ + BANTimescalePoints.ONE_MINUTE, + BANTimescalePoints.ONE_YEAR, + BANTimescalePoints.AVERAGE_HUMAN_LIFESPAN, + BANTimescalePoints.AGE_OF_THE_UNIVERSE, + BANTimescalePoints.LIFETIME_OF_LONGEST_LIVED_STARS ]; // join the strings in each array, placing one on each line @@ -58,10 +58,41 @@ leading: 6 } ); }; + + const createGridBox = ( timescalePoints: BANTimescalePoints[] ): Node => { + + const grid = new GridBox(); + timescalePoints.forEach( ( timescalePoint, rowIndex ) => { + + grid.addRow( [ new Text( timescalePoint.timescaleMarkerStringProperty, { + font: new Font( { + size: '20px', + family: 'monospace', + weight: 'bold' + } ), + layoutOptions: { + minWidth: 10 // TODO:!!!! + } + } ), + new Text( '-', { + font: LEGEND_FONT, + layoutOptions: { + minWidth: 5 // TODO:!!!! + } + } ), + new Text( timescalePoint.timescaleDescriptionStringProperty, { + font: LEGEND_FONT, + layoutOptions: { align: 'left' } + } ) + ] ); + } ); + return grid; + }; + const legend = new HBox( { children: [ - createTextFromStringProperties( leftSideStringProperties ), - createTextFromStringProperties( rightSideStringProperties ) + createGridBox( leftSideTimescalePoints ) + // createGridBox( rightSideTimescalePoints ) ], spacing: 70, align: 'top', @@ -94,36 +125,23 @@ numberLineNodeAndLegend.centerX = contents.centerX; // the half-life's of the strings, in respective order - const halfLifeTime = [ - BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_A_NUCLEUS.numberOfSeconds, - BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_AN_ATOM.numberOfSeconds, - BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_ONE_THOUSAND_ATOMS.numberOfSeconds, - BANTimescalePoints.TIME_FOR_SOUND_TO_TRAVEL_ONE_MILLIMETER.numberOfSeconds, - BANTimescalePoints.A_BLINK_OF_AN_EYE.numberOfSeconds, - BANTimescalePoints.ONE_MINUTE.numberOfSeconds, - BANTimescalePoints.ONE_YEAR.numberOfSeconds, - BANTimescalePoints.AVERAGE_HUMAN_LIFESPAN.numberOfSeconds, - BANTimescalePoints.AGE_OF_THE_UNIVERSE.numberOfSeconds, - BANTimescalePoints.LIFETIME_OF_LONGEST_LIVED_STARS.numberOfSeconds - ]; - - // the labels on the half-life's, in respective order - const halfLifeLabels = [ - BuildANucleusStrings.AStringProperty, - BuildANucleusStrings.BStringProperty, - BuildANucleusStrings.CStringProperty, - BuildANucleusStrings.DStringProperty, - BuildANucleusStrings.EStringProperty, - BuildANucleusStrings.FStringProperty, - BuildANucleusStrings.GStringProperty, - BuildANucleusStrings.HStringProperty, - BuildANucleusStrings.IStringProperty, - BuildANucleusStrings.JStringProperty + const timescalePoints = [ + BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_A_NUCLEUS, + BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_AN_ATOM, + BANTimescalePoints.TIME_FOR_LIGHT_TO_CROSS_ONE_THOUSAND_ATOMS, + BANTimescalePoints.TIME_FOR_SOUND_TO_TRAVEL_ONE_MILLIMETER, + BANTimescalePoints.A_BLINK_OF_AN_EYE, + BANTimescalePoints.ONE_MINUTE, + BANTimescalePoints.ONE_YEAR, + BANTimescalePoints.AVERAGE_HUMAN_LIFESPAN, + BANTimescalePoints.AGE_OF_THE_UNIVERSE, + BANTimescalePoints.LIFETIME_OF_LONGEST_LIVED_STARS ]; // create and add the half-life arrow and label - for ( let i = 0; i < halfLifeTime.length; i++ ) { - halfLifeNumberLineNode.addArrowAndLabel( halfLifeLabels[ i ], halfLifeTime[ i ] ); + for ( let i = 0; i < timescalePoints.length; i++ ) { + const timescalePoint = timescalePoints[ i ]; + halfLifeNumberLineNode.addArrowAndLabel( timescalePoint.timescaleMarkerStringProperty, timescalePoints[ i ].numberOfSeconds ); } const titleNode = new Text( BuildANucleusStrings.halfLifeTimescaleStringProperty, { Index: js/decay/model/BANTimescalePoints.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/decay/model/BANTimescalePoints.ts b/js/decay/model/BANTimescalePoints.ts --- a/js/decay/model/BANTimescalePoints.ts (revision 5bf96458622725cddb04daebfb801452771b14ff) +++ b/js/decay/model/BANTimescalePoints.ts (date 1689965261288) @@ -11,82 +11,76 @@ import buildANucleus from '../../buildANucleus.js'; import BuildANucleusStrings from '../../BuildANucleusStrings.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; -import PatternStringProperty from '../../../../axon/js/PatternStringProperty.js'; -import Property from '../../../../axon/js/Property.js'; const SECONDS_IN_A_YEAR = 365 * 24 * 60 * 60; // 365 days x 24 hrs/day x 60 min/hr x 60 sec/min const TIME_FOR_LIGHT_TO_CROSS_AN_ATOM = Math.pow( 10, -19 ); -// TODO: i18n this pattern perhaps? or discuss further https://github.com/phetsims/build-a-nucleus/issues/90 -const patternStringProperty = new Property( '{{first}}{{second}}' ); - class BANTimescalePoints extends EnumerationValue { public static readonly TIME_FOR_LIGHT_TO_CROSS_A_NUCLEUS = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.AStringProperty, - second: BuildANucleusStrings.timeForLightToCrossANucleusStringProperty - } ), Math.pow( 10, -23 ) ); + BuildANucleusStrings.AStringProperty, + BuildANucleusStrings.timeForLightToCrossANucleusStringProperty, + Math.pow( 10, -23 ) + ); public static readonly TIME_FOR_LIGHT_TO_CROSS_AN_ATOM = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.BStringProperty, - second: BuildANucleusStrings.timeForLightToCrossAnAtomStringProperty - } ), TIME_FOR_LIGHT_TO_CROSS_AN_ATOM ); + BuildANucleusStrings.BStringProperty, + BuildANucleusStrings.timeForLightToCrossAnAtomStringProperty, + TIME_FOR_LIGHT_TO_CROSS_AN_ATOM + ); public static readonly TIME_FOR_LIGHT_TO_CROSS_ONE_THOUSAND_ATOMS = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.CStringProperty, - second: BuildANucleusStrings.timeForLightToCrossOneThousandAtomsStringProperty - } ), + BuildANucleusStrings.CStringProperty, + BuildANucleusStrings.timeForLightToCrossOneThousandAtomsStringProperty, TIME_FOR_LIGHT_TO_CROSS_AN_ATOM * 1000 ); public static readonly TIME_FOR_SOUND_TO_TRAVEL_ONE_MILLIMETER = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.DStringProperty, - second: BuildANucleusStrings.timeForSoundToTravelOneMillimeterStringProperty - } ), 2e-6 ); + BuildANucleusStrings.DStringProperty, + BuildANucleusStrings.timeForSoundToTravelOneMillimeterStringProperty, + 2e-6 + ); public static readonly A_BLINK_OF_AN_EYE = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.EStringProperty, - second: BuildANucleusStrings.aBlinkOfAnEyeStringProperty - } ), 1 / 3 ); + BuildANucleusStrings.EStringProperty, + BuildANucleusStrings.aBlinkOfAnEyeStringProperty, + 1 / 3 + ); public static readonly ONE_MINUTE = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.FStringProperty, - second: BuildANucleusStrings.oneMinuteStringProperty - } ), 60 ); + BuildANucleusStrings.FStringProperty, + BuildANucleusStrings.oneMinuteStringProperty, + 60 + ); public static readonly ONE_YEAR = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.GStringProperty, - second: BuildANucleusStrings.oneYearStringProperty - } ), SECONDS_IN_A_YEAR ); + BuildANucleusStrings.GStringProperty, + BuildANucleusStrings.oneYearStringProperty, + SECONDS_IN_A_YEAR + ); public static readonly AVERAGE_HUMAN_LIFESPAN = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.HStringProperty, - second: BuildANucleusStrings.averageHumanLifespanStringProperty - } ), 72.6 * SECONDS_IN_A_YEAR ); + BuildANucleusStrings.HStringProperty, + BuildANucleusStrings.averageHumanLifespanStringProperty, + 72.6 * SECONDS_IN_A_YEAR + ); public static readonly AGE_OF_THE_UNIVERSE = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.IStringProperty, - second: BuildANucleusStrings.ageOfTheUniverseStringProperty - } ), 13.77e9 * SECONDS_IN_A_YEAR ); + BuildANucleusStrings.IStringProperty, + BuildANucleusStrings.ageOfTheUniverseStringProperty, + 13.77e9 * SECONDS_IN_A_YEAR + ); public static readonly LIFETIME_OF_LONGEST_LIVED_STARS = new BANTimescalePoints( - new PatternStringProperty( patternStringProperty, { - first: BuildANucleusStrings.JStringProperty, - second: BuildANucleusStrings.lifetimeOfLongestLivedStarsStringProperty - } ), 450e18 ); + BuildANucleusStrings.JStringProperty, + BuildANucleusStrings.lifetimeOfLongestLivedStarsStringProperty, + 450e18 + ); public static readonly enumeration = new Enumeration( BANTimescalePoints ); public constructor( - public readonly timescaleStringProperty: TReadOnlyProperty, + public readonly timescaleMarkerStringProperty: TReadOnlyProperty, + public readonly timescaleDescriptionStringProperty: TReadOnlyProperty, public readonly numberOfSeconds: number ) { super(); Index: js/decay/view/HalfLifeInformationNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/decay/view/HalfLifeInformationNode.ts b/js/decay/view/HalfLifeInformationNode.ts --- a/js/decay/view/HalfLifeInformationNode.ts (revision 5bf96458622725cddb04daebfb801452771b14ff) +++ b/js/decay/view/HalfLifeInformationNode.ts (date 1689966045769) @@ -25,10 +25,10 @@ class HalfLifeInformationNode extends Node { public constructor( halfLifeNumberProperty: TReadOnlyProperty, - isStableBooleanProperty: TReadOnlyProperty, - protonCountProperty: TReadOnlyProperty, - neutronCountProperty: TReadOnlyProperty, - doesNuclideExistBooleanProperty: TReadOnlyProperty ) { + isStableBooleanProperty: TReadOnlyProperty, + protonCountProperty: TReadOnlyProperty, + neutronCountProperty: TReadOnlyProperty, + doesNuclideExistBooleanProperty: TReadOnlyProperty ) { super(); // create and add the halfLifeNumberLineNode @@ -46,6 +46,10 @@ const halfLifeInfoDialog = new HalfLifeInfoDialog( halfLifeNumberProperty, isStableBooleanProperty, protonCountProperty, neutronCountProperty, doesNuclideExistBooleanProperty ); + + phet.joist.sim.isConstructionCompleteProperty.link( complete => { + complete && halfLifeInfoDialog.show(); + } ); // create and add the info button const infoButton = new InfoButton( { listener: () => halfLifeInfoDialog.show(),
zepumph commented 1 year ago

I got to a commit point, refactoring things to match exactly, but instead using grid box. I have some concerns:

zepumph commented 1 year ago

We will keep the monospace font because it is the easiest solution to the jumbled-ness that occurs below without it:

image

We will work on fixing the translations too.

zepumph commented 1 year ago

Ok. So we want to create a script that will basically convert all translations from something like:

"value": "<span style=\"font-family: monospace;\"><b>A</b></span>"

to A.

Here is a script:

```js // Copyright 2022, University of Colorado Boulder /* eslint-env node */ // imports const fs = require( 'fs' ); const path = require( 'path' ); const incorrectLetterStrings = [ 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J' ]; const problematicPrefix = ''; const problematicSuffix = ''; const translatedFiles = fs.readdirSync( '../babel/build-a-nucleus/', 'utf8' );//.slice( 0, 1 ); translatedFiles.forEach( filename => { const filenameParts = filename.replace( '.json', '' ).split( '_' ); const locale = filenameParts.slice( 1 ).join( '' ); const babelContents = JSON.parse( fs.readFileSync( path.join( '../babel/build-a-nucleus', filename ), 'utf-8' ) ); incorrectLetterStrings.forEach( letter => { if ( babelContents[ letter ] ) { const noStyle = babelContents[ letter ].value.replace( problematicPrefix, '' ).replace( problematicSuffix, '' ); if ( noStyle !== letter ) { console.log( locale, noStyle ); } } } ); } ); ```

And here is the output. It will only output situations in which the translation wasn't identical to the input (`"<span style=\"font-family: monospace;\">C"). These we may need to handle manually.

```am AA am BB am CC am DD am EE am FF am GG am HH am II am JJ arMA E ha A is A is B is C is D is E is F is G is H is I is J ny >A om AA om BB om CC om DD om EE om FF om GG om HH om II om JJ pt G ru А ru Б ru В ru Г ru Д ru Е ru Ж ru З ru И ru К sl Č sl D sl E sl F sl G sl H sl I sw A sw B sw C sw D sw E sw F sw G sw H sw I sw J uk А uk Б uk В uk Г uk Ґ uk Д uk Е uk Є uk Ж uk З ```

Questions:

REMEMBER THAT WE ABSOLUTELY DO NOT WANT TO DO THIS UNTIL RIGHT BEFORE PUBLICATION.

zepumph commented 1 year ago

Updated script that is parsing out all the weird cases so we can provide the right value for translations.

``` // Copyright 2022, University of Colorado Boulder /* eslint-env node */ // imports const fs = require( 'fs' ); const path = require( 'path' ); const incorrectLetterStrings = [ 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J' ]; const problematicPrefix = ''; const problematicSuffix = ''; const swTranslatedProblematicPrefix = '' const swTranslatedProblematicSuffix = ''; const translatedFiles = fs.readdirSync( '../babel/build-a-nucleus/', 'utf8' );//.slice( 0, 1 ); translatedFiles.forEach( filename => { const filenameParts = filename.replace( '.json', '' ).split( '_' ); const locale = filenameParts.slice( 1 ).join( '' ); const babelContents = JSON.parse( fs.readFileSync( path.join( '../babel/build-a-nucleus', filename ), 'utf-8' ) ); incorrectLetterStrings.forEach( letter => { if ( babelContents[ letter ] ) { const noStyle = babelContents[ letter ].value.replace( problematicPrefix, '' ).replace( problematicSuffix, '' ); /* doc time: is: space after the letter (just trim it) */ if ( noStyle.trim() === letter ) { // Space after letter, for locale `is` // console.log( noStyle === letter, `hi${noStyle[ 0 ]}hi${noStyle[ 1 ]}hi` ); } else if ( noStyle === `${letter}${letter}` ) { // This applies to am and om locales // console.log( 'AHHHH', noStyle ); } else if ( incorrectLetterStrings.includes( noStyle ) && noStyle !== letter ) { // sl and pt both incorrectly translated wrong characters, seems like a bug to me. Ask JB!!! /* console.log( 'AAAH', locale, noStyle, letter ); >>> AAAH pt G H AAAH sl D E AAAH sl E F AAAH sl F G AAAH sl G H AAAH sl H I AAAH sl I J */ } else if ( noStyle.replace( swTranslatedProblematicPrefix, '' ).replace( swTranslatedProblematicSuffix, '' ) === letter ) { // For sw, the html was translated. Get that outta there! // correctLetter = noStyle.replace( swTranslatedProblematicPrefix, '' ).replace( swTranslatedProblematicSuffix, '' ); // console.log( locale, noStyle, noStyle.replace( swTranslatedProblematicPrefix, '' ).replace( swTranslatedProblematicSuffix, '' ), letter ); } else if ( noStyle !== letter ) { // The rest are actually translated letter, and should be presevered // Cyrrlic А looks like A, but isn't console.log( locale, noStyle, noStyle.charCodeAt( 0 ), letter, letter.charCodeAt( 0 ) ); } } } ); } ); ```
zepumph commented 1 year ago

Ok, I'm linking https://github.com/phetsims/build-a-nucleus/issues/125 into this issue, which includes fixing the dashes which are no longer in i18n strings. The commit that broke translations and needs patching is https://github.com/phetsims/build-a-nucleus/commit/8cae8de53cde01d66ae453c79a1ae51b823db3e5.

Here is a script that is pretty much complete at fixing both problems:

```js // Copyright 2022, University of Colorado Boulder /* eslint-env node */ // imports const fs = require( 'fs' ); const path = require( 'path' ); const incorrectLetterStrings = [ 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J' ]; const problematicPrefix = ''; const problematicSuffix = ''; const swTranslatedProblematicPrefix = ''; const swTranslatedProblematicSuffix = ''; const translatedFiles = fs.readdirSync( '../babel/build-a-nucleus/', 'utf8' );//.slice( 0, 1 ); const handleLetters = ( babelContents, locale ) => { incorrectLetterStrings.forEach( letter => { if ( babelContents[ letter ] ) { const noStyle = babelContents[ letter ].value.replace( problematicPrefix, '' ).replace( problematicSuffix, '' ); /* doc time: is: space after the letter (just trim it) */ if ( noStyle.trim() === letter ) { // Space after letter, for locale `is` // console.log( noStyle === letter, `hi${noStyle[ 0 ]}hi${noStyle[ 1 ]}hi` ); babelContents[ letter ].value = letter; } else if ( noStyle === `${letter}${letter}` ) { // This applies to am and om locales // console.log( 'AHHHH', noStyle ); babelContents[ letter ].value = letter; } else if ( incorrectLetterStrings.includes( noStyle ) && noStyle !== letter ) { // sl and pt both incorrectly translated wrong characters, seems like a bug to me. Ask JB!!! /* console.log( 'AAAH', locale, noStyle, letter ); >>> AAAH pt G H AAAH sl D E AAAH sl E F AAAH sl F G AAAH sl G H AAAH sl H I AAAH sl I J */ babelContents[ letter ].value = letter; } else if ( noStyle.replace( swTranslatedProblematicPrefix, '' ).replace( swTranslatedProblematicSuffix, '' ) === letter ) { // For sw, the html was translated. Get that outta there! // correctLetter = noStyle.replace( swTranslatedProblematicPrefix, '' ).replace( swTranslatedProblematicSuffix, '' ); // console.log( locale, noStyle, noStyle.replace( swTranslatedProblematicPrefix, '' ).replace( swTranslatedProblematicSuffix, '' ), letter ); babelContents[ letter ].value = letter; } else if ( noStyle !== letter ) { // The rest are actually translated letter, and should be presevered // Cyrrlic А looks like A, but isn't babelContents[ letter ].value = noStyle; // console.log( 'actual letter translation', locale, noStyle, letter ); // TODO: double check for bad cases here! https://github.com/phetsims/build-a-nucleus/issues/76 } else { // The above should cover all cases, but log here just in case console.log( 'should not be here', locale, noStyle, letter ); // We should never get here!! } } } ); }; const keysWithDashes = [ 'timeForLightToCrossANucleus', 'timeForLightToCrossAnAtom', 'timeForLightToCrossOneThousandAtoms', 'timeForSoundToTravelOneMillimeter', 'aBlinkOfAnEye', 'oneMinute', 'oneYear', 'averageHumanLifespan', 'ageOfTheUniverse', 'lifetimeOfLongestLivedStars' ]; const handleDashes = ( babelContents, locale ) => { keysWithDashes.forEach( key => { if ( babelContents[ key ] ) { const value = babelContents[ key ].value.trim(); let newValue = value; if ( value.startsWith( '-' ) ) { newValue = value.slice( 1 ).trim(); } else if ( value.endsWith( '-' ) ) { newValue = value.slice( 0, value.length - 1 ).trim(); } if ( newValue.includes( '-' ) ) { // TODO: Check this out and make sure that usages of dashes here are not buggy! https://github.com/phetsims/build-a-nucleus/issues/76 // console.log( 'dash-cover', locale, newValue ); } babelContents[ key ].value = newValue; } } ); }; translatedFiles.forEach( filename => { const filenameParts = filename.replace( '.json', '' ).split( '_' ); const locale = filenameParts.slice( 1 ).join( '' ); const localeFilename = path.join( '../babel/build-a-nucleus', filename ); const babelContents = JSON.parse( fs.readFileSync( localeFilename, 'utf-8' ) ); handleLetters( babelContents, locale ); handleDashes( babelContents, locale ); fs.writeFileSync( localeFilename, JSON.stringify( babelContents, null, 2 ) ); } ); ```

Steps once we are ready to publish:

  1. Test to make sure that this is working well still. That means doing the console logs at the end of the dashes and letters functions to make sure that newly different and buggy translations didn't come in between now and publication.
  2. Run it
  3. Revert the unicode-specific changes
  4. commit it.
  5. Then we are ready to publish.
zepumph commented 1 year ago

I reviewed the above with @solaolateju and we both agree with the path forward.

zepumph commented 1 year ago
  • [x] one thing before continuing. The script above changes the unicode string ‪‪\u202a into the actually rendered character. I don't think we want to do this, so I need to look at babel to see how we keep the unicode language in the babel repo.

After discussing with JB, I'll just revert that trouble before continuing.

Nancy-Salpepi commented 1 year ago

Noting that this issue (I think 😂) is present in https://github.com/phetsims/qa/issues/977

Screenshot 2023-09-13 at 7 52 53 PM
Nancy-Salpepi commented 1 year ago

tagging https://github.com/phetsims/qa/issues/988 to keep this on your radar 📡.

zepumph commented 1 year ago

Alright we are ready to do this, pushed above. Proceeding to publication.