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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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

[godismyjudge95]

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.

---

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]

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]

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]

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]

@godismyjudge95 I think this might help now: #12258

[godismyjudge95]

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

[jasonvarga]

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