Add: Navigation menus to the browse mode sidebar. #46436
Conversation
|
Size Change: +551 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
b9ceafa to
8ffa751
Compare
packages/edit-site/src/components/sidebar-navigation-screen-main/index.js
Outdated
Show resolved
Hide resolved
|
Outstanding, thank you! This GIF shows the new navigation item in the sidebar left of the new browse mode interface:
A few things. Let's use singular for menus. But beyond renaming it to "Navigation menu", can we just call it "Navigation"? The other aspect is that ideally a click on a navigation item navigates to that page. So in the GIF above I have Blog, About, Gallery and Contact. While I should be able to rearrange items in the menu, I should also be able to click an item to go there in the browse view. We can think of it as the first iteration of actual browse mode. Probably doesn't have to happen in this PR, but worth noting. We can remove this item now:
Something is off about the positioning here:
There's a missing focus style, so the default chrome focus style is applied:
The alignment of icons here is a bit wrong: I wonder, to get the alignment right, can we use the same componentry as is used for templates?
Nice one. Let me know if you need some help here with the intricacies. |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
8ffa751 to
00c37d3
Compare
|
Hi, @jasmussen, and @mtias, the title was changed to navigation as suggested. Because the base component is not exposed, it requires the Off-canvas navigation editor feature to be enable at wp-admin/admin.php?page=gutenberg-experiments.
Clicking to go to the page will be a follow-up after this PR is merged. Is there any update we should do before merging this PR? |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
00c37d3 to
c0b21e9
Compare
|
A couple of observations: It would be good if the navigation panel items could be more dimensionally aligned with the Templates + Template Part panels. Hopefully this illustration explains what I mean:
The menu dropdown is a bit noisy, would it be possible to style it to match the 'dark' theme? It's a bit strange to have hover / focus styles on the items in the list when there is no on-click interaction. It feels a bit broken like this. I appreciate there's drag/drop but it's not obvious, perhaps a different cursor ( In the ellipsis menu, the inspector toggle ("Show more settings") does nothing. Should we hide it? Locking feels a bit strange in this context, as do the create template part / reusable block actions. I appreciate these items might be something that needs to be fixed in the off-canvas nav component rather than here. I don't know if they should be blockers for this PR. Most likely one for a follow-up, but should there be a way to create new menus here, similar to creating new templates? If so, I wonder if the add menu item should have a label? IE:
|
| @@ -1,4 +1,5 @@ | |||
| .edit-site-sidebar-navigation-item.components-item { | |||
| .edit-site-sidebar-navigation-item.components-item, | |||
| .edit-site-sidebar-navigation-screen__content .edit-site-navigation-inspector .components-button { | |||
There was a problem hiding this comment.
Why do we need a specific style here in a generic component?
There was a problem hiding this comment.
I removed the styles for .edit-site-sidebar-navigation-item.components-item, the other styles are required so the buttons looked "themed" on the dark mode.
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
2 times, most recently
from
44eba45 to
c02d0ff
Compare
|
Flaky tests detected in 7c4c3ba. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3915721386
|
|
Hi @jameskoster, I fixed the margin issue. I updated the cursor to be grabbed.
I'm not sure if we should offer the ability to create menus here. I even imagine that in the future, the menus available to browse may be restricted to the current menus used on the page/template (in most cases, just one menu is used, so the user only sees that menu). But my vision for the navigation section may be wrong. I think the options available on the ellipsis menu can be solved in a separate PR. Like the "Add menu item" is. These are changes on the off-canvas editor itself. |
|
Thanks @jorgefilipecosta. I'll dig into this more tomorrow, but one thing I noticed is that the link styles in this PR are now different to trunk: Trunkbefore.mp4This PRafter.mp4This view isn't very helpful:
If we don't intend to allow the creation of menus here, perhaps we should hide the panel altogether when no menus are found? Edit: I noticed the alignment and widths aren't right yet either. Ideally the icon should align roughly with the edge of the panel title, and reach row should be full width. Something like:
|
|
This PR is looking good in general. The main feedback for me when testing the PR is that now it's becoming more "urgent" to figure out the saving flows outside the frame: when you add a page to the menu, it's still unsaved by default. I'm happy to address separately if needed but we'd need some designs. The other thing is that when clicking pages/posts on the navigation tree, I expected the corresponding template with the right post/page in context to be loaded in the frame and ideally the element should be marked "selected" in the tree (like we do for the template list), thought that last point is arguable as we could potentially "only" select the corresponding template as a first step. |
|
Saving in browse view needs some holistic consideration imo. Specifically; do we want a global save button that opens the multi-entity save panel, or do we want contextual save buttons in the panels that require them (Navigation and Styles)? Alternatively, since creating a template automatically publishes it, we might make a case that any changes made in the browse view are auto-saved. I think a global save button (inside the site hub?) that opens the multi-entity save panel in a modal might work best, but I'd love to hear thoughts from others. Should we open a separate discussion for this?
I think it would be a little mysterious to see the frame update on click, but not display the content of the item you clicked on. So I'd lean towards tackling template+content together as a separate effort. |
I agree it needs to be discussed and solved separately, so yeah let's go with a separate discussion here. I also lean towards a global save button that morphs into the editor's save button when moving to edit mode. |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
c02d0ff to
aafe119
Compare
|
Coming back to this one, just wanted to comment on this:
I agree, both in the navigation block and here, and I wonder if instead of a dropdown ( |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
2 times, most recently
from
db6e4e9 to
7c4c3ba
Compare
|
Now that we have the ability to manage menus via the navigation block, do we really need a dedicated space for menu management? I'm not personally sure we do as we are leaning more in to direct manipulation of menus (e.g. click navigation block -> edit ) as opposed to managing menus out of context. What value do we get and is it worth it? You can also edit menus in isolation via template parts / patterns. If the answer is definitely yes, at the very least the list view experience designed and built for the inspector should be re-used here. |
One key value to hold is to consider the 80% use case. For example, 1 header, 1 menu, 1 footer. In those cases, being able to edit your menu right from the design section, I suspect, will bring with it a lot of ease of use. We still need for multiple menu management, and menu management inside the editor, to be vastly simplified. The improvements happening in that space are exciting and needed. But those still require you to click "Edit" in context of a random page, meaning the global context of a menu is slightly lost. So in that light, I see having access to both styles and menus right in the design section as a win. |
|
Hi @jameskoster, thank you for this review.
Fixed 👍
I applied a fix to try to align things. In most menus, we will have submenus, so we have an arrow to expand the submenu. That row is now aligned with the panel title. But the icon of the menu item is not, as shown in the image, because we need space for the arrow.
I applied the suggestion, and navigation is hidden if there are no menus. |
Hi @jasmussen, I guess we could use the CustomSelectControl component, so we have a "select," and then we could style it to match what we want? From the a11y point of view, we would take advantage of all the work done and the control would still look like a select. |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
7c4c3ba to
0f9989c
Compare
|
Test this one again, for me the important missing piece right now is the ability to click a page/post to view/edit it. |
Actually can we try just locking it, as a starting point, to a single menu? That is to say, the first menu saved. If you need to edit other menus, you have to go into the editor, find that menu, and edit there? This is meant to be the global menu, so it might be good to lock it to that. |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
0f9989c to
7c46588
Compare
Done. I removed the select from the navigation sidebar. We have a smart logic to detect what menu appears, which works well. |
|
Thanks @jorgefilipecosta. The styling is better but a few details remain: Specifically the rows are too narrow, and the ellipsis existing outside of the hover area looks a bit strange.
Minor point: the submenu icon looks a bit odd. I don't think we need that – can we just use the regular icon? It would be really great to align menu item styling across all panels:
Here's the Figma file in case it's helpful. |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
3 times, most recently
from
cabee41 to
bbf1678
Compare
|
Hi @jameskoster, Let me know if any other updates are necessary.
Regarding this point, we are using the icon of the submenu block, because the menu is just the new offsite navigation editor. Changing the icon here would change it everywhere including the list view, block inspector, and the block toolbar. Maybe we can think of a new icon that fits all these cases? |
|
Thanks for your perseverance, that looks much better :)
Fair enough, let's tackle that separately. |
| @@ -15,12 +17,33 @@ import SidebarNavigationScreen from '../sidebar-navigation-screen'; | |||
| import SidebarNavigationItem from '../sidebar-navigation-item'; | |||
|
|
|||
| export default function SidebarNavigationScreenMain() { | |||
| const { navigationMenus } = useSelect( ( select ) => { | |||
There was a problem hiding this comment.
I've noticed that when I open the site editor, I see two menu items (templates and template parts) and a few milliseconds later, I see the navigation one. I wonder if we should use useSuspenseSelect here and add a Suspense provider around the sidebar to show a loader or something before the navigation menus are loaded. (We've never used suspense before so this might not be ready yet but I think worth a try, could be a follow-up)
Yep, this is a pretty crucial piece! |
|
Oh something I noticed here, should we remove the right navigation sidebar when you're in "edit mode". I think it's redundant now and I think this was always a temporary place for it. |
Yes! |
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
bbf1678 to
11ea045
Compare
jorgefilipecosta
force-pushed
the
add/navigation-menus-browse-mode-sidebar
branch
from
11ea045 to
42dd3b5
Compare
juanmaguitar
added
the
[Feature] Site Editor
|
@jorgefilipecosta @jasmussen I was testing this PR with templates that have both header and footer navigation. The Navigation panel displays the menu in the header, which looks to be the expected result. I think this might lead to some confusion, at least it did for me 😅. Since Template and Template Parts allow you to edit all templates and parts, I assumed that I would be able to see and edit all navigation menus within the browse mode Navigation panel. I know that menus can also be managed within the Editor, but this abstracted view is fantastic and feels a lot like the classic menu management screens of old, which I know many people love—food for thought. I also ran into the issue noted here, that saving a modified menu still requires the user to go into Edit mode and then save. |
|
Yeah, we'll likely add the dropdown back at some point, but in a way that is less obtrusive or disorienting than the large white dropdown at the top. It's encouraging to hear this feels like an intuitive interface to manage things at a high level :) |
This PR adds navigation menus to the browse mode sidebar. Allowing the user to manage menus from there.
It moves the current navigation sidebar to the left.
cc: @jasmussen, @jameskoster
Screenhots
Follow up
There is a new navigation block manager being implemented at https://github.com/orgs/WordPress/projects/60/views/1. We should try to use that new component here.