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: develop a content-only representation #65699

Open
Tracked by #66499 ...
mtias opened this issue Sep 27, 2024 · 13 comments · May be fixed by #66753 or #67389
Open
Tracked by #66499 ...

Navigation Block: develop a content-only representation #65699

mtias opened this issue Sep 27, 2024 · 13 comments · May be fixed by #66753 or #67389
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Block] Pattern Affects the Patterns Block [Feature] Navigation Menus Any issue relating to Navigation Menus [Feature] Write mode Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement.

Comments

@mtias
Copy link
Member

mtias commented Sep 27, 2024

There should be a way to make edits to your menu while on content-only mode. The canvas edits should be restricted but the inspector menu should be reachable.

We should explore exposing the "menu list view" in the inspector of template parts or patterns that contain a menu when the user is on content-only mode.

There should be a clear way to reach that panel from the toolbar of the nav block and the template/pattern. Could be an "edit menu" that just toggles the inspector with the tab for the menu open.

@mtias mtias added [Block] Navigation Affects the Navigation Block [Block] Pattern Affects the Patterns Block [Feature] Navigation Menus Any issue relating to Navigation Menus labels Sep 27, 2024
@youknowriad
Copy link
Contributor

How do you see this kind of feature being "declared" for block types. In other words, until now, block types were not aware about whether they are in "content only" mode or not. We only detected blocks that have a "content" role attribute and allowed some specific behavior there. How do you see that evolving, should block types receive some kind of isSection flag and enable/disable behavior.

@mtias
Copy link
Member Author

mtias commented Sep 27, 2024

I don't think we should abstract this too soon. The navigation block already has a special tab in the inspector, so perhaps it's part of formalizing that view down the road. Conceptually, I don't think blocks should be made aware of "modes".

@SaxonF
Copy link
Contributor

SaxonF commented Oct 1, 2024

I think we can tie this work into the content only stream as well as synced patterns in a few iterative ways.

  1. Add an "edit menu" button to the Navigation Block toolbar. This is not directly related to content only but it helps to resolve some of the same pain points. The navigation block would be special in that other blocks would go with a generalised label and less prominent action.
  2. Treat template parts in the same way as synced patterns. Clicking a template part should always select the template part which contains an edit button. Clicking the edit button puts us into focus mode for the part (like synced patterns) with the inspector open. You can currently select the site ltitle block within a template part directly for example.
  3. When editing a template part, and in an content only mode, show editable blocks in the inspector. An editable block may site logo, site title or navigation block.
  4. Adjust the content only behaviour so that clicking a block that has deeper attributes (like navigation block) it drills down in the inspector but offers a pathway back up. This has previously been explored by @talldan as part of Writing and contentOnly modes #60021
  5. Allow the selection of template part when in the new zoom out / content only mode.

It may look something like this

edit-navigation-example.mp4

The above shows two ways of accessing a menu. The first via the content list, and the second via the canvas with an "edit menu" button that just opens inspector with menu tab open.

In future we can look at surfacing overridable properties directly vs drilling down like what is proposed above.

@draganescu
Copy link
Contributor

until now, block types were not aware about whether they are in "content only" mode or not.

Conceptually, I don't think blocks should be made aware of "modes".

If blocks are not aware of the mode they're in than something like #60412 can't work. Either the block tells the block editor that something is editable in content only, or the block editor designates what is content in content only. We went the first route and the block tells the block editor via the role prop of attributes what's editable in content only.

But the navigation block does not really have any content. The content is stored in the navigation link and navigation submenu blocks which are navigation block children. Updating the defition of these two types (link and submenu) to include role content on attributes whcih are content partly gets us there.

navigation-content-only.mp4

