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

Clearer advice-summary #16649

Merged
merged 2 commits into from
Dec 25, 2024
Merged

Clearer advice-summary #16649

merged 2 commits into from
Dec 25, 2024

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

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