Create an administration UI for managing Navigation Blocks

I'll prototype this over the next few days.

@m4olivei

Awesome! Eagerly anticipating this; I hope to make a meaningful contribution here too. :)

Appreciate it.

I got to a good stopping point, so I thought I would post the MR. It's not done yet, but many of the back end pieces are put in place here, including (not exhaustive):

• New navigation_section config entity and supporting handlers
• Aligned NavigationSection plugins to be more block-like, includeing NavigationSection attribute
• Renamed plugin folder from Navigation to NavigationSetion to follow convention
• Define config schema for navigation_section as well as existing navigation_section settings
• Install 4 navigation_section config entities that we currently have
• Use navigation_section config entities to render the navigation toolbar
• Block-like navigation_section theme hook, preprocess and template

Still to do:

• navigation_section list_builder handler evaluation. Might not need this, came with drush generate
• Flesh out the admin UI on top of all the data layer implementation. Keep an eye toward allowing a future for multi-region management, but we only need content region management for now
• Make sure contextual region / menu stuff works as expected
• Much testing

Will keep working on these tomorrow.

Could not find the link to open the entity page therefore added code for the following:
1. Added the collection route for the entity
2. Added links file

Will keep testing.

Thank you.

Thanks for testing @Prashant.c.

As I noted in my previous comments, I'm still actively working on the issue. The default list builder came along with the drush generate command when I initially ran it to get a start on the config entity. We won't be using it, as we're going to create a UI like blocks (with "Place Navigation Section" buttons, dialogs, etc.).

I'll note clearly in comments when I'm done with the initial work and at that time will unassign and invite further commits. It's good to avoid duplicate effort. Thanks!

Thanks, @m4olivei! I didn't notice that you had already assigned it. Nevertheless, I'm looking forward to exploring the features, especially the "Place navigation section." Cheers!

More commits!

I've fleshed out most of the back-end pieces for the Navigation section layout admin UI. You can now:

• Visit /admin/structure/navigation-section and see a listing of existing navigation_section config entities. NOTE: we talked about limiting this to only the content region. It presently works with both regions, just to keep that possibility open. We can restrict that in future.
• Reorder navigation sections, even move into differnt regions. NOTE: presently the Javascript is not active, so you have to use the region select form element to change to a different region.
• Place navigation section brings up a dlalog with a library of navigation section plugins to choose from
• You can create a new navigation section config entity by clicking "Place navigation section" in the library dialog. You get the navigation section plugin form, and warpping navigation section entity form, just like blocks

Several caveats at this point:

• Any functionality depending on Javascript, I haven't gone through yet. Filter mechanism on the navigation section libary dialog is one example.
• Visibility settings don't get used yet
• Rendering multiple of the same can produce weird results, there may be additional themeing considerations to accomodate the added flexibility here.
• Not sure if exposing the 'Category' in the UI is useful? Probably worth it, who knows how this would get implemented, maybe many plugins would proliferate in contrib.

I'm continuing to work through open issues.

I'm at a good stopping point now. I'm going to unassign and set to Needs review.

From #6, I've finished:

• . I decided to keep content and footer region management. Good for testing, also, seems feasible that some users may prefer their shortcuts in the footer, which totally works.
• . More needed!

From #10, I've finished:

Additional notes.

From #10:

Rendering multiple of the same can produce weird results, there may be additional themeing considerations to accomodate the added flexibility here.

This is still true, but would be an issue for a followup ticket. It works now to do this. We can talk about theming considerations / restrictions in a followup.

From #10:

Not sure if exposing the 'Category' in the UI is useful? Probably worth it, who knows how this would get implemented, maybe many plugins would proliferate in contrib.

I thought about this. Category will probably be useful. By default it comes from the module providing the NavigationSection plugin, hence they are all showing as "Navigation". I can imagine a future when this gets into core where the User NavigationSection goes into the user module, the Shortcuts NavigationSection goes into the shortcut module, etc. Plus contrib and it will be useful to have and display. I left it in.

