-
-
Notifications
You must be signed in to change notification settings - Fork 532
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
Define the order of links in sidebar #359
Conversation
🦋 Changeset detectedLatest commit: 8c859a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thank you for this PR @IDurward and for your patience waiting for a review!
I think this looks good in principle! I’ve left a suggestion regarding the exact frontmatter syntax.
We also just added unit testing to the repo (guide/notes here) so now have the opportunity to add some tests to ensure this is working as expected. Would you be able to add some tests for this? Happy to provide guidance if there’s anything unclear there.
…light into IDurward-Add_order
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.
Thanks again for this @IDurward! I took a quick pass at writing a test and gave sidebar
a default value of {}
so that pages aren’t forced to set it if they don’t need it.
This still needs documenting in the frontmatter reference if you’d like to pick that up. Otherwise, I’ll get it to it tomorrow 🙌
During auto-generation of sidebar groups, routes are turned into an object tree, which means we can lose ordering for some keys in particular a numbered key like `123`. This refactor moves that logic to the sidebar logic where it belongs and also paves the way to more easily using other entry data in sidebar generation.
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.
Thanks again for kicking this off @IDurward! And apologies for commandeering your PR — I realised there were some edge cases with the approach sorting in the routing code, so I’ve moved this to our sidebar code. Luckily, these changes will also make it easier in the future to add more features to this sidebar frontmatter config.
That's ok @delucis, I don't mind you commandeering as my time is a bit limited at the moment. |
What kind of changes does this PR include?
Description
This allows the user to define the order that document links are displayed in, in the sidebar, when autogenerate is used. If no order is defined then the default alphabetical order is used.