Overview
#3582478 already landed an ADR, #10. This issue is for implementing the first decision of that: Allow any
. It also implements decisions 3, 4, 5 and 6.ComponentSource plugin to define a ComponentInstanceInputsConfigSchemaGeneratorInterface
This is split off from #3582478's implementation MR
- Extract a new blocking issue/MR from this one, specifically focused on the "provide translatability metadata" aspect. So:
ComponentInstanceInputsConfigSchemaGeneratorInterface+ComponentInputs::getTranslatableInputKeys()+\Drupal\Tests\canvas\Kernel\Plugin\Canvas\ComponentSource\ComponentSourceTestBase::testGetTranslatableInputKeys.
This should cut the size of this issue's MR (now sizable, at ~2K LoC!) MR by 30–50%.
This blocks both:
- symmetrical translations of content-defined component trees' inputs: #3583684: [PP-1] Symmetric translations for content entities should allow only translatable properties in inputs to be overridden
- symmetrical translations of config-defined component trees' inputs: #3582478: Symmetrically translatable config-defined component trees, STEP 2: change config schema type for `inputs` to allow translating via core's Config Translation UI
Proposed resolution
- ✅ Provide an explicit API for this: introduce
\Drupal\canvas\Attribute\ComponentSource::__construct(inputs_config_schema_generator)which requires aComponentInstanceInputsConfigSchemaGeneratorInterface.
(Making the pre-existingComponentSourceBase::getExplicitInputDefinitions()public is insufficient; we'd need to change its return values, which in turn would result in different component version hashes. That's avoidable disruption. This is still not ideal, but that is true for many of the methods on that interface. The point is to first grow it organically to meet all needs, and then distill a sensible API/interface from it. We have #3520484: [META] Production-ready ComponentSource plugins for that. But this was implemented in a way that follows the pre-existing pattern towards a public API:discovery,updaterand nowinputs_config_schema_generator.) - ✅ Make it available in simplified form via
ComponentTreeItem'sinputsproperty. It could be retrieved from$component_tree_item->get('inputs')->getTranslatableInputKeys()but that would be more expensive: config schema construction would ideally not be instantiating field objects like that. Instead, make apublic static ComponentInputs::resolveConfigSchemaMapping()available that is used by both classes.
The result: both config translation (#3582478) and content translation (#3583684) can use the same central metadata to determine which of the key-value pairs in inputs for a component instance are actually translatable.
Test coverage
- ✅ Kernel test coverage for all implementations of the new
ComponentInstanceInputsConfigSchemaGeneratorInterface: ensured for all component sources thanks to\Drupal\Tests\canvas\Kernel\Plugin\Canvas\ComponentSource\ComponentSourceTestBase::testGetTranslatableInputKeys()'s reliance on an abstract data provider - ✅ Kernel test coverage for
\Drupal\canvas\Plugin\DataType\ComponentInputs::getTranslatableInputKeys()'s edge case handling that does not rely on that new interface:ComponentTreeItemListTest::testGetTranslatableInputKeysSourceAgnosticEdgeCases()
User interface changes
None.
Issue fork canvas-3586342
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
wim leersComment #4
wim leersReady for final review!
Total:
of which tests:
Comment #5
tedbow!962 looks good! I had reviewed most of this in !898. Asked a couple question about just for my understanding nothing blocking
Comment #6
wim leersSelf-review, fixed nits, added TODOs for #3586490: Introduce `ComponentSourceInterface::getLabelForExplicitInput()` + #3584178: [later phase] Add alter hook for marking additional SDC/code component prop shapes AND/OR specific props as translatable and addressed the sole remaining TODO:
type: arraysupport, which turned out to be trivial.@tedbow already approved last night; no significant changes since then, so going ahead and merging 👍
Thanks, @tedbow!
Comment #8
wim leersComment #10
wim leers