feat: Export `flattenSidebar` function
Description
Export flattenSidebar function, used by <SidebarSublist> for easier custom overrides.
⚠️ No Changeset found
Latest commit: 9d82c5689e53d1eab5f127dfcd4f34456ee0d2ac
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Deploy Preview for astro-starlight ready!
| Name | Link |
|---|---|
| Latest commit | 9d82c5689e53d1eab5f127dfcd4f34456ee0d2ac |
| Latest deploy log | https://app.netlify.com/projects/astro-starlight/deploys/6917502ec544ee00087dd0da |
| Deploy Preview | https://deploy-preview-3552--astro-starlight.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
Lighthouse |
1 paths audited Performance: 100 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports |
To edit notification comments on pull requests, go to your Netlify project configuration.
Thanks for the contribution.
Export
flattenSidebarfunction, used by<SidebarSublist>for easier custom overrides.
Question about this PR and also #3550: <SidebarSublist> is not a component that can be overridden, and this function is definitely part of Starlight's internals as it's only meant to be used by that component right now. Would you be able to explain a bit more about your use case and the desired outcome?
Sure, let me explain my problem. My main goal was to add a link at the directory level of the sidebar. So tweak this line https://github.com/withastro/starlight/blob/c8bbae8977a2e4b7be64d90d19f7f8ac21dd627b/packages/starlight/components/SidebarSublist.astro#L42 but keep as much as possible the existing behaviours.
To achieve this, since SidebarSublist is used by Sidebar, I had to copy and paste several elements to modify this behavior.
Multiple friction points:
MobileMenuFooter, used inSidebaris internalSidebarRestorePoint, used inSidebarSublistis internalflattenSidebar, used inSidebarSublistis internal
If I’m overriding the sidebar, it’s very likely that I need to use some of the existing logic part of SidebarSubList.
Another point not addressed yet, I would have loved to copy the style from @astrojs/starlight/components/SidebarSublist.astro to avoid duplicating.
I hope this helps understanding the usecase
Taking a look at this, I think I’d prefer a different approach rather than exposing internals.
In general we’ve found it’s better for our components to contain as little business logic as possible and instead be pretty simple representations of the locals.starlightRoute data object we use for each page.
We haven’t really applied this principle as thoroughly to components like this because we consider them “internal” like @HiDeoo mentioned — the intention is for anyone who is reimplementing these to do so from scratch. But we probably should consider it, because like this PR shows, people reimplementing things are likely to have similar requirements.
So in this case I look at it and instead of thinking “flattenSidebar should be exported”, I think “there should be data equivalent to what flattenSidebar() achieves in the route data object”. That way neither our component, nor a user component needs to worry about computing it.
So in this case, I’d propose we add a hasCurrent or containsCurrent property to sidebar groups and then use those here if possible.
Going to close this and the related PRs pending the active discussion in https://github.com/withastro/starlight/discussions/3411! We can always reopen when the time comes — thanks @fabien0102.
