Skip to content

[6.x] Support expanding/collapsing CP subnavs in the sidebar#12169

Closed
godismyjudge95 wants to merge 6 commits intostatamic:masterfrom
godismyjudge95:collapsible-cp-nav-sections
Closed

[6.x] Support expanding/collapsing CP subnavs in the sidebar#12169
godismyjudge95 wants to merge 6 commits intostatamic:masterfrom
godismyjudge95:collapsible-cp-nav-sections

Conversation

@godismyjudge95
Copy link
Copy Markdown
Contributor

This PR adds the ability to expand and collapse CP subnavs in the sidebar without navigating to the parent page. It also adds the ability to expand the subnav on hover by holding down shift while hovering - requiring no clicks.

CPT2508251551-473x448


Note, the tests modified are due to the removal of the line from NavBuilder to resolve all the children of the nav items on each load. This effectively reverts the functionality of lazy loading CP nav children added in this commit - 5574b28

I don't believe this will cause a huge performance hit in the backend because CP navs should be small enough for it to not matter but I could be wrong?

@godismyjudge95
Copy link
Copy Markdown
Contributor Author

Added missing Aria attributes but one of them was a the aria-controls attribute which requires an id on the subnav. Since there is no guarantee the $item->id() is unique (it's based off of section title and the nav item text), I used Str::random(4) for a unique id. There might be a better method of doing this I am not aware of.

@jackmcdade
Copy link
Copy Markdown
Member

I'm pretty on the fence about this, just want to be up front. That nav lazy loading is important for performance because of the potential for a lot of permission checks, especially with addons installed. It's not just a throwaway thing. Also, it still ends up being the same number of clicks to get to the subpages, you're just saving the page refresh. 🤔

@godismyjudge95
Copy link
Copy Markdown
Contributor Author

I'm pretty on the fence about this, just want to be up front. That nav lazy loading is important for performance because of the potential for a lot of permission checks, especially with addons installed. It's not just a throwaway thing. Also, it still ends up being the same number of clicks to get to the subpages, you're just saving the page refresh. 🤔

Can the permission checks themselves be cached better? I think there should be a way for the nav items/permissions to be initialized once per user and then you'd just have to check/change what the current page is. I'd be happy to look into that.

Also, the saving a page refresh is actually a real UX improvement - especially for clients that are still finding their way around the CP or have a slow internet connection.

@andjsch
Copy link
Copy Markdown
Contributor

andjsch commented Sep 4, 2025

@godismyjudge95 I think this might help now: #12258

@godismyjudge95
Copy link
Copy Markdown
Contributor Author

If Inertia gets merged - #12610
Performance shouldn't be an issue on this right? 😄

@jasonvarga
Copy link
Copy Markdown
Member

Thanks for the PR. Since Inertia changes the entire nav, this PR would need to be rewritten anyway. We can revisit after Inertia.

@jasonvarga jasonvarga closed this Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants