Problem/Motivation

In #2729597: [meta] Replace \Drupal with injected services where appropriate in core we generally decided to convert usages of \Drupal methods to inversion-of-control injection, by method.

This issue converts usages of \Drupal::theme() to injection.

Believe it or not, \Drupal::theme() is only used in three non-test classes, and the rest is in .inc files where it's appropriate.

Those three classes are HtmlRenderer ViewExecutable and ThemeSettingsForm.

HtmlRenderer and ViewExecutable are services, so in an ideal world, we would inject theme.manager into their constructor methods, and amend the service definitions.

We could do that for HtmlRenderer if it didn't lead to circular references. #2393329: Replace all drupal_render calls with the service, and inject it, if possible. is an issue where you can start, and branch off and find all the problems.

ViewsExecutable has its own thorny issue on injection: #2102293: Inject the view executable factory where possible Mentioned here because ViewsExecutable is part of the factory pattern that would inject theme.manager. We also see a desire to move away from dependency on the theme manager; #2322623: Try to remove the coupling of a view to the theme system

That leaves us with:

Proposed resolution

ThemeSettingsForm already implements ContainerInjectionInterface, so we adjust the create/construct pattern accordingly.

Replace occurrences of \Drupal::service('theme.manager') with \Drupal::theme().

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Issue summary: View changes
Status: Active » Needs review
Related issues: +#2102293: Inject the view executable factory where possible
FileSize
4.79 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2780397_2.patch, failed testing.

Mile23’s picture

Status: Needs work » Needs review
FileSize
4.78 KB

Ewps.

Patch applies to 8.2.x as well.

martin107’s picture

Status: Needs review » Reviewed & tested by the community

The patch applies.

After a visual scan of the patch -- everything looks in orders ...and moves D8 in a positive direction.

  • catch committed 2b9d7ce on 8.3.x
    Issue #2780397 by Mile23: Replace non-test usages of \Drupal::theme()...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.3.x, thanks!

Status: Fixed » Closed (fixed)

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