Skip to content
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

Navigation block - how do patterns and template parts define a navigation block's content #35947

Closed
talldan opened this issue Oct 26, 2021 · 18 comments · Fixed by #36238
Closed
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress

Comments

@talldan
Copy link
Contributor

talldan commented Oct 26, 2021

What problem does this address?

#35746 changed the way that the navigation block stores its data to be more like a reusable block, inner blocks are saved to a wp_navigation post type. The post containing the data must currently be created up-front when adding a navigation block.

This poses a problem for block patterns that might contain a navigation block, and also for themes that might define a navigation block in a template or template area. When creating a pattern how is the content of the navigation block defined?

Presently the navigation block stores the id of the post it's connected to in an attribute. This wouldn't be suitable for patterns or template parts because those ids are specific to a particular site.

There might be some overlap between this issue and #35750, as that could influence how the nav block defines its data.

What is your proposed solution?

This is a tricky one to solve as it's a chicken/egg situation. How can a pattern or template part define content that hasn't been created yet?

An option could be that a navigation block can still be defined as this structure:

<!-- wp:navigation -->
  <!-- wp:navigation-link /-->
  <!-- wp:navigation-link  /-->
<!-- /wp:navigation -->

Presently when a block like that is encountered, the block shows a prompt like this, asking the user to upgrade.
Screenshot 2021-10-26 at 5 34 30 pm

But maybe this could automatically insert an 'unsaved' version of the navigation block.