But, what is lost, we can't see the tree of links 😢 However that seems to be an issue for the whole block tree which is flattened in content only.

  1. What's the problem with preserving the flat block tree for navigation as well?
  2. What is wrong with editing on canvas? Why would we want the navigation list view? @richtabor expressed his concern about adding drilldown to content only, which is kinda the same with the proposal above (an edit button in navigation's block toolbar). I think we used to have one ...

About 2 there is something else to consider: if a user is only expected to replace pictures and text, then why do we expect them to rearrange the site's hierarchy?

@draganescu

This comment has been minimized.

@draganescu

This comment has been minimized.

@draganescu
Copy link
Contributor

Follow-up from a discussion with @youknowriad we can update content only to:

  • enable blocks with controlled inner blocks to expose these inner blocks in the block inspector via a list view
    • this list view auto engages when a block with controlled inner blocks is selected in content only mode
    • the block inspector will support drilldown for blocks that show this list view of inner blocks
  • make blocks with controlled inner blocks in content only are selectable but not editable in the canvas

This is a good start for an exploratory phase which could answer things like:

  • Where should this list view appear? Does it appear on its own, or do we squish it bethween parent styles and block quick navigation?
  • How is this redefining content only? If all blocks with controlled inner blocks become editable via the inspector, we break the contract that only role="content" is editable in content only.
  • How much can we bloat the list view with? To implement the ideas above we'll need a new flag in the list view that tells it to disregard content only and list the full block tree not the flattened editable one.
  • How much can we bifurcate the implementation of the block inspector? To add drill down just for this list view but not for other blocks may be a very specific execution path with lots of code serving a single small UX surface.
  • Is this UX actually good? Having some exploratory PR will allow hands on interaction.

@draganescu
Copy link
Contributor

@youknowriad following the PR (#66753) the incipient code and the reviews show that we're fighting the system rather than enriching it, implementing the idea here. Ideally we should reassess what this issue means.

The way I read both the descriptio and the answer from @mtias we should start with the navigation block being "special" again. Probably a better approach is to not use the block inspector and just update the navigation block to render a popover with it's tree from inside the block. I'll try this too.

@talldan
Copy link
Contributor

talldan commented Nov 19, 2024

The two main problems in the PR IMO are:

  • Defining this for all controlled block parents in the way the PR does is too broad. This could affect some controlled blocks in weird ways, especially the pattern block which already has its own treatment for editing content. I think it's ok to hard code it to specific block types to start with.
  • The inner blocks of the navigation block are blockEditingMode: 'disabled' in state, but the PR is circumventing that to make them selectable/editable. I feel this is breaking what blockEditingMode: 'disabled' means. It's another thing that could result in some weird bugs. It'd be better to look at giving navigation links (and other navigation inner block types) the correct role: content definitions and return the appropriate blockEditingMode for these blocks (which might be something that doesn't exist right now).

@getdave
Copy link
Contributor

getdave commented Nov 28, 2024

@talldan I've been exploring what you suggested above in #67389 building off your excellent work refactoring all the blockEditingMode stuff to the reducer.

The main Issue I've come up against is how we handle blocks such as Page List which frequently appear as children of the Navigation block but which don't have any attributes that warrant role: content.

Options I've been toying with:

  • adding role: content to one of the attributes such as isNested which don't provide any UI anyway, but which would force the block to be contentOnly
  • hard coding exceptions into the state (seems like a huge hack but it is possible).
  • creating a new supports flag which can be used to derive the contentOnly state for the given blocks

I'd be curious to hear both yours and @draganescu thoughts on this 🙏

@getdave
Copy link
Contributor

getdave commented Nov 28, 2024

I'm also wondering whether we need to provide a means to distinguish between block attributes which are role: content. Then when in contentOnly we conditionalise the render.

We could then incrementally update blocks to render controls for their attributes conditionally. We could then we could swap out the attributes exposed to BlockEdit based on blockEditingMode being contentOnly.

@talldan
Copy link
Contributor

talldan commented Dec 3, 2024

The main Issue I've come up against is how we handle blocks such as Page List which frequently appear as children of the Navigation block but which don't have any attributes that warrant role: content.

It's interesting. I think part of the issue is that I'm not sure how Page List should ideally work in contentOnly mode. It seems like quite a complicated block when the experience is supposed to be purposely limited to simple content updates.

The one aspect of it that could be an option is changing the parent, so perhaps that's the attribute to add role: content to. It does govern the block's content, so maybe it's logical.

There's also Page List Item, which already has attributes that role: content can be added to, so I wonder if that could be visible. Again, I wonder how it should work, if the feature where it converts itself to regular links is still available. It is also possible to show the Page List Items, but hide the Page List. That also partly makes sense to me as it'd be filtering the down to purely content blocks. 🤔

I'm also wondering whether we need to provide a means to distinguish between block attributes which are role: content. Then when in contentOnly we conditionalise the render.
We could then incrementally update blocks to render controls for their attributes conditionally. We could then we could swap out the attributes exposed to BlockEdit based on blockEditingMode being contentOnly.

If I'm understanding correctly, then there's some past discussion around this subject:

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 [Block] Pattern Affects the Patterns Block [Feature] Navigation Menus Any issue relating to Navigation Menus [Feature] Write mode Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement.
Projects
Status: Now
7 participants