Add tree view to the navigation block #35149
Conversation
adamziel
added
[Block] Navigation
adamziel
requested review from
draganescu,
talldan,
jasmussen and
tellthemachines
adamziel
requested review from
ajitbohra,
getdave and
kevin940726
as code owners
adamziel
changed the title
adamziel
changed the title
|
Size Change: +1.17 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for trying this out!
The largest problem with this approach is that horizontal menu + accordion doesn't make much sense, yet it is a valid choice at the moment.
Accordions imply more than just a style change; they bring a whole new behaviour. For instance: the submenus will have to open on click instead of hover. We probably shouldn't even show hover as an option when accordions are enabled. We'll be able to leverage most of the markup we currently use for the open on click option, but we should do some research on whether any further semantics are needed for this pattern, especially from an a11y perspective.
It might make sense to add accordion as another block variation, along with vertical and horizontal. Or else it could be an option, perhaps even something that only appears when open on click is enabled. The design folks should be able to help out with those decisions 😄
The other thing I'm wondering about is what simple navigation links, with no children, will look like with this pattern. They should be visually consistent with the accordion headers, but the semantics will be distinct. Will it be weird to have some top-level items behave as links, and others as accordion headers?
Also, what happens when we add other types of blocks, e.g. site logo, to the navigation? How will they display?
I like making these options mutually exclusive – accordion on hover doesn't make much sense indeed.
A-ha, there's an issue here: Assumption: Clicking an accordion header merely opens/closes the accordion. You can't navigate to that page I wonder what would be a good way to communicate it 🤔 Maybe visually distinguishing the headers from the links? But that could be too vague. CC @jasmussen @javierarce @karmatosed |
|
This has come together incredibly fast, and is mighty impressive. As far as I can tell it boils down to this:
It still leaves me a bit unsure what problem this is trying to solve. While an accordion is a cool feature, it's also a rather complex beast, enough that the ticket for an accordion block has sat there for a while. Even if I can see accordion working as navigation (perhaps weighted towards the vertical variation of the navigation block), I wonder if it's too much complexity to add to an already complex block. What are some key challenges this needs to address? |
|
Good point @jasmussen! The biggest concern I know of is tight coupling between the navigation block and the navigation editor (#29747). The editor overrides very specific CSS rules to provide the "accordion tree" experience. It works now, but any change in block's CSS could break it. To solve it, we need to break the coupling. I think there are two ways to do it:
|
|
There are original user-facing problems that pre-date the technical problems @adamziel mentioned. The visual presentation of the navigation block has been built with full-site editing in mind where the editor closely matches the frontend. That's not the case in the navigation editor, the block will almost never match the frontend. There is no advantage to presenting the navigation block with a 'dropdown' style there. Compared to the classic menu editor, it's potentially harder for a user to build a menu with dropdowns. That's why the more structural accordion style was introduced in the editor. Before this there were some other explorations, like using List View for editing menus in a more structural way alongside the block. Those didn't work out for one reason or another. In terms of why the idea came up of introducing this accordion style in the block, there has been an expression of interest in different styles of navigation menus, including an accordion, so it seems like a good opportunity to converge. It may not be the right time or solution, it's open to debate. |
|
I have some additional thoughts. 😄 I've realised there's a lot of relevance between this and the comment here (which also closely relates to this issue - #34612.) That relevance is that if the future navigation block saves its inner blocks to a custom post type (or template part) to become reusable, then in an FSE theme there's very likely to be a use case for editing a navigation block's content alone (isolated editing). The block data that's being edited may not be assigned to template at all, in which case the theme will not be able to provide complete styling for it (we don't know if it should look like a header nav block or a footer nav block or a sidebar nav block). The thing that has occurred to me is that this is very a similar use case to the navigation editor, so the block should probably cater for it in the future. I'm thinking the navigation block could offer something more akin to a plain structural view when it's used in a context in which the theme cannot provide an opinionated style. This would be when the block is in the navigation editor in the 'classic mode' where the menu is still rendered using a walker. It's not possible to visually match the front-end style. Or as outlined above there's an FSE use case when a navigation block's data is being edited in an isolated way. Furthermore, it's probably worth thinking about how the block could have sensible default styling based on the template it will be used. Using dropdown menus in a header and maybe something more akin to a list in a footer. |
adamziel
changed the title
|
Here's an attempt number 2: We have a block attribute called
I think it plays well with the direction of custom post types / template parts – it's just an attribute on the top-level navigation block, and different blocks could render the same menus using different "display modes". What do you think @talldan? (There are some conflicts, but I'll refrain from investing time in solving them until we converge on direction) |
|
I'd defer to @jasmussen and @tellthemachines for the best feedback here as I know they've worked lots on these options and have some plans for changing how they appear. The challenge is probably having smart defaults, so when selecting accordion, it should probably enable (and prevent modification of) 'click to open submenus' and also switch to vertical. It does feel more like a block variation in that respect, but to do that we'd need to tackle removing the horizontal/vertical variations as per #34872. |
|
My main concern here is that creating an accordion version (variation, or whatever it might be) of Navigation will require us to solve a bunch of complex problems, some of which I raised in my comment above. Plus we have to solve those problems in a way that's compatible with its use in the Nav editor. For instance:
If top-level items in an accordion become static text, can we still treat them as links in the Nav editor? Because in an appearance-agnostic context, they should still be links. But perhaps these are problems that we should be attempting to solve anyway; the accordion (often sliding in from the side) is a common navigation pattern across the internet and it would be great to offer it as a possibility. If so, it's different enough from the existing pattern that it would make sense as a block variation, especially if we're removing vertical/horizontal in #34872. Then we could set it to open on click and disable any other options that don't apply. We should also consider adjusting the responsive menu to match when accordion is selected, because it's a pattern that works well on small viewports. I don't think that the solution of decoupling the Nav editor from block styles is necessarily a bad one though. It'll certainly be a lot easier than implementing accordion for the block 😅 and it'll free us from having to check the editor each time we make adjustments to block styles. |
|
I can add little value to what @tellthemachines already shared, only agree. The accordion behavior is definitely valid and valuable, but due to the likely complexity it will bring it might be useful to keep it separate from the already complex Navigation block (#21584) and then look at converging in the future if that becomes useful. |
@talldan I think this is a very, very good insight 👏🏻 That accordion vertical thing should be a good data-only-view in the block itself. This block easily becomes a world of its own 😂 |
adamziel
force-pushed
the
add/accordion-and-dropdown-as-style-variations
branch
from
9c144ba
to
5ece076
Compare
adamziel
changed the title
|
I pivoted this PR towards a editor-only "tree view" feature for the navigation block: Kapture.2021-10-08.at.16.12.19.mp4Now we don't need to solve any frontend issues with accordion on the first go. Oh, ant the icon is terrible – any suggestions which one would fit better? cc @tellthemachines @jasmussen @draganescu @talldan |
|
|
||
| // Styles for dropdown style menu. | ||
| .wp-block-navigation:where(:not(.is-tree-view)) .has-child { |
There was a problem hiding this comment.
I don't like how an editor-only classname creeps into style.css, but I don't think there's an easy way around this – style.scss is loaded in the editor so it needs to acknowledge editor matters.
|
I don't think we should go in this direction, because this doesn't add value to the Navigation block itself. We already have list view, which works the same for all blocks at all nesting levels. The problem we're trying to solve, as stated in #29747, is the brittleness of Navigation block styles inherited by the editor. We know that we want to optimise editor styles for ease of editing, not WYSIWYG; that's how we arrived at the current editor layout. So it makes sense for the editor to have its own styles, independent from Navigation block styles, which are geared towards WYSIWYG. Also, the exploration in #35418 is a big step in the direction of decoupling navigation content from presentation, and in that case editor specific styles will be necessary. |
I think it's worth questioning this. If the block moves towards storing content in something like a template part or CPT, then full-site editing will also have the same problem. There will very likely be a way to edit that template part/CPT in isolation. When doing so what should the block look like? |
In a block context we can/should rely on the list view IMO. |
Would you be able clarify your thoughts here. What's a 'block context'? I find this term a bit confusing because there's a feature called block context. Also, it should rely on 'list view' as in the List View feature? Should rely on it to do what? That seems like a non-sequitur because it's unrelated to the text you quoted which was asking what the block should look like. |
What if the Nav block isn't included in the Nav Editor at all? Isn't this what might happen if we start treating Nav block as a presentational container and saving the inner blocks to an entity? In this world, the Nav Editor becomes another Isolated Template Part editor concerned with managing the inner nav blocks only (no We would, of course, have to duplicate some CSS from the Nav block to provide a vertical layout for the Nav Editor, but that would be much easier than overriding the CSS coming down the pipe from the Navigation block. Again, imagine the inner blocks are all either raw (no configuration just representative of data) or they are configured via attributes defined on the parent Navigation block. Therefore if no parent Nav block exists then none of the configuration applies to the child nav blocks and thus we get a nice "raw" representation in the Nav Editor onto which we can sprinkle some simple styling. What do folks think? Am I on the wrong page here or does conceptualising the Nav block/editor like this work? |
I think this has been discussed a bit across issues. (#30007 (comment), #34496) There are some caveats. Some of the behaviors defined by the block will no longer work. Things like the new direct insert and default block behavior need to be replicated. Some of the inner blocks also have a So there are trade-offs both ways really. I still think it'd be more elegant if the navigation block were more contextually aware ... it needs to know "am I being rendered within a parent block list (am I being saved to something), or am I the root (am I only saving my data)", and then its behavior changes dependent on that. If it's being rendered within a template part, sidebar, post or page, it can have styling options as there's somewhere to save that (to the parent block list). If the navigation block is the root and there's no parent, there's basically no way to save those styles, so it shouldn't present styling options. There's also no way to know how the block should look visually and so it should maybe present itself more as a hierarchical structure. |
@talldan what I mean is that when we are in a block editor that edits content which will be rendered as blocks, we already have the List View feature so we don't need to change the way the block looks to make navigating its contents easier. There is no point in my head to change how the block edit looks and works depending on wether it is edited in isolation or not, except when we don't have a block rendering target (like we do when editing classic menus with a block editor) and the visual correspondence does not exist anymore. I mean, isn't the List View in the sidebar enough for a schematic data representation of the navigation block without visual enhancements? Why would we need another feature, specific to one block, for this? |
@draganescu I think this is the important bit, what should the block look like in this case? And there's also a situation where a single 'menu' can be rendered in multiple places, so how should it look in that situation? |
I think we're all agreed that an accordion-like view such as we currently have in the Navigation editor is the best for this situation 😅 but we haven't reached a decision on where the styles for such a view should live. I'm not convinced that we should bundle these styles in with the Navigation block, because they are unrelated to the block's presentation; they style an editor view of the block content, not the block itself. Instead, I think we should consider having these styles in the navigation editor package.
In light of #35746 and #35418, I assume that this would be the view of the navigation post type/template part that we're able to access from the "Appearance" menu in wp-admin? Can we leverage the navigation editor for this view too? Given we'd only be viewing the contents of the menu, I think that would make sense. And in that case, having the styles in the editor package would be the obvious choice. |
There's an
It's how it already works and it's considerably problematic for a number of reasons. Contributors don't expect to find block styles outside of the block-library package, so regularly don't update those styles. Better to have them with the block where folks expect them to be. Having them elsewhere completely breaks the BEM concept. |
The problems we've been having with style updates to the Nav block not being tested in the editor are mostly due to the editor using some of the block styling though, aren't they? If the editor view were completely independent from block styles that wouldn't be an issue. We also need to think about how this will work with Navigation as a custom post type. In that case, the editor likely won't be rendering the nav wrapper at all, so importing its styles might not make sense.
I thought BEM was more about naming conventions than where the styles are located. In any case, we're already not really BEM-compliant 😅 |
|
The Navigation Editor isn't an answer because that won't be shipped in 5.9. We need to be realistic, it won't be possible to pivot on the Navigation Editor because that's months of work. Lets focus on FSE here.
This is actually the only thing we need to think about right now. And it's what I've been taking about the whole time (#35149 (comment)). We can see in #35746 that the block styles are still wrong when editing without a wrapping nav block, and we've also established that it also wouldn't be quite right when there is a wrapping nav block and editing a CPT in isolation. I realise #35746 is still a bit up in the air, so I think we need to take a bit of a step back from just these styling challenges and think more solidly about how editing that nav block CPT in isolation will work. Styling is just one of the challenges. |
|
What I'm hearing is that there is nothing directly actionable in this PR for now. It seems to be interdependent with the isolated editor work (cc @kevin940726). Let's put it on hold for now and keep coming back to discuss new ideas as they emerge from all the FSE explorations. |
Description
Explorations related to #29747
This PR moves the tree view CSS from the
edit-navigationpackage to the navigation block, and exposes them a a "tree view" toggle button. We mostly shuffle existing CSS rules with very little new CSS added. Tree view is an editor-only feature and does not affect site rendering.Kapture.2021-10-08.at.16.12.19.mp4
(yes, the icon is confusing at the moment)