@talldan talldan added [Block] Navigation Affects the Navigation Block [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Oct 26, 2021
@mtias
Copy link
Member

mtias commented Oct 26, 2021

The former trivial representation of the menu is a good fit for patterns and should only be transformed to a CPT when the user modifies, imports, or adds its own content to the placeholders. We should make sure existing patterns work fine as starting points as well.

An option could be that a navigation block can still be defined as this structure...

That'd be my inclination too.

@kjellr
Copy link
Contributor

kjellr commented Oct 26, 2021

We should make sure existing patterns work fine as starting points as well.

Yeah, just want to reiterate this part. All of the Twenty Twenty-Two header patterns currently show that prompt from above.

In case it helps, we'd been using a single Page List block in most of our patterns:

<!-- wp:navigation -->
    <!-- wp:page-list {"isNavigationChild":true,"showSubmenuIcon":true,"openSubmenusOnClick":false} /-->
<!-- /wp:navigation -->

@talldan
Copy link
Contributor Author

talldan commented Oct 27, 2021

So it sounds like we should transition away from this idea of 'Upgrading' the block.

I have some ideas for a quick fix. The user could still be asked to save the block to a CPT, but it can be streamlined and only visible on block selection. That way the pattern preview won't have that ugly warning!

From there we should be able to iterate on what it means to have an unsaved navigation block with content. And the second thing we can look at is options for people creating patterns, because that's also challenging after #35746.

@talldan
Copy link
Contributor Author

talldan commented Oct 27, 2021

I've made a quick fix for the pattern previews - #35976.

It might not be ideal, but I think it's better than what we have.

It won't close this issue, let's continue discussing the optimal solution.

@noisysocks noisysocks added the [Priority] High Used to indicate top priority items that need quick attention label Oct 27, 2021
@noisysocks
Copy link
Member

#35976 is a great start, thank you!

So it sounds like we should transition away from this idea of 'Upgrading' the block.

Yes I think so. I think that the key problem we need to address is that we've added two extra steps to the UX for customising a pattern or template.

Before: Insert a pattern (or load a template) → Click on Navigation → Add a link
After: Insert a pattern (or load a template) → Click on Navigation → Click Upgrade → Choose a name → Add a link

I'm uneasy about shipping Gutenberg 11.9 with these two extra steps because we're asking a lot of people to install Gutenberg and test Twenty Twenty-two which uses patterns extensively.

Perhaps we can automatically "upgrade" the block to a CPT when the user makes a change and automatically infer a name for the navigation based on context? (e.g. "Header" if it's in a header)

@talldan
Copy link
Contributor Author

talldan commented Oct 27, 2021

I'm not convinced the two extra steps are a big problem. Other blocks that require saving to a post type (Template Part, Reusable) also require naming ahead of time. I acknowledge the experience in the nav block isn't perfect right now, it can be refined.

I'm happy to explore creating an untitled nav menu, or as you say infer the name in some way. My concern with that would be that a user might end up with many confusingly named menus. But it is an option if we want to reduce the friction here.

@jasmussen
Copy link
Contributor

Thanks for the ticket. The potential for saving navigation is that you can insert a header pattern, choose an existing menu, and you're on your way. The flow can be excellent.

As it exists today, there are some problems with the two-step placeholder, multiple similarly named buttons and labels, and no way to create patterns. These are all solvable, but they are also important to solve. To help the conversation along, I explored a number of flows in mockup form.

Shared here is the Variant C mockup which to me feels the most potent and comes with the fewest UX changes. As the name implies, I have other abandoned variants which I'll resurface later in case they help conversation.

Variant C: Prompt to save

Pros:

  • Minimal changes, allows pattern creation.

Insert a pattern and you get a snackbar with a Save button, as well as a Reusable-Block-like Save button in the block toolbar:
Start with pattern

Start entirely from scratch with an unsaved navigation block. This is is how patterns are created:
Start empty

Created a new synced menu:
Create synced menu

Swapping out a menu with another:
Swapping out a menu

Outside of the tweaks to the setup state, flow, verbiage, and toolbar buttons, the primary change here is that instead of preventing you from editing a block until it's saved, it informs you that you haven't yet. In my explorations I found this to be the simplest and most seamless way to allow the creation and usage of patterns.

@draganescu
Copy link
Contributor

draganescu commented Oct 27, 2021

I can't say why but the notion of "synced menu" is too complicated in my head. Maybe reusable menu? Even so, all meus should be reusable and we should not give the impression that there are two kinds of menus. Even in the menu switch dropdown (which I think is very cool) the title should of the dropdown should be plainly "Menus".

@talldan
Copy link
Contributor Author

talldan commented Oct 27, 2021

Even so, all meus should be reusable and we should not give the impression that there are two kinds of menus

I agree with Andrei, allowing the block to handle both persisted and non-persisted menus seems like a lot of complexity, both for the user and for us to manage.

There's also currently effort underway on how menu data can be retained when switching themes (#35750). One of the basic ideas is that a user shouldn't have to create a menu from scratch when they already have existing menu data. That's a core reason the current explorations promote the reusability of menus first. Bypassing it by allowing non-persisted navigation menus would circumvent this system, and the user loses a lot of benefits.

I think also the same could be true of patterns. If a pattern is for a header, should the user have to create the navigation block from scratch? Maybe it should pull in whatever menu data is already assigned to the header. I realise this wouldn't be perfect for some patterns that depend on the positioning of blocks to create a visual style (like the example above of a site title in the middle of links), but maybe it's a starting point. Every other visual aspect of a navigation block can be customized to look different between patterns.

In my explorations I found this to be the simplest and most seamless way to allow the creation and usage of patterns.

Honest question - Is the creation of patterns something that the average user will be doing? I might not be up to date on the roadmap for patterns, but would personally not have seen it as a priority for the average user. More of an advanced option that could be supported, but maybe not with as much priority in the interface.

I definitely think inserting and customizing patterns is something that could be improved.

@kjellr
Copy link
Contributor

kjellr commented Oct 27, 2021

My primary concern is just that existing patterns should still work without displaying the big message. As @mtias noted, we should allow them to work as one-off nav blocks until the user interacts with them.

This is more or less what @jasmussen is suggesting in the first step of his suggestion above. Showing a save button works, but maybe we only show that once the user has made a change to the block? It would be helpful if we can explain why they need to hit save too — users don't have to perform that extra action for most blocks.

The question of how patterns are created is a somewhat more complicated one. As @talldan noted, I don't think most folks will be creating patterns. But we do need to provide a way to do it for advanced users. It used to be as simple as just copying/pasting block markup, but I personally couldn't figure out how to use the navigation block at all in a pattern with the latest changes... I ended up having to copy/paste some markup from an old pattern. 😅 As a short-term fix, maybe there's a toggle in the Advanced section of the sidebar that lets you create a one-off nav block?

@gwwar
Copy link
Contributor

gwwar commented Oct 27, 2021

This is a pretty tricky one! Thanks for exploring this further.

So I think some of the UX oddness that I'm feeling is that the block is prompting me to do things before I understand why I need to do said thing. It also stops me from my what I'm trying to do at the moment, turning into a friction point. For example, when I click on "add link", I initially want to add some links, not pause to wonder what I'm saving and think of a name.

(If possible I'd really like for us to provide some named title default, then let folks change the name later).

CleanShot 2021-10-27 at 13 51 54@2x

Instead of copy heavy notices, I wonder if we can reuse some of the learnings from the Site Editor. Visually there are areas that are used in many areas by default, and editing some parts may modify multiple cpts. This is explained on save, but folks can edit everything else by default without interrupting their flow.

It is of course a technical challenge to accomplish, but I personally think we have a lot of implementation flexibility for doing that. For example, as an offhand example I don't think it'd be that odd to create a CPT autodraft on pattern drop that will populate with the suggested content.

For folks creating themes, it'd be even better if they could provide default named menu items/patterns. (Not sure how feasible this is).

CleanShot 2021-10-27 at 13 59 27@2x

Honest question - Is the creation of patterns something that the average user will be doing?

It's a less common use case, but I do think it's pretty powerful to be able to build patterns in the editor. That said, it might make sense to have a dedicated editor for this, or a specific action, like "export to pattern".

@talldan
Copy link
Contributor Author

talldan commented Oct 28, 2021

I've attempted another iteration where an wp_navigation post is automatically created on template insertion - #36024.

@jasmussen
Copy link
Contributor

jasmussen commented Oct 28, 2021

Important context for this conversation is to look at the user experience as it currently exists in trunk, and consider whether we can ship that in the next version of the plugin. At the moment the big warning that appears on selecting a navigation block from a pattern is causing big layout shifts:
trunk
pattern insertion

I would not feel comfortable shipping that in the plugin as is.

It also feels worth surfacing that as of the change to allow existing patterns to work, we effectively do allow both persisted and non-persisted navigation blocks to coexist. The Disabled box is just an invisible wall. You can still edit it using the list view and the code editor:

list view

You can change orientation, and you can change properties in the inspector:

inspector

The Disabled component can only ever work as a warning, a thin veneer. If instead we embrace allowing both persisted and non-persisted navigations to coexist, then we can focus on making it trivial and easy to wire things up to your menus. That feels like a much more productive path forward than trying to put walls in front of the content.

The thing is, we are so close to something amazing. The fact that I can insert a beautifully designed navigation pattern, and then swap it out with one of my existing ones, that's so potent:

choose another

As I look at that, I see such potential to help users wire up dummy content with real content in just a few clicks and be on their way! But right now you can't even swap out a pattern before you save it first — and what you save will be a pattern you only ever meant to customize and not keep.

That is all context for why I feel like we should remove the invisible walls, and instead use every tool in our book to let the user know that they have an unsaved menu, and unless they wire it up, it won't sync.

@jasmussen
Copy link
Contributor

Here's a new set of mockups that requires saving navigation, but postpones it and handles it all in the multi entity saving flow. As a net result, you can't save your post or page without also saving the navigation. But the choice to save isn't right upfront.

Variant D: Multi entity saving

Start with an existing menu. This one starts with a simplified placeholder and a dropdown with segmented submenus:

Start with existing

Start empty. Notice how as soon as you insert an empty navigation block, the multi-entity-saving dot appears on the publish button, meaning you can't leave the page until you've made a choice to save the menu. Note also how menu item name is automatically suggested, but with option to rename. This is validated by #36024 wherein the flow feels entirely transparent.

Start empty

Start with pattern. Similar to starting empty, in that it immediately adds the unsaved dot.

Start with pattern

@talldan
Copy link
Contributor Author

talldan commented Oct 28, 2021

@jasmussen I understand your passion around this particular issue, but we have to focus on two things:
a) Something that is achievable
b) Something that doesn't break the entire model of a persisted navigation block, because as mentioned it's central to so many things we're exploring.

Most of what you're proposing seems like an interesting north star, but it's not something anyone can realistically achieve in the time we have. We need to look at the smaller steps we can take to make the experience acceptable.

The idea of hooking into the entity saving panel does seem like a better direction to me, I'll try to think about whether I can make that happen.

Just to reply to some of the more individual points:

It also feels worth surfacing that as of the change to allow existing patterns to work, we effectively do allow both persisted and non-persisted navigation blocks to coexist.

This has actually always been the case since I shipped #35746. That it works like this can be considered more of a begrudging acknowledgement that there are non-persisted blocks in the wild, and we can't break those. Ideally we'd be thinking about how we can transition away from that and the opportunities we have rather than going back to it.

I would not feel comfortable shipping that in the plugin as is.

I'd reiterate though, we need to be smart about this given the limited time we have. How can we bring this up to an acceptable standard. Later down the line we can consider more fundamental changes to saving. There are always some rough edges when we try something disruptive. Ultimately, if we can't achieve the level of comfort we're happy with, I think the only option will be to revert the changes that make the navigation block reusable.

@jasmussen
Copy link
Contributor

Here are a few mockups that embrace and mimic how the Template Part block works. It uses multi entity saving and it has rename options in the Advanced panel.

You can pick an already created menu:

Start with existing

Start empty creates a new menu:

Start empty

The Replace dropdown could be even more similar to that of the Template Part, and be augmented with thumbnails, but even without that, it's a good way to swap out the menu of a pattern:

Start with pattern

@noisysocks noisysocks removed the [Priority] High Used to indicate top priority items that need quick attention label Nov 3, 2021
@noisysocks
Copy link
Member

@talldan: Where is this at now that #36024 is merged?

@talldan
Copy link
Contributor Author

talldan commented Nov 4, 2021

@noisysocks I still want to look into a solution for what @kjellr mentioned at the end of the comment here:
#35947 (comment)

But other than that, this should be close to being closed. I think we should open a separate issue for some of the designs above so that we don't mix up task tracking for feature freeze tomorrow with long-term plans.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress
Projects
None yet
7 participants