Stolexiy/styleguide

Use dependency injection instead of \Drupal::... in OO code

Closed this issue · 8 comments

Sutharsan commented
Use dependency injection instead of \Drupal::... in OO code
Stolexiy commented

Added dependency injections. Can't make it work only in the StyleguideLocalTasks Plugin (https://github.com/Shdulin/styleguide/blob/8.x-1.x/src/Plugin/Derivative/StyleguideLocalTasks.php). As I understand it because this plugin is always called with single param and there are no ways to add more params to the constructor(), but I may be wrong.

Sutharsan commented

Two examples I found of plugins that uses DI: Drupal\tour\Plugin\tour\tip\TipPluginText, Drupal\image\ImageEffectBase

Stolexiy commented

I tried to implement it this way first, but it doesn't work, since this Plugin isn't usual. \Drupal\Core\Plugin\Discovery\ContainerDeriverInterface need to be implemented in this case.
All \Drupal::... were converted to use dependency injection.

Sutharsan commented

I will have a look. But it will probably take a few days until I have time.

Sutharsan commented

I 've got it working. My first step was to hard code the Local tasks (below). And then rebuild the data dynamically in StyleguideLocalTasks. I will make a PR later today.

# admin/appearance/styleguide
styleguide.page:
  route_name: styleguide.page
  title: 'Styleguide'
  base_route: system.themes_page
  weight: 90

# admin/appearance/styleguide/bartik
styleguide.bartik:
  route_name: styleguide.page
  title: 'Bartik'
  parent_id: styleguide.page
  weight: 0

#  admin/appearance/styleguide/seven
styleguide.seven:
  route_name: styleguide.seven
  title: 'Seven'
  parent_id: styleguide.page
  weight: 1
Sutharsan commented

PR: #8

andypost commented

Looks that could be closed, now issue #23

Sutharsan commented

Indeed