Problem/Motivation

The backend parts of these were attempted to be moved in #3611762: Move toolbar hook implementations to the toolbar module but in many cases the utility classes, CSS/JS libraries, etc. stayed around with the modules apparently.
-
1. \Drupal\announcements_feed\LazyBuilders:48 — uses toolbar CSS classes and announcements_feed/drupal.announcements_feed.toolbar library.

2. User module integration in user.services.yml:53 — defines user.toolbar_link_builder and ToolbarLinkBuilder.php itself, despite the hook already moved to Toolbar.

3. Contextual menu integration for Toolbar has this toggle feature that shows ALL the contextually editable things. Navigation does not implement this. It was decided in #3465295: Integrate Top Bar Navigation with Contextual editing 2 years ago not do that. The hook implementation was moved to Toolbar already but contextual.libraries.yml:25 was not.

Steps to reproduce

Proposed resolution

Move the remaining toolbar-only code from the three modules into Toolbar. Icons the modules still use themselves stay where they are and Toolbar references them in place. Icons that are only used in Toolbar move to Toolbar.

User

  • Drupal\user\ToolbarLinkBuilder moves to user module along with the service definition (and name).
  • The .toolbar-icon-user rules move from user/drupal.user.icons into Toolbar's icon stylesheet and its Claro and Default Admin variants. The library is removed. person.svg had no other users, so it moves to core/modules/toolbar/icons.
  • big_pipe_test used the old service as an arbitrary lazy builder. It now has its own callback.

Contextual

  • contextual/drupal.contextual-toolbar (edit mode JS and tab CSS) becomes toolbar/toolbar.contextual. JS namespaces are unchanged.
  • The .toolbar-icon-edit rules move into Toolbar's icon stylesheets. The pencil icon stays in core/misc/icons as it is used elsewhere.

Announcements

  • LazyBuilders and RenderCallbacks and service definition move to Toolbar.
  • announcements_feed/drupal.announcements_feed.toolbar moves to toolbar. The bell icon stays in the announcements module as the dialog stylesheet uses it too.
  • The renderer, controller, block, dialog and page libraries stay. The block uses them without Toolbar.

Remaining tasks

Review.

User interface changes

Introduced terminology

API changes

Renamed without deprecation similar to #3620812: Move toolbar support from Workspaces to Toolbar:

  • Drupal\user\ToolbarLinkBuilder
  • Drupal\announcements_feed\LazyBuilders
  • Drupal\announcements_feed\RenderCallbacks
  • user.toolbar_link_builder service
  • announcements_feed.lazy_builders service
  • user/drupal.user.icons library,
  • contextual/drupal.contextual-toolbar library,
  • announcements_feed/drupal.announcements_feed.toolbar library

Data model changes

Release notes snippet

LLM disclosure

LLM was used to find the issues and partially to resolve them. With human review.

CommentFileSizeAuthor
#31 3618746-nr-bot_cqpfcdco.txt91 bytesneeds-review-queue-bot
#28 3618746-nr-bot_yoeb7wrn.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3618746

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

gábor hojtsy created an issue. See original summary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes

virag.jain made their first commit to this issue’s fork.

quietone’s picture

quietone commented
Issue summary: View changes

I created an issue for #1 as a sibling #3618772: Move toolbar_alter hook implementations to the toolbar module.

virag.jain’s picture

virag.jain commented
Assigned: Unassigned » virag.jain

Working on it. As @quietone has already addressed #1, so I’ll pick up the remaining work.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

We can keep announcements in #3618772: Move toolbar_alter hook implementations to the toolbar module I think, but then it needs to cover the library and the PHP class :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Move supporting PHP, CSS and JS code for core module integrations with Toolbar to Toolbar » Move supporting PHP, CSS and JS code for user, contextual and workspaces Toolbar integrations to Toolbar
Issue summary: View changes

Removed the announcements feed item from the summary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
Issue tags: +Needs issue summary update

I am adding back the items in the announcements module except the moving of the toolbar_alter hook. That is one piece of this that can be scoped separately.

This needs an issue summary update to state how things are being moved around.

quietone’s picture

quietone commented
Assigned: virag.jain » Unassigned

@virag.jain, For Drupal core, it is preferred that contributors add a comment that they are working on an issue instead of assigning it to themselves. See Assigning ownership of a Drupal core issue. Since you have commented, thank you, I am unassinging this issue.

quietone’s picture

quietone commented
Title: Move supporting PHP, CSS and JS code for user, contextual and workspaces Toolbar integrations to Toolbar » Toolbar uses in Announcement, User, Contextual and Workspaces

Let's have a concise title

virag.jain’s picture

virag.jain commented

Thanks @quietone for the correction. I’ll keep it in mind for future issues.

