Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Many plugins, such as Blocks, need to translate strings when building a render array. Currently to get proper DI you have to e.g. use the ContainerFactoryPluginInterface and inject a translation object for every class.
Proposed resolution
Create a new class that is in the Core namespace that extends or replaces PluginBase and has available a $this->t()
Remaining tasks
TODO
User interface changes
None
API changes
Core plugins (e.g. BlockBase) that commonly need to translate strings can use the Core PluginBase
Related Issues
#2018411: Figure out a nice DX when working with injected translation
Follow Up Issues
#2170471: ContextAwarePluginBase compromised after commit of Core PluginBase
Original report by @pwolanin
Comment | File | Size | Author |
---|---|---|---|
#12 | plugin-2087231-10.patch | 12.2 KB | tim.plunkett |
#12 | interdiff.txt | 9.18 KB | tim.plunkett |
#4 | plugin-2087231-3.patch | 3.02 KB | tim.plunkett |
#4 | interdiff.txt | 2.2 KB | tim.plunkett |
#2 | 2087231-2.patch | 4.25 KB | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedfix title
Comment #2
pwolanin CreditAttribution: pwolanin commentedGiven the truly minimal code in the Component class, it seemed cleaner to not inherit.
Here's a 1st stab, which copies the methods form FromBase and converts BlockBase.
Comment #4
tim.plunkettDiscussed in IRC, let's use the base class.
Comment #6
tim.plunkett#4: plugin-2087231-3.patch queued for re-testing.
Comment #7
ParisLiakos CreditAttribution: ParisLiakos commentedwe need one issue to do the same for DerivativeBase
i am rtbcing this, assuming we dont need to update all block plugins that use t() here
Comment #8
pwolanin CreditAttribution: pwolanin commented#4: plugin-2087231-3.patch queued for re-testing.
Comment #9
webchickThe one thing I took issue with is this and asked if we should instead rename that class to ComponentPluginBase, because it can be very confusing for there to be two classes called "PluginBase" (yes,as I realize namespaces differentiate them but that isn't always reflected in code editors).
Tim and James noted that if we were going to rename anything, it would make more sense to rename the core implementation (e.g. DrupalPluginBase or CorePluginBase) since the plugin system in Components is intended to be re-used outside of Drupal. Since we don't follow that pattern anywhere else (e.g. DrupalFormBase, which is ick), it doesn't make sense to introduce it here. If all of the implementations in core are deriving from the Core version, copy/paste developers will get the right one anyway.
However, this is not currently happening, because we only have one of these hunks in the patch. So let's make sure everything is switched over to Core's plugin base in this patch, so we don't leave core in an inconsistent state.
Comment #10
neclimdulPersonally I don't think we should rename it. This is the only class that will interact with the two classes so it shouldn't matter. As stated if we where to rename a class the Core would be the one I'd vote for.
Comment #11
webchickSorry. I totally agree with not renaming it. That's a bad pattern to go down.
But I do think we need to make that same change we're making in BlockBase.php to all of the existing PluginBase-derived classes so that core is consistent with itself.
Comment #12
tim.plunkettThis should be all of them.
Comment #13
tim.plunkettRetitled, we have no other obvious helpers to add at the moment, and getting all of the base plugins classes switched over will let us add anything more easily in the future.
Comment #14
larowlanNeat!
Comment #15
webchickCommitted and pushed to 8.x. Thanks!
Will need a change notice.
Comment #16
yched CreditAttribution: yched commentedGreat patch !
Wouldn't url() / l() be helpful too ?
Comment #17
pwolanin CreditAttribution: pwolanin commented@yched - I'm less sure about that. Almost all plugins have some text many fewer generate link (though certainly some).
Comment #18
tim.plunkettOpened #2112575: Add a DerivativeBase in the Core namespace with t() as a helper method for #7
Comment #19
dawehnerhttps://drupal.org/node/2116303
Comment #19.0
dawehnerUpdated issue summary.
Comment #20
jgSnell CreditAttribution: jgSnell commentedWorking on the change notice now...
Comment #21
jgSnell CreditAttribution: jgSnell commentedChange notice is here: https://drupal.org/node/2116303
Comment #22
EclipseGc CreditAttribution: EclipseGc commented