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
Comment | File | Size | Author |
---|---|---|---|
#4 | 2780397_4.patch | 4.78 KB | Mile23 |
#2 | 2780397_2.patch | 4.79 KB | Mile23 |
Comments
Comment #2
Mile23Comment #4
Mile23Ewps.
Patch applies to 8.2.x as well.
Comment #5
martin107 CreditAttribution: martin107 as a volunteer commentedThe patch applies.
After a visual scan of the patch -- everything looks in orders ...and moves D8 in a positive direction.
Comment #7
catchCommitted/pushed to 8.3.x, thanks!