Problem/Motivation

The ToolbarHandler class looks a lot like a service, as it uses DI, but it's not declared as one, and instead, has to be instantiated with the ClassResolver.

This is confusing DX, as there's more code than needs to be.

It's also inefficcient, as if it were a service, the container would keep the instantiated object. Currently, in one request, the class is instantiated three times AFAICT.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 Capture d’écran, le 2024-06-03 à 14.23.58.png268.77 KBsamsylve
#7 environment_indicator_issues_3426089.png139.98 KBrubberg

Issue fork environment_indicator-3426089

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

joachim created an issue. See original summary.

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

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

Denist3r opened merge request !41

denist3r’s picture

denist3r commented
Status: Active » Needs review

Implemented ToolbarHandler as a service. Please check.

devkinetic’s picture

devkinetic commented

This looks good to me, a simple but effective change.

rubberg’s picture

rubberg commented
StatusFileSize
new environment_indicator_issues_3426089.png139.98 KB

I checked the commit and that is exactly what the reporter asked for and what I was about to do.

Applied merge request !41 as a patch to branch 4.X.

Tested on a Drupal 9.5.7 and php 8.1.28 install and works fine. (see caption)

Thank you @Denist3r for your work!

samsylve’s picture

samsylve commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Capture d’écran, le 2024-06-03 à 14.23.58.png268.77 KB

Tested on a Drupal 10.2.6 and php 8.1.27 install and works fine. (see caption)

samsylve’s picture

samsylve commented

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

isholgueras’s picture

isholgueras commented
Status: Reviewed & tested by the community » Fixed

I've included new changes from 4.x, there was a conflict, and also formatted the service argument as a list. Thanks!

Status: Fixed » Closed (fixed)

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