-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
fix(content-docs): translate generated index pagination title #8123
base: main
Are you sure you want to change the base?
Conversation
@slorber I'm not sure if this qualifies as a breaking change since we removed some metadata returned from |
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: 0 B Total Size: 819 kB ℹ️ View Unchanged
|
So what I understand is that we should add the navigation metadata to the docs only after having translated the sidebars (ie the category index titles). What annoys be me a bit is that I wish the Maybe we need an extra Note: is this expected that the sidebar became untranslated? I'm assuming your local test didn't include downloading all the translated MD and you just translated the category index title locally?
To me removing data from a lifecycle is a breaking change. But I wouldn't consider it a breaking change only if we added new data. At the same time we don't officially provide an official public API to override I think we can mark it as a breaking change. It is a minor i18n bugfix that can wait 3.0 IMHO and it's not really worth it to backport it immediately. |
I guess you didn't update the snapshots for now to help the review, can you make the PR pass? Also, we had the navigation snapshotted before, is there a good way to still prevent possible navigation regressions with our current test setup? |
There are other tests failing because this has changed the loaded content. I will fix that later. And yes—I only hand-wrote the translation that needed to be written.
This is the reason I haven't fixed the tests yet. I think we can only snapshot the generated data instead. |
2630d28
to
0835d5f
Compare
OK! I think I've finally got the tests figured out. |
deda01a
to
543bcb8
Compare
543bcb8
to
80fe374
Compare
Maybe not yet😅 Such a pain |
This comment was marked as off-topic.
This comment was marked as off-topic.
any progress about this 👀 |
+1 |
Would much appreciate if we can take another look at this issue. This is esp. bad if EN is not the default locale. |
We're encountering the same issue, and as @yujingz it feels really weird for users as the primary language of website is not EN. |
Hi, we are experiencing the same issue with the generated index not being fully translated. Any plan to correct this shortly ? Is it possible to help in a way or another ? |
Any updates on this? |
Pre-flight checklist
Motivation
This is the same kind of fix as #7634 — deferring the generation of some metadata until
contentLoaded
.Test Plan
We don't have tests for i18n + metadata generation, but I did test locally:
Previous:
Current:
Test links
Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/
Related issues/PRs