-
Notifications
You must be signed in to change notification settings - Fork 919
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
Update copy on developer edition page (fix #13970) #14021
Update copy on developer edition page (fix #13970) #14021
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.
Nice work!
I'm going to approve this PR, but avoid merging it until January 3rd since we're currently on a code freeze for the holidays. 🎄
@reemhamz seeing as this is a new string, shouldn't it use a new fluent string ID? |
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.
Sorry about this, but my holiday brain took over, and I didn't review this critically. I left you tips and suggestions on how we'd usually update strings that are placed in .ftl
files. It's a long-looking explanation, but it's super easy and simple to get done. Let me know if you need any more help :) Hapy new year!
l10n/en/firefox/developer.ftl
Outdated
firefox-developer-all-the-latest = All the latest developer tools in beta, plus <strong>experimental features</strong> like the Multi-line Console Editor and WebSocket Inspector. | ||
firefox-developer-all-the-latest = All the latest developer tools in beta, in addition to features like the Multi-line Console Editor and WebSocket Inspector. |
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 my bad, I had a fried holiday brain and missed how we usually do string changes!
So, when we want to update Fluent strings on our site (strings found in .ftl
files that are sent to our translators), we want to make sure to create a new string instead of update a current one.
We do this because translators take time to translate new strings, and we usually use the now-older strings as fallbacks, and place a comment above the now-older string that says it's an obsolete string
You can read more about how that's done here: https://bedrock.readthedocs.io/en/latest/l10n.html#obsolete-strings
So, in this case, we'd fix these changes to instead be:
firefox-developer-all-the-latest = All the latest developer tools in beta, plus <strong>experimental features</strong> like the Multi-line Console Editor and WebSocket Inspector. | |
firefox-developer-all-the-latest = All the latest developer tools in beta, in addition to features like the Multi-line Console Editor and WebSocket Inspector. | |
# Obsolete string | |
firefox-developer-all-the-latest = All the latest developer tools in beta, plus <strong>experimental features</strong> like the Multi-line Console Editor and WebSocket Inspector. | |
firefox-developer-all-the-latest-v2 = All the latest developer tools in beta, in addition to features like the Multi-line Console Editor and WebSocket Inspector. |
So, we essentially placed a string above the older string ID mentioning it is now obsolete, and we gave the new string a similar string ID that now ends in -v2
. This is how we typically pattern new string updates if the new strings only have slight changes.
And now in the relative HTML file where this string is mentioned, we update the ftl
helper function to the new string ID, and we use the older string ID as a fallback. You can see a good example here: https://bedrock.readthedocs.io/en/latest/l10n.html#the-ftl-helper-function
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.
@reemhamz, thank you for your patience and explanation!
I was unsure if my approach was correct regarding localization updates and should have asked before pushing for it.
Also, I understand that we don't need to update the ftl_has_messages
helper function's conditions, as we choose to keep the translation of the obsolete string until a translation for v2 is provided, correct?
Happy new year!
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.
Of course, happy to help and I'm glad the feedback was useful.
we don't need to update the ftl_has_messages helper function's conditions
Sorry, do you mean the fallback
helper? Since we're not using the ftl_has_messages
helper here.
keep the translation of the obsolete string until a translation for v2 is provided, correct?
Yes, correct. Typically, most translations of a string can be completed within a 2-3 month period (usually earlier), so we make sure to keep the obsolete string in the HTML as a fallback just in case.
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.
Sorry, do you mean the fallback helper? Since we're not using the ftl_has_messages helper here.
Oh, what I meant is that there are some files that generate content conditioned on the existence of a firefox-developer-all-the-latest
. For example in "firefox/developer/index.html":
{% if ftl_has_messages('firefox-developer-made-for-developers', 'firefox-developer-all-the-latest') %}
{% include '/firefox/developer/includes/for-developers.html' %}
{% else %}
{% include '/firefox/developer/includes/performance.html' %}
{% endif %}
But from what I get, it is preferable that in other languages we continue to have the obsolete version translated than to stop generating the content (which would happen if we changed the first line of code above to {% if ftl_has_messages('firefox-developer-made-for-developers', 'firefox-developer-all-the-latest-v2') %}
)
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.
Thanks for explaining! Sorry, my mind has been slow this week and I didn't realise we have this string ID being used in multiple pages 😅 Good job digging deep into this 🧠
I've looked into what this requires, and it seems to be out-of-scope for this ticket since you're a contributor and the work would be getting too complex. Essentially, it looks like that ftl_has_messages
helper was placed when we created the for-developers.html
file with the new strings years ago, and we're going to need to do some updates and deletions to make sure things flow correctly.
I'm going to go ahead and approve your ticket since you successfully managed to get this ticket done 💯 Thanks so much for your contribution 🥇
One-line summary
Update copy on developer edition page to match the current status of the features mentioned
Issue / Bugzilla link
#13970