Symmetrically translatable config-defined component trees, STEP 2: change config schema type for `inputs` to allow translating via core's Config Translation UI for ContentTemplates & PageRegions

See

    // Verify the translation was stored, in the expected efficient manner: only
    // the translatable subset, instead of duplicating everything.
    // @see \Drupal\language\Config\LanguageConfigFactoryOverride::onConfigSave()
    // @see \Drupal\Core\Config\ConfigFactoryOverrideBase::filterOverride()
    $override = $language_manager->getLanguageConfigOverride('nl', $template->getConfigDependencyName());
    self::assertFalse($override->isNew());
    self::assertSame([
      'component_tree' => [
        '435d1d20-a697-4d36-9892-9d61c825c99c' => [
          'inputs' => [
            'text' => 'Aangedreven door Drupal Canvas',
            // @todo This MUST disappear, too! https://www.drupal.org/project/canvas/issues/3582478 will make it so, because then the config schema will no longer mark the entirety of `inputs` as translatable, but individual key-value pairs.
            'href' => 'https://drupal.org/project/canvas',
          ],
        ],
      ],
    ], $override->getRawData());

in the sibling MR.

Goal: fixing #2.

Working PoC that achieves it: https://git.drupalcode.org/project/canvas/-/merge_requests/868/diffs?com...

I think this is enough for @tedbow to continue experimenting if he has the time on Monday. Otherwise, I'll happily continue on Tuesday 🤓

IS updated now that the overall approach seems to be viable. Still some work left to get to green though.

Discussed with @tedbow; he's +1 given the dynamic schema bit here is not introducing significant complexity — he expected worse :)

To be continued tomorrow, but well on track now. 87 failures/158 errors (this AM) → 7 failures/13 errors (now). 🚀

Extracted #3583494: CanvasTestSetup and ContentTemplateOnDependencyRemovalTest should not rely on magic and specify component versions for its test component tree already as blocking technical debt.

Down to the last failure; AFAICT that's solely due to missing update path + test coverage.

Update IS to to reflect change in direction to make auto-save functionality work with zero changes.

Point 4 in the issue summary is now also working!

In doing so, finally created #3583854: [upstream] Validate LanguageConfigOverrides targeting Canvas config entities, because the risks for Canvas and oversights upstream in core have now become sufficiently clear.

And now also solved point 3: https://git.drupalcode.org/project/canvas/-/merge_requests/868/diffs?com...

Next up: ADR.

Made sure that this does NOT corner us into symmetrical translations only — we will want to add asymmetrical translation support for config entities in the future. Proven: #3583945-2: [later phase] Support asymmetric translation of config-defined component trees.

Green!

And manually confirmed my hypothesis in the IS: 💡 This should also make the "JSON-blobs-in-config-YAML" disappear.

I think that I was wrong in in #8: no update path is needed for this: this merely refines config schema, reads existing config entities just fine, and just changes how it's represented as YAML at export time:

• no more JSON blobs
• reordered according to config schema

That requires no update path, but #3582464: [PP-2] Symmetrically translatable config-defined component trees, STEP 3: adjust sequence keys to avoid translations breaking when component instances are moved will. So let's land this simple subset first, that does NOT require #3582464's significant changes which will require an update path 🤞

Per #14, I had AI extract the subset of MR868 that does not bring over all of #3582464: [PP-2] Symmetrically translatable config-defined component trees, STEP 3: adjust sequence keys to avoid translations breaking when component instances are moved. See MR 898.

