-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Site Editor: Add chevrons to off canvas list view posts & pages #50149
Conversation
Size Change: +896 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
…se are the only items that will have a submenu in the navigation site editor menu.
I've marked this one as ready for review. My main concern here is isolating which blocks in the list that actually have submenus. It's hardcoded here because it seems there are only a small handful. Open to suggestions on how this might be automatically calculated instead. I couldn't think of any good ways :) |
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.
I like the idea of using the chevrons to indicate that there's an extra level of drilldown 👍
I'm wondering about a couple of potential challenges with this approach — it looks like it also adds chevrons in the off canvas editor that's current used in the inspector sidebar:
I'm not too sure which direction would work best, but I had a few thoughts / questions:
- The Navigation block's sidebar controls are slated to swap out the off canvas editor for using the "real"
ListView
component: Navigation: Use the ListView in the Navigation block inspector controls #49417 - Once that's consolidated, is the plan to remove the off canvas editor component altogether and use
ListView
everywhere, or is it still intended to be maintained? If it's to be removed, we might want to look at enhancements to theListView
in the longer term, to prevent the off canvas editor from drifting too far from the original component. CC: @scruffian and @getdave in case they have more context there. - To generalise the feature, would it be worth declaring whether to display chevrons as part of the component's API? That way we would only display them in contexts where they should be shown.
Beyond that, something else I was curious about is that in some of the designs that @jasmussen has shared (e.g. on #49597 (comment)) the navigation in this panel is referred to as Site Structure. In the longer term, is the plan for a user to be able to drill down into their posts and pages separately from editing the Navigation block? If so, then I was wondering if the drilldown to posts and pages should happen within the list view, or if there would be another way of managing it. Currently the list view is primarily concerned with blocks, rather than posts or pages.
const hasChevron = [ 'core/navigation-submenu', 'post', 'page' ].includes( | ||
blockInformation.name | ||
); |
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.
This bakes in knowledge about particular blocks into the component itself. Will that affect reusability of the component?
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.
Agreed that it should be passed to the component. 👍
Apologies, I missed this comment before adding my review! Since the |
Thans for the PR. On the one hand I understand the desire, on the other hand it both gets a bit noisy, and it further uses valuable space that's needed for ellipsis menus and otherwise. Potentially the chevron could appear on hovering the title and replacing buttons on the right, but it seems like the main inspector list view could use some improvements before that, to just make sure space is well used and buttons align right. I'd hold off on this one until it's in a more solid place.
I want to note that this is still being explored and @jameskoster and @SaxonF have a larger prototype for this behavior to share soon. We might even find that the section you refer to, whatever it ends up getting called could be just about the main published pages and even content templates (404, search) on your site, to give content editing access to every facing piece. There might then instead be a separate Menus item, which lists every navigation block used on the site, with a link to edit that just as if it was a reusable block. I'll defer to Jay and Saxon on the details here, but that's something to consider.
This off-canvas list view interface I expect to be a core interface that gets refined and leveraged not just for the navigation block, but for contentOnly locked patterns, and even the wp:pattern block (#48458), so it's definitely an interface we want to refine, polish, and get right. It will be used in lots of places, and the key problem it solves is when a block has complex nesting structures, it can provide an ergonomic inspector interface that allows you to easily pick the right block and customize it, rather than have to pick it in the canvas where it might be layered under another block or otherwise being hard to select. The contentOnly locking system already has an older variant of this interface that is not as expanded, and #49980 by @noisysocks implements something like it. But ultimately I see all of those converging, and preferrably using the Navigator component that was built for global styles, so that the inspector slides and animates when you drill into blocks. |
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.
IMO we should remove this component ASAP. I think we can just replace it with List View and then make the corresponding changes in List View if necessary. If we make these changes here then when we swap out this component for the List View we'll lose them. We are working on that here: #49417
Thanks for the context @scruffian I wasn't aware of that work happening. Makes sense to pause this work until you have that swapped out. |
@scruffian Is the list view component more stable? I'm noticing with the number of items I have in my navigation (all of them), it regularly crashes Safari when looking at it in this site editor view. |
They are essentially the same component but OffCanvasEditor has a few extra props. I made a PR here to replace it but I hit some snags: #50287 |
This is now unblocked as we use the list view here. I think we can just close the issue :) |
@scruffian Which issue do you mean? I checked trunk and I'm not seeing chevrons still, so I'm assuming I'm good to continue here now? 👍 |
The plan for the view is to move that functionality into the ellipsis menu (#50698), which is why I think the issue can be closed - it's not something we want to do anymore! |
Follow up to #50076
What?
Update the navigation section of the site editor so that pages and posts in this list will show the chevron icon to indicate there is a deeper sub-level.
Why?
To match the changes in #50076
How?
🤖 Generated by Copilot at 5e0994a
chevronRightSmall
icon as a drilldown indicator for some block types in the list view (link,link,link)name
property to the object returned by theuseBlockDisplayInformation
hook (link,link)Testing Instructions
Screenshots or screencast