Skip to content

Conversation

@dav1312
Copy link
Contributor

@dav1312 dav1312 commented Dec 22, 2024

Capitalize errors so they match acpl and accuracy.
Separate players when there is no learn from your mistakes button.
Apply a bold style to player names to make them stand out.
Remove margin 0 when the screen is small.

Before After
image image
image image
image image

@ornicar ornicar merged commit 50a387f into lichess-org:master Dec 25, 2024
3 checks passed
@superuser-does
Copy link
Collaborator

There has been a issue logged on the translation platform regarding the capitalisation:

My translation in Galician for both strings (1-3-25) shows all words in capital letters, which is wrong:
%s Metida De Zoca

This also happens in other languages.

text-transform: capitalize is quite a blunt technique that doesn't work for many languages where e.g. "de" should never be capitalised. This should be held in the source string if you want to change it, but I believe visual balance could be achieved through layout instead.

Would you consider a different solution please @dav1312?

@dav1312
Copy link
Contributor Author

dav1312 commented Jan 4, 2025

My bad, I guess changing the source string is a better idea.

@superuser-does
Copy link
Collaborator

superuser-does commented Jan 4, 2025

Fair enough. If the solution will be changing some source strings,

I would prefer to change the averageCentipawnLoss and accuracy strings when it comes to changing source strings. This will distinguish the analysis results from White and Black used in anonymous games. Per convention, White and Black are capitalised, as they are being used in place of names.

So all the results being in lower case would provide some helpful distinction, without adding any padding for example.

Let me know if you are preparing a pull request, otherwise I can do the PR myself.

@dav1312
Copy link
Contributor Author

dav1312 commented Jan 4, 2025

So all the results being in lower case would provide some helpful distinction, without adding any padding for example.

Not sure I like how it looks in lowercase...
image

Maybe ideally we should switch to the Lichess beta style
Screenshot_20250104_130627

@superuser-does
Copy link
Collaborator

I wasn't aware the mobile app did it like that. In this case, should uppercase nbInaccuracies, nbMistakes and nbBlunders in the source strings, as mobile is most probably already using a capitalize transformation to achieve that.

Say if you are happy with it below and leave me to process it as I can do a trick with Crowdin's pre-translation feature, which will at least ping every translator to review the re-published strings.

@dav1312
Copy link
Contributor Author

dav1312 commented Jan 4, 2025

Sure, capitalize the strings (and revert my capitalize change)

superuser-does added a commit that referenced this pull request Jan 5, 2025
New key introduced; the existing key will be retained in /translation/source until Crowdin pre-translation is complete.
superuser-does added a commit that referenced this pull request Jan 5, 2025
New keys introduced; the existing keys will be retained in /translation/source until Crowdin pre-translation is complete.
The old keys should be deleted only after the pre-translations have been pulled into master.
@superuser-does
Copy link
Collaborator

I have created the pull request as promised in #16731.

As a side note:

[...] mobile is most probably already using a capitalize transformation to achieve that.

I tested the app with the offending language (Galician). This particular string renders as %s Metida de zoca in the app instead of %s Metida De Zoca on the website.
Either the app is more language-aware than my browser and it's inheriting this from Android for example, or the app uses a different type of text transformation that only capitalises the first letter after %s. I don't know enough Flutter to say for sure.

Still - for assurance, and as a principle, we should avoid system-driven transformations when it comes to i18n. So pull #16731 should do good on the mobile side as well.

@dav1312
Copy link
Contributor Author

dav1312 commented Jan 5, 2025

different type of text transformation that only capitalises the first letter

If its using css it might be using ::first-letter the problem is that that only works block-level elements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants