- Issue created by @nx2611
- Status changed to Fixed
about 2 months ago 8:27am 27 May 2025 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
The
type
is wrong:content_attributes: type: string title: 'Content attributes' description: 'Same as attributes, except applied to the main content tag that appears in the template.'
should be
content_attributes: type: Drupal\Core\Template\Attribute title: 'Content attributes' description: 'Same as attributes, except applied to the main content tag that appears in the template.'
If you do that, it'll be ignored automatically :) ๐
Interested in knowing why/how? โ See this bit in
\Drupal\experience_builder\PropShape\PropShape::getComponentPropsForMetadata()
:// TRICKY: `attributes` is a special case โ it is kind of a reserved // prop. // @see \Drupal\sdc\Twig\TwigExtension::mergeAdditionalRenderContext() // @see https://www.drupal.org/project/drupal/issues/3352063#comment-15277820 if ($prop_name === 'attributes') { assert($prop_schema['type'][0] === Attribute::class); continue; }
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Ah, lol, I misread my own code ๐
if ($prop_name === 'attributes') {
โ what I wrote is not true. It should just check the type?
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
MR with test coverage up. ๐
But โฆ does this even work? Doesn't
\Drupal\Core\Template\ComponentsTwigExtension::mergeAdditionalRenderContext()
only support a singleattributes
prop? ๐So that's definitely an upstream bug in SDC.
- ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
Assigning for the feedback you asked and removing tag. Not saying that you are the one who should fix the tests ๐
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Per #3513406-8: ValueError: "Drupal\Core\Template\Attribute" is not a valid backing value for enum โ , this affects the quite popular https://www.drupal.org/project/radix โ theme significantly.
Basically this just needs to be generalized:
\Drupal\experience_builder\ComponentMetadataRequirementsChecker::check()
foreach ($metadata->schema['properties'] as $prop_name => $prop) { if ($prop_name === 'attributes') { continue; }
โฆ which is the direction @penyaskito's in-progress MR already is taking ๐ Let's revive & land it!
- ๐ณ๐ฑNetherlands balintbrews Amsterdam, NL
wim leers โ credited balintbrews โ .
- ๐ญ๐บHungary nevergone Nyรญregyhรกza, Hungary, Europe
wim leers โ credited nevergone โ .
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Marked #3513406 as a duplicate per #3513406-13: ValueError: "Drupal\Core\Template\Attribute" is not a valid backing value for enum โ . Crediting the contributors there that helped get clarity there, which allowed marking it as a duplicate ๐
- ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
Mentioned today, while we are here let's ensure we have a plan (fix here or follow-up) for Attributes prop marked as required works, even if that doesn't make any sense.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Mentioned today, while we are here let's ensure we have a plan (fix here or follow-up) for Attributes prop marked as required works, even if that doesn't make any sense.
๐ This is not yet addressed, but the MR is ready, so tagging .
Also linking the core issues that @penyaskito found in his research at https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
LGTM โ only needs addressing of @larowlan's edge case, and just have 2 minor questions.
- ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
Handled feedback. Created follow-up` ๐ Handle required `type: Drupal\Core\Template\Attribute`s props in SDCs Active
-
wim leers โ
committed 67a54b6f on 1.x
Issue #3493068 by penyaskito, wim leers, nx2611, nevergone, kiwad,...
-
wim leers โ
committed 67a54b6f on 1.x