-
-
Notifications
You must be signed in to change notification settings - Fork 9k
feat(content-blog): allow sorting posts in ascending order #5787
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
Conversation
e6baf77
to
f5595d5
Compare
✔️ [V2] 🔨 Explore the source changes: e6baf77 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6177269237c261000741f929 😎 Browse the preview: https://deploy-preview-5787--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5787--docusaurus-2.netlify.app/ |
✔️ [V2] 🔨 Explore the source changes: f5595d5 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/617727b6c80df900076f3595 😎 Browse the preview: https://deploy-preview-5787--docusaurus-2.netlify.app |
✔️ [V2] 🔨 Explore the source changes: f3f14f9 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/618a437801d89a000797e8c7 😎 Browse the preview: https://deploy-preview-5787--docusaurus-2.netlify.app |
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.
Sure, sounds good I'll resubmit PR at the end or just after weekend |
No need to close the current one, just change the current one |
|
d5104eb
to
f28279e
Compare
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 think it mostly looks good in terms of implementation. In terms of whether we want this API or not... not sure🤔
Several API design suggestions:
postSortDirection
=>sortPosts
(we can have other options in the future)'desc'
=>'descending'
(it's better to be explicit IMO)
b52ff03
to
dfc615c
Compare
@@ -59,6 +59,7 @@ Accepted fields: | |||
| `feedOptions.description` | `string` | <code>\`${siteConfig.title} Blog\`</code> | Description of the feed. | | |||
| `feedOptions.copyright` | `string` | `undefined` | Copyright message. | | |||
| `feedOptions.language` | `string` (See [documentation](http://www.w3.org/TR/REC-html40/struct/dirlang.html#langcodes) for possible values) | `undefined` | Language metadata of the feed. | | |||
| `postSorting` | <code>'descending' \| 'ascending' </code> | `'descending'` | Governs the direction of blog post sorting. | |
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.
| `postSorting` | <code>'descending' \| 'ascending' </code> | `'descending'` | Governs the direction of blog post sorting. | | |
| `sortPosts` | <code>'descending' \| 'ascending' </code> | `'descending'` | Governs the direction of blog post sorting. | |
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. Looking for @slorber
LGTM
|
Just curious, are you using this feature @cerkiewny? Do you expect the RSS feed to be in ascending order too? 🤔 Asking for #9827 (comment) |
Motivation
Fix #5692
Have you read the Contributing Guidelines on pull requests?
Yes.
Test Plan
Regression tests for older relevant tests.
Created new test that sorts posts in reverse creation order and compare it to default option.
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)