fix(i18n): add missing translation keys#2140
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces numerous hard-coded UI strings with Vue i18n translation bindings across multiple components and pages. Changes include accessibility labels, button text, badges, error/loading messages and sr-only text in Dependencies, ListToolbar, SkillsModal, VersionDistribution, VersionSelector, diff ViewerPanel, blog list page and the compare versions page. Adds corresponding English translation entries and updates i18n schema to include the new keys. No control flow, data fetching, or public API signatures were modified. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
i18n/locales/en.json (1)
1193-1193: Consider parameterising the expected URL format.Ending the message with a colon suggests the format example is rendered outside the translation. A
{format}placeholder would give translators full control over sentence order.♻️ Possible tidy-up
- "version_invalid_url": "Invalid comparison URL. Use format:", + "version_invalid_url": "Invalid comparison URL. Use format: {format}",{{ $t('compare.version_invalid_url', { format: expectedFormat }) }}
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 171a138a-3021-441e-8e7b-a81c096e8960
📒 Files selected for processing (10)
app/components/Package/Dependencies.vueapp/components/Package/ListToolbar.vueapp/components/Package/SkillsModal.vueapp/components/Package/VersionDistribution.vueapp/components/VersionSelector.vueapp/components/diff/ViewerPanel.vueapp/pages/blog/index.vueapp/pages/diff/[[org]]/[packageName]/v/[versionRange].vuei18n/locales/en.jsoni18n/schema.json
| {{ $t('compare.version_invalid_url') }} | ||
| <code class="font-mono text-sm">/diff/package/v/from...to</code> |
There was a problem hiding this comment.
I wasn't sure what the best way to handle this would be, and if we should attempt to add translations for package/from/to. Seems like if we did, we'd want additional brackets or something to separate them out from the "hard-coded" parts of the url (e.g. /diff/[packageName]/v/[from]...[to]). Not totally sure what a i18n key for that would look like.
There was a problem hiding this comment.
I do think we need to translate the "from" and "to" strings.
something like $t('compare.version_diff_format', { packageName }) and the string would be:
en.json:
{
"version_invalid_format": {
"hint": "Invalid comparison URL. Use format: {0}",
"format": "/diff/{packageName}/v/from...to"
},
}then you use the i18n-t component with something like this
<i18n-t keypath="version_invalid_format.hint" tag="p" for="format">
<code>{{ $t("version_invalid_format.format") }}</code>
</i18n-t>I hope this is what you were looking for
There was a problem hiding this comment.
Thanks for that! I added the i18n component, and split the format details into additional keys for from and to, while leaving the rest hard-coded (with the packageName interpolated). Hopefully that reduces the potential for introducing errors to the full string in the translations.
There was a problem hiding this comment.
Thanks! I do find the HTML a bit harder to read, but I think it would probably reduce the number of typos when translating the example.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
MatteoGabriele
left a comment
There was a problem hiding this comment.
Thanks for being so responsive and getting this done! 🙏
|
Thanks for your first contribution, @nadaniels! 💪 We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role. |
🔗 Linked issue
Resolves #2128
Related PR: #2134
📚 Description
This PR addresses the linked issue by adding missing translation keys and removing hardcoded text, including 22 new keys across 7 files.
Additional Notes:
descriptionis enforced by the schema.skills-npmin the SkillsModal as mentioned in the linked issue, since it's an official package name. A key forskills CLIwas added, following the existing pattern fornpm CLI.I'm not sure how best to proceed with the other PR that was opened while this was in progress. Happy to combine efforts if that's the best way to move forward, given additional guidance. Thanks!