-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fix missing translation on Simul & FAQ page #13922
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
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.
Can you include a screenshot of the pages on your development site after the translations have been applied? That would show us that you successfully tested it and that the work is correct.
translation/source/faq.xml
Outdated
| <string name="goToLichess">Go to lichess.org</string> | ||
| <string name="browsersMozillaFirefox">Press Ctrl-i on linux/windows or cmd-i on macos</string> | ||
| <string name="browsersMozillaFirefox1">Click the Permissions tab</string> | ||
| <string name="browsersMozillaFirefox2">Allow Audio and Video on lichess.org</string> | ||
| <string name="browsersGoogleChrome">Click the lock icon in the address bar</string> | ||
| <string name="browsersGoogleChrome1">Click Site Settings</string> | ||
| <string name="browsersGoogleChrome2">Allow Sound</string> | ||
| <string name="browsersSafari">Click Safari in the menu bar</string> | ||
| <string name="browsersSafari1">Click Settings for This Website</string> | ||
| <string name="browsersSafari2">Allow Auto-Play</string> | ||
| <string name="browsersMicrosoftEdge">Click the three dots in the top right corner</string> | ||
| <string name="browsersMicrosoftEdge1">Click Settings</string> | ||
| <string name="browsersMicrosoftEdge2">Click Cookies and Site Permissions</string> | ||
| <string name="browsersMicrosoftEdge3">Scroll down and click Media autoplay</string> | ||
| <string name="browsersMicrosoftEdge4">Add lichess.org to Allow</string> |
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.
ornicar mentioned in the other PR that these list items can be consolidated.
| <string name="goToLichess">Go to lichess.org</string> | |
| <string name="browsersMozillaFirefox">Press Ctrl-i on linux/windows or cmd-i on macos</string> | |
| <string name="browsersMozillaFirefox1">Click the Permissions tab</string> | |
| <string name="browsersMozillaFirefox2">Allow Audio and Video on lichess.org</string> | |
| <string name="browsersGoogleChrome">Click the lock icon in the address bar</string> | |
| <string name="browsersGoogleChrome1">Click Site Settings</string> | |
| <string name="browsersGoogleChrome2">Allow Sound</string> | |
| <string name="browsersSafari">Click Safari in the menu bar</string> | |
| <string name="browsersSafari1">Click Settings for This Website</string> | |
| <string name="browsersSafari2">Allow Auto-Play</string> | |
| <string name="browsersMicrosoftEdge">Click the three dots in the top right corner</string> | |
| <string name="browsersMicrosoftEdge1">Click Settings</string> | |
| <string name="browsersMicrosoftEdge2">Click Cookies and Site Permissions</string> | |
| <string name="browsersMicrosoftEdge3">Scroll down and click Media autoplay</string> | |
| <string name="browsersMicrosoftEdge4">Add lichess.org to Allow</string> | |
| <string name="enableAutoplayForSoundsFirefox">1. Go to lichess.org | |
| 2. Press ctrl-i on linux/windows or cmd-i on macos | |
| 3. Click the Permissions tab | |
| 4. Allow Audio and Video on lichess.org</string> | |
| <string name="enableAutoplayForSoundsChrome">1. Go to lichess.org | |
| 2. Click the lock icon in the address bar | |
| 3. Click Site Settings | |
| 4. Allow Sound</string> | |
| <string name="enableAutoplayForSoundsSafari">1. Go to lichess.org | |
| 2. Click Safari in the menu bar | |
| 3. Click Settings for This Website | |
| 4. Allow Auto-Play</string> | |
| <string name="enableAutoplayForSoundsMicrosoftEdge">1. Click the three dots in the top right corner | |
| 2. Click Settings | |
| 3. Click Cookies and Site Permissions | |
| 4. Scroll down and click Media autoplay | |
| 5. Add lichess.org to Allow</string> |
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.
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.
@M-DinhHoangViet Can you apply these changes? And then use these keys in faq.scala like this:
h3("Mozilla Firefox (", desktop(), "):"),
p(enableAutoplayForSoundsFirefox()),
h3("Google Chrome (", desktop(), "):"),
p(enableAutoplayForSoundsChrome()),
h3("Safari (", desktop(), "):"),
p(enableAutoplayForSoundsSafari()),
h3("Microsoft Edge (", desktop(), "):"),
p(enableAutoplayForSoundsMicrosoftEdge()),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 did that and the same thing
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.
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.
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.
Ok, glad it worked. Can you commit those changes to this branch? They don't show in the pull request.
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.
Oh no @fitztrev
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.
@M-DinhHoangViet That's what we want. You can commit that. Then once we apply @schlawg's suggestion about adding nl2br, it will appear formatted correctly.
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'm testing now!
|
I'm not sure we want to combine the first three paragraphs just to save a few translation keys. At least not without edits to make the sentence flow less abrupt. As a native English speaker, I could make those edits. But I'd much prefer to use the original text in three separate paragraphs. |
If so, if you want to combine them in one go, they cannot go down the line. See for example: https://lichess.org/faq#timeout, https://github.com/lichess-org/lila/blob/master/app/views/site/faq.scala#L155, https://github.com/lichess-org/lila/blob/master/translation/source/faq.xml#L51 |
|
Question for M-DinhHoangViet - is your given name Hoang or Viet? What's the best way to address you that's shorter than 15 characters? |
Many people mistake my name as Dinh, and some mistake my name as Hoang. But actually my name is Viet |
|
Hello Viet, I was referring to the first three paragraphs: I understand that there is no obvious way to place paragraph breaks within a single translation. But for this text, when the paragraphs are combined at the view layer, the resulting text becomes harder to read and understand. So I'm wondering if we should treat each paragraph as its own translation so they can be rendered individually. |
|
Apparently there is a function that can help us here. nl2br will parse the translation text and replace newlines with |
|
|
Try this line instead: p(nl2br(enableAutoplayForSoundsA.txt())), |
|
@M-DinhHoangViet To fix the build now you can run these 2 commands in gitpod in the terminal. That will fix the tab and space combination issue. Then you can commit the change. |
|
done! |
* master: rename CC button - closes lichess-org#13945 don't expand images in streamer mod notes fix new lines in translated HTML New Crowdin updates (lichess-org#13939) remove non-working disable button from voice gear menu remove unnecessary junk same arch variable names as lila-ws/pekko arm64 native reactivemongo
`key()` instead of `nl2br(key.txt())`
|
Well done everyone, that's a wrap |
Fixes #13888