-
Notifications
You must be signed in to change notification settings - Fork 262
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
Revamp sight light text #4565
Revamp sight light text #4565
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I don't know, whether I should "approve" or simply comment. I'm going to do it anyway.)
"wikiLinkReferral", | ||
I18N.getText( | ||
"sightLight.wikiLinkReferral", | ||
"<i>wiki.rptools.info/index.php/Introduction_to_Lights_and_Sights</i>")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This format < i > sticks out like a sore thumb. Is it really necessary here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't format it as a link (preferred) because it won't actually open in a browser (or at least it didn't when I tried). I wanted it to stand out so people know what to copy-paste. I can't hard-code the colour because that may not be visible in some themes and linking to theme.css didn't work for utilising theme-sensitive styling like accent colour.
That left me with bold, italic, and underline as means of differentiation. Underline implies clickable which is inappropriate and bold fails my aesthetic text for the second part of a sentence. That left me with italic, the best of an admittedly stagnant pool of options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it have to be formatted at all? This is the only place I saw, where formatting is part of the "value". The other sections have the formatting in the "container". E.g., here it could be "...{sightLight.wikiLinkReferral}..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to help keep the translation text context I'm passing it as a parameter.
So the translator sees something like "referral.wikilink = find more in the wiki online at {0}."
Because it is being injected into the sentence it needs to carry its own formatting if I want to keep the tags out of the translatable text.
Formatting is not necessary, but I want it, so I coded it to be kept out of the way of everyone except developers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that part is never to be translated anyway. The chances are less than slim, that this would ever be referenced as wiki.rptools.info/index.php/es/introduccion_a_luces_y_vistas...
String syntaxHeading = | ||
""" | ||
</ul> | ||
<u><font size=5>${subheadingDefinitionSyntax}</font></u><br><br> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally dislike the idea of having the closing syntax (here < / u l > in 658) in a separate string from the beginning (here < u l > in 641). This is side-effect programming and hard to follow and latter to maintain valid html, when names and other items change. Line 640 is better.
<td${alignCellCenter}>+100</td> | ||
</tr> | ||
</table> | ||
"""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing as above. If such blocks are necessary, have the opening and closing tags in separate strings (called blaOpen and blaClose). That removes the implicit order for the first and last text blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Easy done. Shall implement.
* | ||
* @return String[] | ||
*/ | ||
private String[] generateHelpText() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe extract this into a helper class? The method length would suggest this and linters probably already complain about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bwahahaha... you should have seen the long version.
Depending on how fast we get the new form-based UI for light/sight this version may not last very long.
light.example.text.aura.rangeArcs = A series of cones starting every 30 ft and ending 0.4 ft later. | ||
light.example.text.aura.rangeCircles= A series of circles starting every 30 ft and ending 0.4 ft later. | ||
light.example.text.aura.sideFields = 4 cones with an arc of 90 degrees and range of 12.5, each one offset to cover a different region. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Editorial only: Why have justified "equals" here, when this isn't done anywhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Residual effect of multiple edits and probably Notepad++ disagreeing with Spotless.
@irisiflimsi All comments welcome. I've reworked this so many times in the last week I've lost some of my perspective of the whole. Since the main driver is to make the translation keys and strings easier to understand for the translation community, anything that makes it easier to read, understand, and maintain is worth noting. |
Identify the Bug or Feature request
resolves #4549
Description of the Change
Rewrote sight and light help texts to remove formatting and simplify translation components.
HTML is now constructed by substituting i18n text into HTML template string.
Possible Drawbacks
A lot more individual key/values need to be translated.
Languages with no active translators will get the old text.
Documentation Notes
Reformatted sight & light help texts for easier translation and easier transition to new UI.
Release Notes
Reformatted sight & light help texts
This change is