Problem/Motivation

Right now, we're printing a manually created menu in web/modules/navigation/src/Plugin/Navigation/NavigationUser.php into the footer of the admin toolbar. But the links listed in there are not the same ones the real menu would have.

Current hard-coded user menu

Proposed resolution

Place the real user menu in there. For now it won't need to be customized on any way, and it will always be printed if the user is logged in.

This way the menu should print the user menu we would see with the old Toolbar, and the mapping would be:

  • 1st level item "User menu" -> should print the actual name of the user, the same way it prints it now.
    • Profile -> View profile
    • Settings -> Edit profile
    • Sign out -> Log out

The links should be the ones provided by the menu, but this menu should be printed with the current classes we are using on the hard coded one.

Note: Post MVP we might add the option to print the picture of the user if there is one, but it is totally out of scope for this issue.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 user-menu-from-user-module-service.png69.03 KBprashant.c
#5 user-menu.png24.52 KBprashant.c
user-menu.png76.26 KBckrina

Issue fork navigation-3408260

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:

About issue forks

Comments

ckrina created an issue. See original summary.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Title: Print the real user menu in the footer » Print the real User account menu in the footer

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

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Active » Needs review
StatusFileSize
new user-menu.png24.52 KB

Attempted to explore a solution using the "/admin/structure/menu/manage/account" menu to display user menu links. By default, it only presents two menu links: "My account" and "Logout" (or "Login" for anonymous users).

User Menu
The menu links supplied by the toolbar are also hardcoded in the core/modules/user/src/ToolbarLinkBuilder.php file:

  • View profile
  • Edit profile
  • Log out

I would appreciate hearing your thoughts on this approach.

Thanks!

Prashant.c opened merge request !149

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Needs review » Needs work

Ah, good point @Prashant.c! I didn't know the Toolbar was using hard-coded links now. We need the 3 links we have right now in the Toolbar:

  1. A way to view the profile
  2. A way to edit the profile, which right now is going into the form
  3. A way to log out

So I would say then using those hard-coded links in the new navigation makes sense. Would it be a way to integrate this existing core code into this navigation and print those 3 links? If that was possible it would be logic that we would get rid of in this module and move it into core. We could change the naming of ToolbarLinkBuilder.php if needed, but we're actually working on a toolbar and naming it Toolbar too so it might not even be needed.

What I would try to do then also is using the existing user name (like "admin" for example) as the value of the parent item, that right now has the "User".

Thank you!

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
StatusFileSize
new user-menu-from-user-module-service.png69.03 KB

An alternative method involves retrieving menu links directly from the user module by utilizing the user.toolbar_link_builder service.
User menu
We can explore these strategies, or if you have alternative plans for implementation, we can discuss those as well.

Thanks

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Needs work » Needs review

What I would try to do then also is using the existing user name (like "admin" for example) as the value of the parent item, that right now has the "User".

I have done it, you might want to review the modifications in the most recent commit.
Thanks

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented

This is what was looking for exactly! Thank you! I'll RTBC it but it would be great to get a back-end dev to review the code :)

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Needs review » Reviewed & tested by the community

Changing the state.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Reviewed & tested by the community » Needs work

Changed the status to NW per @larowlan's feedback. Please change it back to NR if the changes have been made.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Needs work » Needs review

@ckrina

Changes have already been made yesterday https://www.drupal.org/project/navigation/issues/3408260#mr149-note242559

Thanks

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs review » Needs work

I've added some feedback in the PR.

It looks like @larowlan's feedback was all addressed. I made some additional comments, mostly with respect to keeping the existing naming and structure of the theme hook (menu_region__footer) as close to what it already is as possible.

Great work so far though, funcationally working as advertised.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Needs work » Needs review

@m4olivei, appreciate your review. I've taken care of all the comments you provided. Updating the issue status to NR once more. Thanks.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs review » Reviewed & tested by the community

Looks great! Thanks for that changes @Prashant.c. Also noting that all of @larowlan's requested changes seem to have been incorporated as well.

This is RTBC for me.

ckrina credited larowlan.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Reviewed & tested by the community » Fixed

Thank all, fixed!

Just a small suggestion if I may ask: it would make my life way easier that when a thread is resolved it is marked as so on a MR, so then I'm sure it's done (since it's not as easy for me to review back-end code) :)

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented

@ckrina Sure, we will do that.

Thank you.

Status: Fixed » Closed (fixed)

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