Any feedback on functionality or code review is very much welcome.

1. i don't really think that contextual links should be added. This is mostly admin menu. It is kind of annoying action for 99% of users who don't want to use anything except default navigation blocs layout.

https://gyazo.com/47492d4feda61a3b2f8a54b4c39ad4d6

2. I don't want jQuery appear in new core code.
https://www.drupal.org/project/drupal/issues/3052002 🌱 [meta] Replace JQuery with vanilla Javascript in core Active
I would better rewrite it with vanilla js.
Gonna work on it in next few days.

3. Probably we need to have button "Reset to default" in navigation layout settings? I guess can be much more blocks in future. Unexperienced users easy can break all site controls.

1. I concur that there should be a "Reset to Default" button available on /admin/structure/navigation-section for resetting the navigation section. This feature is essential in case users or admins inadvertently modify the settings and wish to revert to the default state. Currently, this option is unavailable, and having it would be a valuable addition.

2. Despite updating the section title in the backend, the displayed title remains unchanged. Refer to the image below for clarification:

3. There is an issue with the scrolling functionality when the "Administration" section is positioned in the footer. The navigation bar does not scroll due to an overlap problem, and no scrollbar appears. Please see the image below for a visual representation:

Changing IS to NW.

@m4olivei, excellent job!

1. i don't really think that contextual links should be added. This is mostly admin menu. It is kind of annoying action for 99% of users who don't want to use anything except default navigation blocs layout.

I tend to agree with you. When @ckrina and I originally discussed this, we said lets include contextual links, but yeah... now that I see it, it's problematic. Let's see what @ckrina thinks when she has a minute to review. I'm leaning towards excluding contextual links, or at least hiding it here for now and addressing the UX in a follow up in order to be able to include them.

2. I don't want jQuery appear in new core code. https://www.drupal.org/project/drupal/issues/3052002 🌱 [meta] Replace JQuery with vanilla Javascript in core Active . I would better rewrite it with vanilla js.

Yep! This Javascript was copy pasted from core's block code hence why jQuery came along for the ride. We can for sure rewrite to avoid jQuery.

3. Probably we need to have button "Reset to default" in navigation layout settings? I guess can be much more blocks in future. Unexperienced users easy can break all site controls.

Perhaps? Although I'd arguee for most users, they wouldn't venture into the admin UI to adjust anything here, especially if we remove contextual links. Not sure it's worth the effort.

2. Despite updating the section title in the backend, the displayed title remains unchanged.

Nice catch @Prashant.c. I've fixed that in the last commit.

3. There is an issue with the scrolling functionality when the "Administration" section is positioned in the footer. The navigation bar does not scroll due to an overlap problem, and no scrollbar appears. Please see the image below for a visual representation.

Yeah, there will be dragons when placing things outside their intended place. This is a front-end issue and outside the scope here. We can talk about restrictions we may or may not want to place here. Adjusting styles to cope with more flexible placement would be out of scope here and a good candidate for a follow up.

Updated issue summary with more remaining tasks based on discussion.

1. The concern mentioned in Point 2 on https://www.drupal.org/project/navigation/issues/3411099#comment-15378332 📌 Create an administration UI for managing Navigation Sections Needs work has been resolved.
2. The remaining two points pertain to frontend issues.
3. Should we consider treating the conversion of jQuery to JS as an independent task for the entire module?
4. Currently, it seems that decisions regarding "Reset to default" and "Contextual links" are the only outstanding matters.

Overall, looks good to me.

I’ve just manually tested it and it’s really cool seeing this working :D

we talked about limiting this to only the content region. It presently works with both regions, just to keep that possibility open. We can restrict that in future.

1. We should restrict this, yes. Either in here or in a follow-up, I don’t mind. The more important reason is the Usability problem it opens because a huge bottom region will the difficult to use in different breakpoints. Plus it’s supposed to be a “complimentary” and less important region with the main purpose to give the open/close functionality in desktop: this functionality disappears on mobile and the footer stays at the bottom with only the almost no used User menu.

