Convert navigation sections to blocks and use the menu system

Yesterday, we had a call reviewing this with @ckrina, @laurii, @ted.bowman, @tim.plunkett, and myself, along with some followup chats.

This is from my own notes on the call, feel free to comment if I misrepresent anything!

In terms of functionality, site builders:

• Should be able to customize the links in menus, just like they can today.
• Should be able to add brand-new menus that they create from scratch, or menus from other modules.
• Should be able to reorder menu blocks themselves.
• Should be able to add non-menu blocks to the green content area in this screenshot, as long as those blocks have been explicitly tagged somehow as being navigation menu compatible. For example, we could easily imagine site builders wanting to somehow put a list of recently edited content int the menu. While we wouldn't want to expose all Views blocks by default, once we have a way to mark those blocks as good in the nav menu we want site builders, and not developers to be able to manage them.

Developers:

• Should be able to customize the top region and the bottom region, for modules like Workspaces or Environment Indicator.

From a technical perspective, this is actually the first time plugin implementations have been subclassed in core. @tim.plunkett pointed out that there's real dragons as far as backwards compatibility goes, especially with constructors. There are instances of contrib modules doing this (like superfish), but in practice we may make things simpler and just ditch the whole aspect of subclassing. As well, it may be easier to alter in additional block annotation properties than to try and do an entirely separate annotation type.

For next steps, I would like to:

• Implement an example integration for a core module like Announcements or Workspaces.
• Use that to learn what makes sense as far as the existing "as block plugins" approach goes.

I've redone how the plugins are handled and I'm much happier with the approach. IMO it's simpler and more robust.

Instead of a separate annotation, we now look for navigation on the normal Block annotation. I've defined it as an array, so we can add keys like default_region, weight, and so on. To do this, we add our own plugin manager that wraps the block manager (it's not really decoration because we don't swap the service). It handles filtering available plugins and validating the additional keys.

Then, I implemented the announcements feed as a demo block. An interesting side effect is that the Announcements menu item works as is, showing the dialog of announcements on the left hand side.

For next steps, it would be great for someone else to look at what I've done with the navigation block manager and share any compatibility concerns or similar I may have missed.

To followup and summarize on next steps for this:

There's some skepticism that it will be possible to have a single block implementation that can ever work in both a navigation and non-navigation context.
The block system has intentionally been designed to not include any sort of real rendering context in the `build()` method.
There's also some worry that if there are separate blocks implemented to handle these cases, that the regular block admin could have duplicate blocks.
While Blocks have a context system (\Drupal\layout_builder\Plugin\Block\ExtraFieldBlock does this to associate an entity object), in practice they are best used for data and not theme or rendering information.

A good next step would be to walk this back to having a separate plugin implementation, _but_ still have Navigation blocks implement or extend \Drupal\Core\Block\BlockPluginInterface. That interface already defines most of what we need for navigation blocks, and would provide a familiar pattern for contrib modules to implement navigation support, or to more easily extend and share code for existing blocks if it does make sense.

Prashant.c → made their first commit to this issue’s fork.

@deviantintegral,

Your handling of blocks and menus in this implementation is exceptionally elegant.

@ckrina,
The following tasks have been completed by @deviantintegral; please correct me if I've missed anything:

1. Created a menu named "Content" using the config file.
2. Developed two new plugin types: one for "Navigation Block" and the other for "Navigation Section."
3. src/Plugin/Block/NavigationMenuBlock.php renders the menu links from the "Content" menu.

While there are various other changes made, for now, I suggest focusing on incorporating the Navigation Block and related code to display "Content" menu links in the sidebar from the backend.

I am ready to commence integrating the necessary modifications into https://www.drupal.org/project/navigation/issues/3408298 📌 Create the Content menu Needs review

Thanks!

Thanks Prashant! I see two next steps depending on your comfort and interest.

