-
-
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(blog): authors count incorrectly rendered #10431
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: +917 B (+0.01%) Total Size: 11.6 MB
ℹ️ View Unchanged
|
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 but you need to run a yarn command to update the translations, cf the CI test failure logs and GitHub check annotations
packages/docusaurus-theme-classic/src/theme/Blog/Pages/BlogAuthorsPostsPage/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-common/src/translations/blogTranslations.tsx
Outdated
Show resolved
Hide resolved
@@ -45,6 +45,7 @@ | |||
"theme.blog.archive.description___DESCRIPTION": "The page & hero description of the blog archive page", | |||
"theme.blog.archive.title": "Archive", | |||
"theme.blog.archive.title___DESCRIPTION": "The page & hero title of the blog archive page", | |||
"theme.blog.author.noPosts": "This author has not written any posts yet.", |
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 suspect you didn't run the command the CI / tests tells you to run, this is not the expected result from running it.
What did you run?
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.
yarn jest
, what am I supposed to run ?
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.
The tests are now passing, but you didn't read the message that the previously failing tests printed:
Errors thrown in packages/docusaurus-theme-translations/locales/tests/locales.test.ts
Summary of all failing tests
FAIL packages/docusaurus-theme-translations/locales/tests/locales.test.ts
● theme translations › has base messages files contain EXACTLY all the translations extracted from the theme. Please run "yarn workspace @docusaurus/theme-translations update" to keep base messages files up-to-date
I can see this because your extracted message has no description, and it only modified the base locale, not all the other localized files.
You are not supposed to enter that message manually in theme translations: it's the CLI command that it supposed to do so
I added the missing translations here:
d7f0fe6
We add the message everywhere so that translators just have to see what is untranslated in their local file, without having to compare it to the English file
BTW I've been meaning to enable type-aware linting in this repo, so we could catch this type of errors, but first we need to get ts-eslint up to date. |
Motivation
Fix #10427
Test Plan
Preview
Test links
Deploy preview: https://deploy-preview-10431--docusaurus-2.netlify.app/tests/blog/authors