2. Location of the UI: it just go into Configuration not Structure. It’s a UI to change of an administration UI, not the structure of the site being built. I’d recomment movig this to Configuration > User Interface > Navigation sections.

3. Contextual links. 100% agree with #13. But there should be a quick way to get to the UI to manage this. Maybe a link or something like that. But I’d leave that for a follow-up.

Not sure if exposing the 'Category' in the UI is useful? Probably worth it, who knows how this would get implemented, maybe many plugins would proliferate in contrib.

4. Agreed on keeping it. In the future maybe other kind of custom block could be available, like the bar Umami adds to say it’s a testing site.

Probably we need to have button "Reset to default" in navigation layout settings? I guess can be much more blocks in future. Unexperienced users easy can break all site controls.

5. From #13. I’m not sure about this being a need: this pattern doesn’t exist on the Block Layout UI nor in the menus themselves, and showing a button for that you should clearly explain what will change to, what is in the defaults first. Plus it can actually be a disruptive change for whoever click in there. I’d say this lays into the responsibility of whoever has given the permission to a user?

What about trying to solve this potential stressing situation for newbies with proper module documentation in Drupal.org and maybe linking it with something like “See the recommended defaults”?
 Either way I’d move this to a follow-up too.

Should we consider treating the conversion of jQuery to JS as an independent task for the entire module?

6. 
From #18.3. 
It would be great to not introduce any jQuery at all, but happy with it happening in a follow-up.

7. Why is it called “Navigation section” and not “Navigation block”? I find it confusing since I expect a region as a place to put several blocks if we rely in "Drupal terminology".

8. If you leave the Footer region without menus you loose the capacity to collapse the sidebar because the Collapse button disappears.

Probably it can be one contextual link in top of sidebar? in logo region.

Great feedback. I will work on these items this morning.

We met this morning to discuss the remaining items here. In summary:

We will create follow up issues for the following:

• I've hidden the contextual links. In a follow up, we will work on a single contextual link for the whole Navigation to link to the Navigation section UI
• Restrict configuration of the toolbar footer items. Users should only be allowed too manage the content region of the navigation through the UI.
• The help link, which has already been taken out of main nav, and is in footer of Navigation now, will be made its own item to be able to reorder it
• Revert to default in navigation layout settings from the UI
• Refactor jQuery to vanilla Javascript

Work to do in this issue

• . Done in 9c93a9f7.
• Rename ‘navigation sections’. Consensus amongst those in the meeting was 'Navigation components'. @ckrina to solicit some buy in on this, as the change is tedious to make. It's a machine name that gets used in the code a lot.

m4olivei → closed merge request !156

m4olivei → reopened merge request !156

ckrina → closed merge request !156

ckrina → reopened merge request !156

Rename ‘navigation sections’. Consensus amongst those in the meeting was 'Navigation components'. @ckrina to solicit some buy in on this, as the change is tedious to make. It's a machine name that gets used in the code a lot.

We discussed this with @lauriii: he suggested that we better go with "Navigation block" instead of components because he foresees conflicts and misconceptions with the components word the way it is used in Drupal. So let's go with Navigation blocks" and unblock this issue :)

Made changes to issue title and description to reflect new "Navigation Block" language.

I've made the change to rename "Navigation Section" to "Navigation Block" in the code. Ready for review.

Thanks @plopesc! I'll get to work on those straight away.

Fixes made! Please review.

Also see my comment in the one open thread. Let me know if your OK with that.

Thanks @plopesc!

Thank you for taking your time to address the feedback! Code looks good to me, also played with the TB environment and I didn't found any remarkable issue.
Only open point would be the automated tests, but I don't know how you plan to address them.

Thanks @plopesc!

I think with both @plopesc and @Prashant.c's reviews and all threads resolved, this is good to go.

Marking as RTBC.

Merged. Great work!! 🙌🏼

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