Problem/Motivation

The navigation prototype is a HTML mockup where the menu links have been hard-coded. In order to progress further and enhance the functionality of our prototype we need to get it closed to what regular menu items do. As a child of #3381754: [PLAN] Use Menus to generate the links in the sidebar, this is the first step to move towards that direction.

Proposed resolution

Use a new Navigation Section plugin type where each "section" (ex. Content, Admin, Bookmarks), responsible for generating it's menu items. Bookmarks and admin menu items are provided by a Drupal service. Otherwise, items from the other sections are hard coded in the plugin for now.

Remaining tasks

User interface changes

None

API changes

Data model changes

Issue fork navigation-3383896

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

hooroomoo created an issue. See original summary.

hooroomoo’s picture

Assigned: Unassigned » hooroomoo
Issue summary: View changes
hooroomoo’s picture

The variables are declared inside of the twig file so it is more of an exploratory issue for now

hooroomoo’s picture

Title: Convert twig template to use variables » [WIP] Convert twig template to use variables
lauriii’s picture

hooroomoo’s picture

Title: [WIP] Convert twig template to use variables » [WIP] Render menu items using Plugins
hooroomoo’s picture

Title: [WIP] Render menu items using Plugins » [WIP] Generate menu items using Plugins
hooroomoo’s picture

Title: [WIP] Generate menu items using Plugins » Generate menu items using Plugins
hooroomoo’s picture

Title: Generate menu items using Plugins » Generate menu items using plugins
hooroomoo’s picture

Issue summary: View changes
hooroomoo’s picture

Status: Active » Needs review
hooroomoo’s picture

Status: Needs review » Active
hooroomoo’s picture

Assigned: hooroomoo » Unassigned
Status: Active » Needs review

claireristow made their first commit to this issue’s fork.

ckrina’s picture

Issue summary: View changes

Updating issue summary to make it easier to understand what the issue does, so we can get reviewers easier.

claireristow’s picture

Code-wise, I don't have enough module experience to approve but functionality-wise, this is looking great!

mherchel’s picture

Can you bring this up to date with 1.x now that #3386509: Refactor JavaScript is merged?

tedbow’s picture

Status: Needs review » Needs work

Looking good. Did a code review. See MR comments

hooroomoo’s picture

Status: Needs work » Needs review
mherchel’s picture

Currently working on the templates here to ensure strings are translated, markup and aria attributes are correct.

mherchel’s picture

The templates are in good shape now. There's a number of @todo's that we can get to later.

Still needs PHP and functionality reviews.

Note that when tried to test this, I ran into an error

The website encountered an unexpected error. Please try again later.

Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "plugin.manager.navigation_section_manager". Did you mean this: "plugin.manager.navigation_plugin_manager"? in Drupal\Component\DependencyInjection\Container->get() (line 157 of core/lib/Drupal/Component/DependencyInjection/Container.php).

I resolved this by ) checking out a previous version of the code (7db614c10cb472347875141e19d239cbe2df5624), 2) uninstalling the module, 3) checking out the latest version of the code, and 4) reinstalling the module.

ckrina’s picture

After following the steps @mherchel mentioned in #22 (because I got the same error) I've been able to test this and feature wise works as expected.

mherchel’s picture

Thanks for the reviews @deviantintegral and @tedbow!

What's the next step here? Should these be followups, or does this issue still need work?

mherchel’s picture

Status: Needs review » Reviewed & tested by the community

Had discussions with @deviantintegral and @tedbow at https://drupal.slack.com/archives/C7AB68LJV/p1695052420501349.

Both of them are confused on where the data is coming from.

From @tedbow

I am little confused about the purpose of this issue vs using the menu system. It sort of uses the menu system, as in NavigationAdmin uses a menu but NavigationUser does not. Eventually will these all use menus? If so couldn’t you have just as much control over the markup by just having specific templates for these menu rather than just introducing the new NavigationSection concept
does 'base hook' => 'menu', mean that all the theme process stuff will still apply to these navigation sections also? (edited)

from @deviantintegral

I was also a little confused about this, but assumed I'd just missed prior context. The big thing I see is that I could see modules adding items to navigation sections that aren't menus, but some other UI widget. Like I think environment indicator is a good example of that? But I also think until we start wanting to create releases there isn't much harm in merging the MR, given that it's less hardcoded than the current code.

For now I'm committing this (so it can unblock some work on the templates and CSS). I'm adding this additional comments and info to #3381754: [PLAN] Use Menus to generate the links in the sidebar, where we can figure out how we want to get this data.

  • mherchel committed 17970de1 on 1.x authored by hooroomoo
    Issue #3383896 by hooroomoo, mherchel, claireristow, tedbow, ckrina,...
mherchel’s picture

mherchel’s picture

Status: Reviewed & tested by the community » Fixed

Thanks everyone!

Status: Fixed » Closed (fixed)

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