d8-contrib-modules / addthis

Port AddThis to D8
GNU General Public License v2.0
3 stars 13 forks source link

Fix issue with global $language returning null #27

Closed jenter closed 9 years ago

jenter commented 9 years ago

TODO in modules/addthis/src/Services/AddThisScriptManager.php private function getJsAddThisConfig() { }

Per @damontgomery's comment on our last PR:

So, the recommended way to access the language setting is to get a language handler service using dependency injection. The use is actually shown here, https://www.drupal.org/node/2133171

I think you'd actually want to inject Drupal\Core\Language\LanguageManager using language_manager as the service name or @language_manager as the argument to this service. Then you can assign this to $this->languageManager and get the language with the method $this->languageManager->getCurrentLanguage()

nerdstein commented 9 years ago

@damontgomery can you take this?

nerdstein commented 9 years ago

If so, please assign self

nerdstein commented 9 years ago

Relates to PR #26

damontgomery commented 9 years ago

@jenter & @nerdstein, I can help with this. I went over some of the ideas with Jason Enter on Friday, does it make sense for me to give this a pass?

The main update I suggested was to add dependency injection to the service being used instead of making static / global calls to things.

I can tackle this on Monday and we can review the changes.

jenter commented 9 years ago

@damontgomery @doylejd we can discuss on Monday

doylejd commented 9 years ago

Closing this as @damontgomery handled it in his PR.