Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: autogenerated sidebar alphabetical sort #1298

Merged
merged 13 commits into from
Jan 9, 2024
Merged

fix: autogenerated sidebar alphabetical sort #1298

merged 13 commits into from
Jan 9, 2024

Conversation

kevinzunigacuellar
Copy link
Sponsor Member

@kevinzunigacuellar kevinzunigacuellar commented Dec 19, 2023 •
edited
Loading

Description

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Dec 19, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 5d3aa3b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight Patch

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

@vercel Vercel
Copy link

vercel bot commented Dec 19, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Jan 9, 2024 9:32pm
1 Ignored Deployment
Name Status Preview Updated (UTC)
starlight-i18n ⬜️ Ignored (Inspect) Visit Preview Jan 9, 2024 9:32pm

@github-actions github-actions bot added the 🌟 core Changes to Starlight’s main package label Dec 19, 2023
@astrobot-houston
Copy link
Collaborator

astrobot-houston commented Dec 19, 2023 •
edited
Loading

size-limit report 📦

Path Size
/index.html 5.21 KB (0%)
/_astro/*.js 21.42 KB (0%)
/_astro/*.css 13.05 KB (0%)

@HiDeoo
Copy link
Member

HiDeoo commented Dec 19, 2023

Left some comment in #1151 (comment) as I think this may break the expected behavior based on the current documentation.

@kevinzunigacuellar
Copy link
Sponsor Member Author

Thanks for the review @HiDeoo 💜 . In that case, we can use only the filename name, without its slug path to do the sorting. I will update this PR accordingly.

delucis
delucis reviewed Dec 19, 2023
View reviewed changes
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kevinzunigacuellar! Love you digging into these tricky corners 💜 Left some thoughts.

packages/starlight/__tests__/basics/navigation-labels.test.ts Outdated Show resolved Hide resolved
packages/starlight/utils/navigation.ts Outdated Show resolved Hide resolved
Fryuni
Fryuni approved these changes Jan 5, 2024
View reviewed changes
Copy link
Member

@Fryuni Fryuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current behavior will sort the sidebar page differently depending on the locale of the page it is being shown. With this PR it will have a consistent ordering. But what is the ordering is not properly documented.

I noticed this recently when translating the docs that it is mentioned there that the sidebar is sorted alphabetically, but the implementation uses collation, which is not the same thing. Not all languages sort alphabetically and even the definition of "alphbetically" may change between them.

More information and examples of how things can differ here.

I don't think this should be changed in this same PR, but just leaving it here as a note that I think this should probably be mentioned in the docs. Both on the defaultLocale and on the sidebar guide.

@delucis
Update changeset
5d3aa3b 
Kevin told me I could
delucis
delucis approved these changes Jan 9, 2024
View reviewed changes
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s patch and GO! ✨

@delucis delucis added the 🌟 patch Change that triggers a patch release label Jan 9, 2024
@delucis delucis merged commit c7e995c into main Jan 9, 2024
9 checks passed
@delucis delucis deleted the fix-sidebar-sort branch January 9, 2024 21:48
@astrobot-houston astrobot-houston mentioned this pull request Jan 9, 2024
Merged
@delucis delucis mentioned this pull request Jan 10, 2024
Merged
HiDeoo added a commit to HiDeoo/starlight that referenced this pull request Jan 15, 2024
@HiDeoo
Merge branch 'main' into hd-docs-pnpm-install-add
631a404 
* main: (55 commits)
  [ci] format
  i18n(es): Update `index` (withastro#1360)
  [ci] format
  i18n(fr): Update index (withastro#1367)
  i18n(es): remove extra section (withastro#1370)
  i18n(ko-KR): update `index.mdx` (withastro#1363)
  [ci] format
  i18n(zh-cn): Update index.mdx (withastro#1361)
  docs(showcase): add OpenSaaS.sh (withastro#1359)
  feat(Testimonials): add testimonials to website (withastro#1104)
  [ci] format
  [ci] release (withastro#1332)
  fix: autogenerated sidebar alphabetical sort (withastro#1298)
  Avoid sidebar scrollbar hiding behind navbar (withastro#1353)
  Use spawnSync instead of execaSync in `git.ts` (withastro#1347)
  [ci] format
  [i18nIgnore] Add src alias (withastro#1322)
  Italian translation for search.devWarning (withastro#1351)
  [ci] format
  i18n(pt-BR): Add translation for `guides/sidebar` (withastro#1346)
  ...
HiDeoo added a commit to HiDeoo/starlight that referenced this pull request Jan 15, 2024
@HiDeoo
Merge branch 'main' into hd-file-tree
93a654d 
* main: (69 commits)
  [i18nIgnore] docs: `pnpm install` → `pnpm add` (withastro#1324)
  [ci] format
  i18n(zh-cn): Update frontmatter.mdx (withastro#1362)
  [ci] format
  i18n(es): Update `index` (withastro#1360)
  [ci] format
  i18n(fr): Update index (withastro#1367)
  i18n(es): remove extra section (withastro#1370)
  i18n(ko-KR): update `index.mdx` (withastro#1363)
  [ci] format
  i18n(zh-cn): Update index.mdx (withastro#1361)
  docs(showcase): add OpenSaaS.sh (withastro#1359)
  feat(Testimonials): add testimonials to website (withastro#1104)
  [ci] format
  [ci] release (withastro#1332)
  fix: autogenerated sidebar alphabetical sort (withastro#1298)
  Avoid sidebar scrollbar hiding behind navbar (withastro#1353)
  Use spawnSync instead of execaSync in `git.ts` (withastro#1347)
  [ci] format
  [i18nIgnore] Add src alias (withastro#1322)
  ...
HiDeoo added a commit to HiDeoo/starlight that referenced this pull request Jan 15, 2024
@HiDeoo
Merge branch 'main' into hd-virtual-pages
823b259 
* main: (62 commits)
  [i18nIgnore] docs: `pnpm install` → `pnpm add` (withastro#1324)
  [ci] format
  i18n(zh-cn): Update frontmatter.mdx (withastro#1362)
  [ci] format
  i18n(es): Update `index` (withastro#1360)
  [ci] format
  i18n(fr): Update index (withastro#1367)
  i18n(es): remove extra section (withastro#1370)
  i18n(ko-KR): update `index.mdx` (withastro#1363)
  [ci] format
  i18n(zh-cn): Update index.mdx (withastro#1361)
  docs(showcase): add OpenSaaS.sh (withastro#1359)
  feat(Testimonials): add testimonials to website (withastro#1104)
  [ci] format
  [ci] release (withastro#1332)
  fix: autogenerated sidebar alphabetical sort (withastro#1298)
  Avoid sidebar scrollbar hiding behind navbar (withastro#1353)
  Use spawnSync instead of execaSync in `git.ts` (withastro#1347)
  [ci] format
  [i18nIgnore] Add src alias (withastro#1322)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@delucis delucis delucis approved these changes

@Fryuni Fryuni Fryuni approved these changes

Assignees
No one assigned
Labels
🌟 core Changes to Starlight’s main package 🌟 patch Change that triggers a patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

alphabetical sorting not functioning properly in autogenerated sidebars
5 participants
@kevinzunigacuellar @astrobot-houston @HiDeoo @delucis @Fryuni