front : fix added missing translation keys to sdd DetailsBox - #18094
front : fix added missing translation keys to sdd DetailsBox#18094BabuBahir wants to merge 1 commit into
Conversation
github-actions
Bot
added
area:front
|
@theocrsb would you like to have a first look at the PR :) |
|
/review @SharglutDev |
|
After correcting the translation, everything LGTM :) |
69b7fde to
ba2d511
Compare
SharglutDev
left a comment
SharglutDev
left a comment
There was a problem hiding this comment.
Thank you for the contribution ! Left some comments
| "speedDistanceDiagram": "GEV", | ||
| "speedDistanceSettings": { | ||
| "accelerating": "En accélération", | ||
| "coasting": "En roue libre", |
There was a problem hiding this comment.
Haha this is indeed the literal translation but definitely not the one we want to use.
This is also a french expression meaning something like : "ffa" / "doing crazy things".
The business term for that is "Marche sur l'erre"
| "speedDistanceDiagram": "GSD", | ||
| "speedDistanceSettings": { | ||
| "accelerating": "Beschleunigung", | ||
| "coasting": "Ausrollen", |
There was a problem hiding this comment.
Business says this is more accurate
| "coasting": "Ausrollen", | |
| "coasting": "Auslauf", |
| const reticleValues = translations?.detailsBoxDisplay.reticleValues; | ||
| const translatedModeText = | ||
| modeText === '--' | ||
| ? modeText | ||
| : (reticleValues?.[modeText as keyof typeof reticleValues] ?? modeText); | ||
| const translatedEffortText = | ||
| reticleValues?.[effortText as keyof typeof reticleValues] ?? effortText; | ||
| const translatedElectricalProfileText = | ||
| electricalProfileText === 'incompatible' | ||
| ? (reticleValues?.incompatible ?? electricalProfileText) | ||
| : electricalProfileText; |
There was a problem hiding this comment.
This doesn't look very clean since we compute first the text and replace it some times with the translations.
Maybe it would be better if we could pass translations directly in drawCursor to use them over there to compute the text. Do you think it's acceptable for osrd-ui @emersion ?
There was a problem hiding this comment.
OK as in movieng out the null/Undefined checking to the parent ,
or how about
const reticleValues = translations?.detailsBoxDisplay.reticleValues as Record<string, string> | undefined;
const translatedModeText = reticleValues?.[modeText] ?? modeText;
const translatedEffortText = reticleValues?.[effortText] ?? effortText;
const translatedElectricalProfileText = reticleValues?.[electricalProfileText] ?? electricalProfileText;
There was a problem hiding this comment.
I get the point about compute-then-replace, but I'm not sure passing translations down to drawCursor is the right move : it's a canvas drawing function running on every mousemove, and we'd have to thread translations through DrawFunctionParams, which is shared with all the other draw* functions.
To me the actual issue is that modeText and effortText are typed as plain string in TrainDetails. That's what forces the as keyof typeof reticleValues cast. If we type them as unions instead :
export type ReticleMode = 'electric' | '--';
export type ReticleEffort = 'coasting' | 'accelerating' | 'decelerating';the lookup gets simple, no cast and no '--' special case :
const reticleValues = translations?.detailsBoxDisplay.reticleValues;
const translatedEffortText = reticleValues?.[effortText] ?? effortText;
...What do you think ?
There was a problem hiding this comment.
can we have a look @kmer2016 @SharglutDev
618e3c0 to
e71771d
Compare
e71771d to
e6c3d7a
Compare
e6c3d7a to
1942bef
Compare
There was a problem hiding this comment.
I believe the changes on this file is a mistake.
There was a problem hiding this comment.
I am seeing a lot of file changes which I feel wasn't done ...
There was a problem hiding this comment.
Github shows the file changed, but if I ignore the whitespace, it looks like no line has changed, and yet, the file is shown. I wonder if you (by mistake), didn’t change the permissions of the file (read/write/executable also known as rwx)? Anyway, if you’re sure no useful change has been done on this file, a git restore <the file> should help.
dd93b75 to
7b3a884
Compare
7ed5204 to
abc0b2f
Compare
5 participants
WHAT
Fixes #14931
How
Keys were added to speedDistanceSettings .json for the Details Box in SDD
So now the translation Appears
Reulst
Formatted
Caution :
There already exists a key
simulationResults.electricalProfiles.incompatible. I added asimulationResults.speedDistanceSettings.incompatible. So does this makessimulationResults.electricalProfiles.incompatibleredundant . Was this key intentionally added ?