(The AI's work is the new oldest commit. And then I fixed the mistakes it made: the second commit. After that, all commits have been replayed.)

From +1554,−473to +851, −36, and NOT needing an update path. 🚀

🤖 The "lift and replay" MR 898 was AI-accelerated.

Part of the diff reduction was thanks to #3583494: CanvasTestSetup and ContentTemplateOnDependencyRemovalTest should not rely on magic and specify component versions for its test component tree (+33, −15).

Opened #3584143: Incorrect property paths due to bugs in `BlockComponent::validateComponentInput()` + `ConstraintPropertyPathTranslatorTrait` for another out-of-scope piece of technical debt that is blocking this.

#19: That's fine — just this change alone won't make config-defined component trees properly translatable anyway. We need #3582464: [PP-2] Symmetrically translatable config-defined component trees, STEP 3: adjust sequence keys to avoid translations breaking when component instances are moved too. Which is why the test coverage (just updated the IS to point to those) specifically points to #3582464 in a number of places :)

🆕 #3584178: [later phase] Add alter hook for marking additional SDC/code component prop shapes AND/OR specific props as translatable

Merged: #3584143: Incorrect property paths due to bugs in `BlockComponent::validateComponentInput()` + `ConstraintPropertyPathTranslatorTrait` — rebasing !898.

ADR up: https://git.drupalcode.org/project/canvas/-/merge_requests/901. #10, because #9 is not yet claimed but should be: #3514033-26: Document high-level architecture decisions for extensions API 😅

ADR MR needs review. I'm not 100% happy with it, but there's so many things to consider that writing this overwhelming — especially in balancing that with the need for not going into full detail in an ADR 😬

The actual MR still needs to be rebased, one test failure fixed.

Rebased !898 on top of #3584143: Incorrect property paths due to bugs in `BlockComponent::validateComponentInput()` + `ConstraintPropertyPathTranslatorTrait`. 5 fewer files changed, and from +851, −36 to +827,−22.

DO NOT YET review !898, only the ADR MR !901 🙏 To simplify reviewing, look at the rendered result: https://git.drupalcode.org/issue/canvas-3582478/-/blob/3582478-ADR-confi...

@lauriii and @phenaproxima provided valuable feedback on the ADR MR. Addressed that.

Shifts:

1. For SDCs and code components, this now reuses the same field widget infrastructure when translating, which is why @lauriii's request to also allow translating type: string, format: uri-shaped props became trivial to implement 🥳
2. This in turn made #3584178: [later phase] Add alter hook for marking additional SDC/code component prop shapes AND/OR specific props as translatable trivial!
3. Descoped TMGMT. TMGMT's philosophy ("extract strings") is kind of at odds with how config translation works, is definitely very hard to reconcile with how Canvas uses field types to populate SDCs+code components and is kind of in conflict with what @lauriii just requested: it specifically disallows translating URIs on link fields. Details on MR. @lauriii approved descoping TMGMT for now on the ADR MR.
4. I don't yet see how we would generate a translation form for block component instances 😬🫠 If we'd use TMGMT, that'd be simpler.
   EDIT: see #27, where I second guess we even need this 😇

My current hunch on how to best incrementally ship valuable symmetric translation features without cornering us:

1. symmetric translations for config-defined component trees (templates, regions), using the Config Translation UI → first this issue, then #3582464: [PP-2] Symmetrically translatable config-defined component trees, STEP 3: adjust sequence keys to avoid translations breaking when component instances are moved. Do not advertise loudly to the public.
2. figure out how to use TMGMT instead (which reuses form_element_class of the Config Translation UI when translating config — so step 1 is not wasted). See #25.3's "details on MR" link how that could possibly work. This would allow translating SDC+code+block for config entities, and SDC+code for content entities. Advertise loudly.
3. Make block component instances in content entities translatable via TMGMT.

@lauriii is +1 to this sequence if it indeed doesn't corner us.

@tedbow, WDYT? You've been investigating #3583684: [PP-1] Symmetric translations for content entities should allow only translatable properties in inputs to be overridden so you'll be able to shine a light on this from the content entity angle? 🤞

Now that there is a plan to actually review, I'm trying to assess whether truly all these things are necessary.

Doing so, I realized that symmetrical translations for block component instances may be almost pointless because:

1. pretty much only the (currently unused+hidden in Canvas!) label setting for every block plugin will be translatable: https://git.drupalcode.org/project/canvas/-/merge_requests/901/diffs#not...

   Includes screenshots to illustrate the common case.

2. Block plugins are all about executing logic, and offering the content author some useful dials and knobs to control that logic. N results, the Zth menu level, a TRUE/FALSE decision … those are the typical settings for block plugins.

WDYT @lauriii? This would eliminate #25.4 and #26.3 and simplify #26.1+2.

Issue summary updated per #25–#28, to reflect that:

• config translation now fully works, is tested, and supports more than only plain+HTML strings.
• TMGMT is out of scope here

Back to @tedbow per #23.

I think there's 3 things left here:

1. Refactor ComponentSourceInterface::getExplicitInputDefinitionsAsConfigSchemaMapping() (point 3 in the proposed resolution) to something less arcane per @phenaproxima's feedback. ← in progress
2. One missing goal: added as point 6 to the proposed resolution.
3. Make the one failing update path test pass again.

ADR MR updated:

• #29's TMGMT is out of scope here → removed the claimed benefit from the ADR too.
• @lauriii responded negatively to my proposal in #27; updated ADR accordingly.
• Documented the rationale behind the CanvasStaticPropSourceFieldWidget form_element_class

Fixed #30.1 in the 8 commits I just pushed, all dated April 17 — basically: "introduce ComponentInstanceInputsConfigSchemaGeneratorInterface, similar to the updater and discovery handlers component source plugins specify".

I used AI to accelerate it. Docs have been updated.

ADR MR has been updated too.

Monday:

1. Make it possible for a translation to populate an optional translatable input that is not populated in the default translation — @lauriii confirmed we want that
2. #30.2
3. #30.3

#30.2/item 6 in the proposed resolution is now done!

Implemented that as far as possible, but a single class to power both ComponentTreeItem::propertyDefinitions([inputs]) and canvas.component_tree_node:inputs turned out to be impossible 😬 The attached patch gets as far as possible, and it mostly works, but it eventually fails on \Drupal\canvas\Plugin\DataType\ComponentInputs::getValue() returning a JSON string rather than a mapping/an associative array. That in turn is necessary only because of a missing core feature that's been around for >12 years 😅

1. Test coverage #30.2/item 6 in the proposed resolution is now done: tests now prove that regardless of where a component instance is defined (in a content or config entity), for the same stored inputs, the same set of translatable input keys is returned. Also added explicit test coverage
2. ADR MR updated. I think this is ready to go now.

Proposed next steps

1. Land this issue's ADR MR. It got positive feedback from @lauriii and @phenaproxima >1 week ago. @phenaproxima's comments have been addressed. The overall ADR is much more thorough now (and lists fewer implementation details that previously made this ADR less timeless).
2. 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%.
3. Make this issue's implementation MR RTBC-worthy: #30.3 + self-review + clean up this MR (the many shifts along the way (plus the use of LLMs) have left parts of this MR in a confusing state).
4. Then land this issue's implementation MR, which thanks to the blocking MR can focus on "make config translation work end-to-end".

ADR MR in, next up: #34.2.

For #34.2: #3586342: Symmetrically translatable component trees, STEP 1: introduce `ComponentInstanceInputsConfigSchemaGeneratorInterface`.

Blocker in!

EDIT: after merging in what landed upstream, this is now down to 29 files, +1329, −28, a 35% reduction 👍

Explicit update path + tests added: ConfigComponentInputsMustBeArraysUpdateTest. Tests 2 scenarios:

1. when >=1 config-defined component instance's inputs still is a JSON blob (typical scenario)
2. when it already is an array (atypical, but possible when e.g. using recipes, possible due to type: ignore not applying any transformation at saving time).

Simplify IS: 2 pieces of proposed test coverage in the IS actually were lifted out of here and landed as part of #3586342: Symmetrically translatable component trees, STEP 1: introduce `ComponentInstanceInputsConfigSchemaGeneratorInterface`.

• Issue summary simplified further. #3586216: Follow-up for #3545236: move triplicated logic into `ComponentTreeConfigEntityBase` to simplify future update paths targeting component trees landed, making this MR again a bit simpler.
• Determined root cause for the 11.3-only failure: CKEditor 5 started relying on the host entity for the field item list objects it received 😵 — will work around it here and leave the full fix for #3571785-2: Refactor away the confusing "dangling component tree" concept.
• Made CKEditor 5 work in Config Translation.

Next up: clean-up + make 11.3 pass.

Updating issue title to reflect this also makes translating PageRegions possible.
Everything is passing except a single one on 11.3, will fix tomorrow, I already know what's going on. Will fix tomorrow.
Tests exist, removing tag.

I'll clean up tomorrow, but I think this is ready for final review by somebody who's been involved with this whole symmetrical translation process: @tedbow.

Fix screenshot.

Now fully ready for final review by @tedbow 🤞 Should pass on 11.3, and clean-up/tidying up of tests has been applied.

• IS finalized.
• CR ready: https://www.drupal.org/node/3586291
• Final diff: 35 files, +1702, -33. Of this:
  • 416 is for /src/ConfigTranslation to make the Config Translation UI usable but ugly.
  • 228 is for update path + test coverage
  • 612 is for testing the full config translation life cycle + rendering of config translations + proving the Config Translation UI works

Happy to walk you through this, @tedbow.