1. Create an issue that does nothing other than create the Content menu, and pull the related code out into that. Don't even worry about displaying the menu; let's just get a menu for Content in that we can reference in future work.
2. Change the current plugin implementation back to having a totally separate plugin manager from Blocks. Our plugin type should still require that they implement or extend \Drupal\Core\Block\BlockPluginInterface. But, the discovery, namespace, and directory should be separated out again.

I've had some things come up that will limit my availability for this, so I'm really glad to see you here offering to help. Thanks!

Thanks @deviantintegral for the directions!

Then work from 1 can be addressed in 📌 Create the Content menu Needs review , and this issue can focus on 2.

Will be working on this early next week. I have some things done locally, will aim to get a more complete MR ready.

I've pushed up my work. I'm not totally convinced this is done, would appretiate thoughts / opinions. In summary, I have:

• Brought changes from 📌 Create the Content menu Needs review so we have the Content menu created, able to use
• Converted from Annotations to Attributes, per [#3395582]
• Adjusted core requirement in info.yml to be ^10.2 for plugin type PHP attribute support
• As per @deviantintegral, we now have a separate plugin manager, but we use BlockPluginInterface. Our NavigationSection attribute extends from the Block attribute. We also directly use BlockPluginInterface in the NavigationSectionManager. I'm not sure we want this tight coupling? Interested in thoughts around this.
• Changes required from a plugin usage and implementation standpoint as a result of moving to be Block-like
• Updated the NavigationContent plugin to use the actual Content menu. It functions similar to NavigationAdmin plugin

@m4olivei I tested it manually and seems to be working fine and changes are reflecting properly in the frontend for the following:

1. Rearrange menu items.
2. Disabling menu items.
3. Adding menu items.

@m4olivei,

It appears that the merge request has conflicts. Would you mind checking and resolving them at your convenience?
Changing the issue status to NW.

Thank you.

Thanks @Prashant.c. I'm expecting more merge conflicts when 📌 Create the Content menu Needs review is merged, which feels like it will be soon. Once that lands, I'll sort it all out here.

I've resolved the conflicts that the recent merges to 1.x brought.

ckrina → closed merge request !118

ckrina → reopened merge request !118

Just to note, Tugboat is not happy ATM b/c I bumped the core_version_requirement to ^10.2. It fails on trying to enable the module b/c of that.

This shouldn't be an issue b/c we're declaring a tag of q0rban/tugboat-drupal:10 for the Tugboat image. Per the docs that should net us the latest 10.x release (at time of writing 10.2.0). Looks like the images live on DockerHub, and the last tagged images were posted 6 months ago. Latest 10.1.x is 10.1.0 (despite latest being 10.1.7) and there are no 10.2.x tagged images. I've reached out to the Tugboat support folks on this.

Thanks @deviantintegral. I've made all the suggested changes.

I think I could go further here, but it's as good a place as any to stop. I'm thinking that when we start to tackle an admin UI for placing Navigation Sections, we're going to need a new config entity type that is like the block config entity type in order to store config for each instance of a Navigation Section placed. Just like Blocks. The next step in my mind is to create that config entity, and then arrange for navigation module to be installed with the default instances of Navigation Section config entity in the navigation side bar that we have now. But I think that's distinct enough that we're at a good stopping point.

@ckrina I've marked all threads in the MR with ✅ indicating they are resolved. I don't see any pending feedback from @Prashant.c. Also as a bonus, Tugboat is back in action for this MR! The Tugboat support folks fixed an issue where the Drupal 10.2 images were missing in action.

Given @deviantintegral's thumbs up, I'll go ahead and mark this as RTBC. Feel free to adjust if I'm overstepping.

Seeing this as RTBC is great news! I understand this can go further but it can happen in follow-ups, so merging this to be sure the follow-ups are easier to address.

Thanks all!

Yay! Thansk @ckrina!

Automatically closed - issue fixed for 2 weeks with no activity.