Should I work on the User, Contextual, or Workspaces pieces? Just checking so we don’t duplicate effort.

quietone’s picture

quietone commented

@virag.jain, anything you like!

virag.jain opened merge request !16859

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Are the icons used anywhere outside of Toolbar? If not we need to move them to Toolbar itself and remove them from core. Looking at the CSS path references we might need to do that anyway, as we don't know which directory level the contrib version of the module will be installed at (and hence how many ../ we need)

quietone’s picture

quietone commented

@virag.jain, Drupal core relies on the issue summary here on the issue and that is where reviewers expect to find the overview of what is happening on any issue. Can you move the summary of changes to the 'proposed resolution' section. Thanks.

quietone’s picture

quietone commented
Issue summary: View changes
Status: Active » Needs work

@virag.jain, thanks for your work.

Still to do are Announcements, Contextual and Workspaces

catch’s picture

catch
he/him
Primary language English
commented
Title: Toolbar uses in Announcement, User, Contextual and Workspaces » Toolbar uses in Announcement, User, Contextual

Split workspaces to its own issue #3620812: Move toolbar support from Workspaces to Toolbar for easier review/commit hopefully.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes

Removed workspaces from issue summary following the breaking that out to #3620812: Move toolbar support from Workspaces to Toolbar. Looking at the rest of the items myself now.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Toolbar uses in Announcement, User, Contextual » Move Toolbar uses in Announcement, User, Contextual to Toolbar
Status: Needs work » Needs review

Added coverage for contextual and announcements. (LLM was involved).

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
Issue tags: -Needs issue summary update
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
quietone’s picture

quietone commented

@gábor hojtsy, thanks.

I removed toolbar from comments in two files.

And it is still used in a comment in a twig file.

$ git grep toolbar | grep core/modules/announcements_feed/
core/modules/announcements_feed/templates/announcements-feed.html.twig: * This template will get rendered when the user clicks the announcement button in the toolbar.
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

@quietone: good find, that template has nothing to do with how its triggered, so it should not say how it is triggered :) I updated it. Does this look complete now?

quietone’s picture

quietone commented

After applying the diff a simple grep for 'toolbar' in the modules changed in the MR returned no instances.

$ git grep toolbar | grep core/modules/announcements_feed;git grep toolbar | grep core/modules/big_pipe;git grep toolbar | grep core/modules/contextual;git grep toolbar | grep core/modules/user
$

So we have all the instances. However, I lack the frontend knowledge 'to do a proper review on this, so leaving at needs review.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Needs work

Left a review

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs work » Needs review

Back at needs review.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3618746-nr-bot_yoeb7wrn.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

quietone’s picture

quietone commented
Status: Needs work » Needs review
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Let's do it.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3618746-nr-bot_cqpfcdco.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

quietone’s picture

quietone commented

Rebased, conflicts due to #3620812: Move toolbar support from Workspaces to Toolbar

And somehow I messed up the rebase and there are not failing tests I am not able to fix. Sorry about that.

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

godotislate’s picture

godotislate
he/him
commented

OK, compared diffs for the last good MR tag: previous/3618746-move-supporting-php/2026-09-10 with the current MR branch and looks like the addition of the toolbar.contextual got left out. Failing tests pass locally with the change restored, so pushing.

godotislate’s picture

godotislate
he/him
commented
Status: Needs work » Needs review

Test pass now. Putting back to NR so that someone can double check the rebase.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

https://git.drupalcode.org/project/drupal/-/merge_requests/16859/diffs?d...

I think the views router rebuild change crept in from another issue.

godotislate’s picture

godotislate
he/him
commented

@catch are you maybe looking at an old diff? https://git.drupalcode.org/project/drupal/-/merge_requests/16859/diffs Doesn't have the Views changes.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

Not sure how I managed that, but yes I can't see the views changes in there now.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC in fact, the rest looks fine.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs review

Added some questions about SVG files.

quietone’s picture

quietone commented

First, thanks for cleaning up my mistakes!

I left comments in the MR to answer the queried by @longwave. The short answer is I think this MR is fine as is and a followup can be made to check all the URLs used in the css files.

I am posting here because comments made on an MR are not being posted here as well.

quietone’s picture

quietone commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs followup

Better yet, setting back to RTBC

  • catch committed 2d9bc63c on main 
    task: #3618746 Move Toolbar uses in Announcement, User, Contextual to...
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: -Needs followup

Committed/pushed to main, thanks! Moving to 11.x for backport.

I opened #3622941: Copy icons into toolbar for the icons.

quietone opened merge request !17116

quietone’s picture

quietone commented
Status: Patch (to be ported) » Needs review

11.x MR ready for review

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems like a good backport.

  • catch committed c647ff03 on 11.x 
    task: #3618746 Move Toolbar uses in Announcement, User, Contextual to...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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