- Issue created by @wim leers
- Assigned to lauriii
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Asked @lauriii to confirm he agrees with the proposed resolution, especially with an eye towards ๐ [PP-2] Add ComponentAuditabilityTest Active .
- Issue was unassigned.
- ๐ซ๐ฎFinland lauriii Finland
I responded on ๐ [PP-2] Add ComponentAuditabilityTest Active . I'm concerned that storing this information in the
Component
config impacts DX negatively. We need to consider how are components authored, and how are they edited during their lifecycle, and how that overall experience looks like. Today, if components or templates are being changed, the changes are reflected immediately to the site. We should strive that same experience. We need to remove as many barriers from the XB from authors; the goal is for it to feel just as if you're editing a regular template or component. - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Today, if components or templates are being changed, the changes are reflected immediately to the site.
This is not true:
- To have template changes reflect immediately (including SDC template changes), you'd need to enable at
/admin/config/development/settings
- To have component changes (aka component metadata changes) reflect โฆ there's no way to do that, not even
https://www.drupal.org/project/cl_devel โ
does that. The only way to achieve that is to manually clear the
discovery
cache bin'scomponent_plugins
cache item! And since there's no cache tags associated with that, the only way to achieve that is to go to/admin/config/development/performance
and click (more efficiently through the CLI:drush cc discovery
).
Actually โฆ the way it's cached is so aggressive that a major core bug for it was just fixed yesterday: ๐ Single directory component CSS asset library not picked up in admin theme immediately after module install without cache clear Active . ๐
IOW: to actually achieve what you describe, we'd need an , which re-discovers all SDC metadata on every request. We could and should do that! Opened issue for that: ๐ SDC *.component.yml metadata is cached aggressively, gets in the way of component development Active .
And โฆ when that mode is on โฆ we'd also just automatically update the
Component
config entity! ๐คResult:
- the exact DX you describe in #8
- the auditability described in ๐ [PP-2] Add ComponentAuditabilityTest Active
- To have template changes reflect immediately (including SDC template changes), you'd need to enable at
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
This blocks #3464042, and at #3464042-4: [PP-2] Add ComponentAuditabilityTest โ you can find a detailed write-up that justifies both this issue as well as that one, based on XB's product requirements.
- Assigned to f.mazeikis
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings Fixed is in.
@f.mazeikis, see #3461499-24: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings โ for some additional context, but here's the most relevant bit:
In the MR, there's 4
@todo
s pointing to ๐ Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria Fixed , which is where the automatic fallback logic that this added can be removed. "Automatic fallback logic" is a nice alternative phrasing for "magic ๐ง", and we should avoid it. Removing that magic is an order of magnitude simpler once these component config entities are fully auto-generated. So, deferring that part to that issue ๐ - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
AFAICT this issue would likely have independently surfaced ๐ Follow-up for #3461499: transform at-rest field storage settings using FieldItemInterface::settingsFromConfigData() RTBC , especially if ๐ Introduce an example set of representative SDC components; transition from "component list" to "component tree" Fixed lands before this.
#3465981 is only a soft blocker to this though, because there's plenty of other pieces to be done in this issue.
- ๐ฌ๐งUnited Kingdom f.mazeikis Brighton
f.mazeikis โ made their first commit to this issueโs fork.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ Follow-up for #3461422: display SDC prop's human-readable name (`title`), not its machine name Needs review surfaced one additional requirement.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Discussed in detail with @f.mazeikis. He discovered oversights in
(Candidate)StorablePropShape
+ the config schema while working on this MR. The solution is evident: afieldInstanceSettings
sibling tofieldStorageSettings
. He'll extract that as a blocking issue, with a fairly trivial MR that can land very quickly, to unblock this issue to do things "properly". ๐ - Status changed to Postponed
4 months ago 9:07pm 12 August 2024 - ๐ฌ๐งUnited Kingdom f.mazeikis Brighton
Postponed until blocker ๐ Add fieldInstanceSettings to (Candidate)StorablePropShape Active is resolved.
- Status changed to Active
4 months ago 12:10pm 14 August 2024 - ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Rather than this being automated, what about an import wizard from the list builder as an action link? When we support blocks that could also be included
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
#20:
@lauriii has stated it's a hard requirement that it is possible to do it fully automatically.
As a next step, we would only do it fully automatically when in "development mode" (because on production you wouldn't want new SDCs appearing in XB. See ๐ [PP-2] Add ComponentAuditabilityTest Active , and the discussions with @catch on that subject over at ๐ฑ [META] Configuration management: define needed config entity types Active .
- Merge request !174#3463999: Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria โ (Merged) created by Unnamed author
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Reviewed ๐
The failing
Cypress E2E
tests are showing this in the output:{ "status": "PARSING_ERROR", "originalStatus": 500, "data": "The website encountered an unexpected error. Try again later.<br><br><em class=\"placeholder\">Twig\\Error\\RuntimeError</em>: An exception has been thrown during the rendering of a template ("[activation] String value found, but an array or an object is required"). in <em class=\"placeholder\">Twig\\Template->yield()</em> (line <em class=\"placeholder\">1</em> of <em class=\"placeholder\">modules/custom/experience_builder/components/containers/shoe_tab_group/shoe_tab_group.twig</em>). <pre class=\"backtrace\">Drupal\\Core\\Template\\ComponentsTwigExtension->doValidateProps(Array, 'experience_builder:shoe_tab_group') (Line: 109)\nDrupal\\Core\\Template\\ComponentsTwigExtension->validateProps(Arr ...
โฆ and sure enough,
components/containers/shoe_tab_group/shoe_tab_group.component.yml
does look rather bizarre:activation: type: array title: Activation description: When set to auto, navigating tabs with the arrow keys will instantly show the corresponding tab panel. When set to manual, the tab will receive focus but will not show until the user presses space bar or enter. default: auto examples: ['auto', 'manual']
I went back to ๐ Introduce an example set of representative SDC components; transition from "component list" to "component tree" Fixed and found that @ctrlADel's original implementation of that (commit:
ef34f8106dfe2103df38055154efc9bef9ba9b1d
) was never changed by @tedbow nor me. So I think this was just an honest mistake.Fixed in https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
A bunch of the components that @ctrlADel added in ๐ Introduce an example set of representative SDC components; transition from "component list" to "component tree" Fixed do have
examples
but then specify no examples. That's odd/weird, but XB should graciously handle that: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... - Status changed to Needs work
4 months ago 10:40am 19 August 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
I pushed a handful of tiny commits to help get this MR to green โ because the failures were happening in areas far removed from the changes that are actually in the scope of this MR (and far outside areas @f.mazeikis has previously touched).
I also solved one very tricky thing WRT default config: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1....
This MR still needs:
- test coverage (see https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...)
- to address remaining review feedback
- conflict resolution
โฆ but those should be smooth sailing now ๐๐ค Looking forward to seeing this one land! ๐
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ Unable to save node article form โ remove obsolete TwoTerribleTextAreasWidget + fix duplicate `XB:image` SDC Fixed unblocked this issue โ thanks @f.mazeikis for reviewing that!
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Tests look great!
I bet fixing https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... will also fix the failing Cypress E2E test.
This feels very close now ๐๐ฅณ
- Issue was unassigned.
- Status changed to RTBC
4 months ago 7:30am 22 August 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
This issue represents a huge leap forward! ๐
Most importantly in the short term: this will make it an order of magnitude easier to start testing https://www.drupal.org/project/demo_design_system โ ๐
Also:
- This unblocks ๐ [PP-2] Remove Component config entity's add/edit form Postponed , which likely will be quite satisfying for @f.mazeikis to do :)
- It also unblocks the follow-up ๐ New component requirement: each SDC prop must have StorablePropShape Active , which will also help testing https://www.drupal.org/project/demo_design_system โ
- This made me realize that we should do (the newly opened) ๐ Prepare for multiple component types: ComponentTreeStructure should contain Component config entity IDs, not SDC IDs Fixed ASAP for a consistent data model.
-
Wim Leers โ
committed 3e7ee244 on 0.x authored by
f.mazeikis โ
Issue #3463999 by f.mazeikis, Wim Leers, lauriii: Auto-create/update...
-
Wim Leers โ
committed 3e7ee244 on 0.x authored by
f.mazeikis โ
- Status changed to Fixed
4 months ago 8:04am 22 August 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Automatically closed - issue fixed for 2 weeks with no activity.