-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
7342 edit this translation #7389
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
apps/site/components/withMetaBar.tsx:62
- Ensure the translation key 'components.metabar.contributeText' is defined and covered by tests.
{t('components.metabar.contributeText')}
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.
LGTM, thanks
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.
SGTM
Lighthouse Results
|
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.
LGTM
I'm not 100% a fan. I think that if it's the default language, we should have ‘edit content’ and if it's another language we should have ‘edit source content’ and if translation is available for this page we should have ‘translate this page’. It might be possible to move forward with this pr to avoid problems. And open an exit to have this feature more ‘fancy’. |
Thanks for sharing your opinion. Re-reading #7342 (comment), you probably would have wanted something more nuanced. I went with this approach after realizing we have already been translating this string, just not using it. Changing it will take time. Would you be okay with this solution as an improvement on the existing state? I took it up during some downtime to improve reader/contributor UX. |
yeah let's merge with actual state and then open another change |
Description
Usesthe (already, but unused)
edit this page
translation in the metabar, and conditionally sends the user to the appropriate resourceValidation
Live
Rendering
en
(default)locale: the url resolves as usualRelated Issues
closes #7342
Check List
npm run format
to ensure the code follows the style guide.npm run test
to check if all tests are passing.npx turbo build
to check if the website builds without errors.