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] Investigate third party extensibility and customisation #17194

Closed
draganescu opened this issue Aug 26, 2019 · 4 comments
Closed
Labels
[Block] Navigation Affects the Navigation Block [Feature] Extensibility The ability to extend blocks or the editing experience

Comments

@draganescu
Copy link
Contributor

draganescu commented Aug 26, 2019

2020 Update on this issue

One idea being worked on is #19170 a remake of the Menus screen, the one in Appearance > Menus, to be another implementation of the block editor (probably something like the currently experimental widgets).

As a result, the navigation block is no longer independent from the current WP menus, and it becomes its responsibility to feature backwards compatibility to how current themes expect menus (structure, functions, markup etc.) - a development which makes this comment very relevant.

This means the following:

  • the menu navigation has to be able to create, update and map the menu it generates to the current menu structure stored in the database (the current menu entity)
  • on this point, it is essential that we have the menus endpoints in the REST API which allow full CRUD operations on the menu entity.
  • more than just working with the current menu entity, the navigation block should provide seamless access for current themes to the menus resulting from it, therefore:
    • the rendering on the server should use a Walker class
    • the resulting menu should have the same utility classes as the current result of wp_nav_menu

On the subject of using a Walker class, since the Navigation block will work with the same menu entity that exists today, all the menu related functions of WordPress will just continue to work. As a result, the only reason why the actual rendering of the Navigation block should follow the current rendering of menus is for the following situation:

  • a user is having their website using a classic theme (not block-based)
  • the user adds a menu in the Appearance > Menus page using the new Navigation block
  • as they are using a classic theme, the user will pick a location (even if not named so, we need it so we can respect the theme's functionality)
  • then, in the editor, the user has a page and uses the Navigation block to make a new menu or show an existing menu.
  • the result is that in the "location" the menu comes from wp_nav_menu while in the post's content the menu is the result of rendering the Navigation block. They should be the same thing.
  • the theme might use a Walker class to render the menu. That should reflect in what the Navigation block renders as well.

Open questions

Does it make sense that the navigation block is available in the post/page editor?
Maybe it should be a block reserved only for block-based themes and the template/template part editors.

In either case, the Navigation block still needs to work with the current menu entity in order to allow switching between a classic and a block-based theme. But, if the navigation block can only be used in block-based themes its own dynamic rendering can be whatever is best, including implementing good separation of concerns such as the one described in #20178

Props @talldan @obenland for chiming in with valuable perspectives.

Original issue

The navigation menu is getting dynamic following the merge of the #16796 PR. The current server-side rendering is the most basic possible, with a recursive function which builds an HTML list.

The original intent was to use a Walker class but while implementing we couldn't exactly figure out why would we?

The first idea we had was to hook in wp_nav_menu in such a way that themes could render navigation menus made with the block as if these menus were standard WP menus.

This idea however means that we should also make these menus visible in WP admin so the user can map them to theme defined locations.

Another idea was that by using a walker we could allow theme developers to alter the menu's HTML.

However, why would they, as these menus will either be in post body or in block areas. When they'll be in block areas, we're in full site edititing land and theme building will be very different from what it is today. One major difference is that themes will be unlikely to have to modify the HTML of a block to achieve an effect.

Therefore, I am opening this issue to have some feedback on how to best render the navigation on the server and why would using a WP Walker be bring any benefits.

@noisysocks noisysocks added [Block] Navigation Affects the Navigation Block [Feature] Extensibility The ability to extend blocks or the editing experience labels Aug 26, 2019
@noisysocks noisysocks changed the title Should we render the navigation block with a Walker? Navigation block: Investigate third party extensibility and customisation Sep 24, 2019
@obenland
Copy link
Member

There are several advantages to using the existing menus api, mainly backwards and forwards compatibility.

Menus are complicated enough as it is. Creating a third interface that is incompatible with the existing two will not help that. As a user I'd also expect to be able to use existing menus in the new navigation block, and at the same time reuse menus that I build within that block in other menu locations. Anything else seems like an oversight.

WordPress as a project has been taking pride into the effort it puts into backwards compatibility and ease of use. "Why would we use the Walker class?" feels like the wrong question to ask. Why wouldn't we try to make sure that navigation blocks look similar to existing navigation menus, is more what comes to mind for me.

@draganescu
Copy link
Contributor Author

Well @obenland I fully agree with your take, but the problem here is that these menus made in Gutenberg are not meant to show up in the current menu management screen, nor to be used in "menu locations".

Also, FSE will probably deprecate menu locations by design, not because locations have anything wrong, but if you can visually place the menu anywhere as a block, the theme does not need to specify an available location.

Using the Walker has the advantage that themes can render specific HTML for the menu, indeed. However, given Gutenberg's strive to represent content in the editor as faithfully close as possible to the front end result, I am not sure we want that, unless we render in the editor using the walker as well. If we render the edit component using the walker, that means it will be a SSR component ...

Again, I am completely in the tune of backwards compatibility and maintaining existing APIs and I have actually implemented the rendering of the menu on the server using a walker. However, the use cases for this block (the menu navigation block) don't really show the need for it (using a walker).

@obenland
Copy link
Member

If this is actually something that's still open for discussion, I'd encourage you to reach out to the theme review team, maybe @westonruter who worked on menus in the Customizer, and bring it up in a dev chat for broader consideration.

As to why use the Walker: So you're not tossing out navigation styles of every theme ever built.

@draganescu draganescu changed the title Navigation block: Investigate third party extensibility and customisation [Navigation] Investigate third party extensibility and customisation Feb 18, 2020
@draganescu
Copy link
Contributor Author

I believe it is time to close this issue. In the past two years:

  • The navigation editor work has been sidelined in favor of introducing an isolated navigation editor for the site editor
  • The classic menu block explorations were a dead end
  • Adding blocks to classic menus did not seem like an achievable and worthwhile endeavour

Therefore the discussions here are obsolete.

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] Extensibility The ability to extend blocks or the editing experience
Projects
None yet
Development

No branches or pull requests

3 participants