Problem/Motivation
The LayoutBuilderTaskDeriver hardcodes local tasks for sectionStorage plugins.
Proposed resolution
The LayoutBuilderTaskDeriver should let the plugins build their own local tasks.
Remaining tasks
N/A
User interface changes
N/A
API changes
API addition: section storage plugins are now directly consulted when building local tasks (similar to how it is consulted for route building).
Data model changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 2995071-layout_localtask-15.patch | 13.41 KB | tim.plunkett |
Comments
Comment #2
sugaroverflow commentedWorked on this patch with @tim.plunkett at mwds
Comment #3
sugaroverflow commentedComment #4
tim.plunkettI think this is unused.
Nit: Keep this on one line, please
It's so great to remove this duplicate code!
Comment #5
sugaroverflow commentedThank you for reviewing!
Uploading a new patch and interdiff. Running phpcs also removed two unused use statements from the deriver.
Comment #6
tim.plunkettYay! Thanks.
This is a great refactor that will allow other implementations of LB to participate in the local tasks building.
Comment #7
tim.plunkettComment #8
damienmckennaIs this something that could be backported to 8.6, or is it sufficient of an internal API change that it's out of consideration?
Comment #9
tim.plunkettI see no reason this couldn't be backported, but that's up to the committers.
Comment #10
larowlanSo I think this is an API change, as we don't have a base class so can't apply the 1:1 rule.
So I think we need a BC shim, which would be a new interface with this method and an instanceof check with a fall back.
Comment #11
tim.plunkettAdd an empty implementation to the base class, Drupal\layout_builder\Plugin\SectionStorage\SectionStorageBase.
Comment #12
larowlanPerfect
Comment #13
samuel.mortensonLGTM.
Comment #14
larowlanThanks, can we get a change record here?
Comment #15
tim.plunkettWhile trying to write the CR, I had a realization that while we absolutely need to provide Local Tasks in the current iteration of Layout Builder, we have no explicit need for them in the final designs. We're only using local tasks until #2936655: Layout Builder should use the toolbar modes system once it exists. is resolved.
So even though adding to the interface is supported by the BC policy, we don't want to.
CR is at https://www.drupal.org/node/2999418
Comment #16
jibranThis refactoring makes more sense IMO.
Comment #17
larowlanAdding review credit
Comment #19
larowlanCommitted 58b776b and pushed to 8.7.x. Thanks!
Published change record
Comment #21
webchickBackported to 8.6.x also.
Comment #22
tim.plunkettRetroactively tagging