Skip to content

Commit 6658bba

Browse files
tjwiebelltjwiebell
committed
MC-2228: Pagebuilder content type configuration merging issue
- Add optional storage_key attribute to style and attribute XSD - Update XSD so name is always guaranteed to be unique - Update all content types with duplicate names to begin using this new field and make names unique - Updated documentation with schema changes
1 parent 0954b2e commit 6658bba

25 files changed

+181
-120
lines changed

‎app/code/Magento/PageBuilder/Model/Config/ContentType/Converter.php

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ private function convertProperties(\DOMElement $elementNode): array
267267
if ($propertiesNode) {
268268
foreach ($propertiesNode->getElementsByTagName('property') as $propertyNode) {
269269
$propertiesData[] = [
270-
'var' => $this->getAttributeValue($propertyNode, 'name'),
270+
'var' => $this->extractVariableName($propertyNode),
271271
'name' => $this->getAttributeValue($propertyNode, 'source'),
272272
'converter' => $this->getAttributeValue($propertyNode, 'converter'),
273273
'preview_converter' => $this->getAttributeValue($propertyNode, 'preview_converter'),
@@ -277,7 +277,7 @@ private function convertProperties(\DOMElement $elementNode): array
277277
}
278278
foreach ($propertiesNode->getElementsByTagName('complex_property') as $propertyNode) {
279279
$propertiesData[] = [
280-
'var' => $this->getAttributeValue($propertyNode, 'name'),
280+
'var' => $this->extractVariableName($propertyNode),
281281
'reader' => $this->getAttributeValue($propertyNode, 'reader'),
282282
'converter' => $this->getAttributeValue($propertyNode, 'converter'),
283283
'preview_converter' => $this->getAttributeValue($propertyNode, 'preview_converter'),
@@ -309,7 +309,7 @@ private function convertAttributes(\DOMElement $elementNode): array
309309
if ($attributesNode) {
310310
foreach ($attributesNode->getElementsByTagName('attribute') as $attributeNode) {
311311
$attributesData[] = [
312-
'var' => $this->getAttributeValue($attributeNode, 'name'),
312+
'var' => $this->extractVariableName($attributeNode),
313313
'name' => $this->getAttributeValue($attributeNode, 'source'),
314314
'converter' => $this->getAttributeValue($attributeNode, 'converter'),
315315
'preview_converter' => $this->getAttributeValue($attributeNode, 'preview_converter'),
@@ -326,7 +326,7 @@ private function convertAttributes(\DOMElement $elementNode): array
326326
}
327327
foreach ($attributesNode->getElementsByTagName('complex_attribute') as $attributeNode) {
328328
$attributesData[] = [
329-
'var' => $this->getAttributeValue($attributeNode, 'name'),
329+
'var' => $this->extractVariableName($attributeNode),
330330
'reader' => $this->getAttributeValue($attributeNode, 'reader'),
331331
'converter' => $this->getAttributeValue($attributeNode, 'converter'),
332332
'preview_converter' => $this->getAttributeValue($attributeNode, 'preview_converter'),
@@ -458,4 +458,16 @@ private function getAttributeValue(\DOMElement $attributeNode, $attributeName)
458458
? $attributeNode->attributes->getNamedItem($attributeName)->nodeValue
459459
: null;
460460
}
461+
462+
/**
463+
* Extract variable name from property and attribute nodes
464+
*
465+
* @param \DOMElement $node
466+
* @return string
467+
*/
468+
private function extractVariableName(\DOMElement $node): string
469+
{
470+
return $this->getAttributeValue($node, 'storage_key')
471+
?: $this->getAttributeValue($node, 'name');
472+
}
461473
}

‎app/code/Magento/PageBuilder/docs/content-type-configuration.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ The following is an example of a content type configuration in `view/adminhtml/p
123123
<element name="link" path=".//a">
124124
<attributes>
125125
<complex_attribute name="link_url" reader="Magento_PageBuilder/js/property/link" persist="false"/>
126-
<attribute name="link_url" source="href" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-href" />
127-
<attribute name="link_url" source="target" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-target" />
128-
<attribute name="link_url" source="data-link-type" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-type" />
126+
<attribute name="virtual_link_href" storage_key="link_url" source="href" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-href"/>
127+
<attribute name="virtual_link_target" storage_key="link_url" source="target" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-target"/>
128+
<attribute name="virtual_link_type" storage_key="link_url" source="data-link-type" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-type"/>
129129
</attributes>
130130
</element>
131131
<element name="overlay" path=".//a/div[2]/div">
@@ -137,7 +137,7 @@ The following is an example of a content type configuration in `view/adminhtml/p
137137
<attributes>
138138
<attribute name="overlay_color" source="data-overlay-color" persist="false" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-color"/>
139139
<attribute name="overlay_transparency" source="data-overlay-color" persist="false" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-transparency"/>
140-
<attribute name="overlay_transparency" source="data-overlay-color" virtual="true" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-color-transparency"/>
140+
<attribute name="virtual_overlay_transparency" storage_key="overlay_transparency" source="data-overlay-color" virtual="true" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-color-transparency"/>
141141
</attributes>
142142
</element>
143143
<element name="desktop_image" path=".//a/div[1]">
@@ -316,16 +316,16 @@ Set the `default` attribute to "true" in an `appearance` node to set the default
316316
<element name="link" path=".//a">
317317
<attributes>
318318
<complex_attribute name="link_url" reader="Magento_PageBuilder/js/property/link" persist="false"/>
319-
<attribute name="link_url" source="href" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-href" />
320-
<attribute name="link_url" source="target" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-target" />
321-
<attribute name="link_url" source="data-link-type" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-type" />
319+
<attribute name="virtual_link_href" storage_key="link_url" source="href" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-href"/>
320+
<attribute name="virtual_link_target" storage_key="link_url" source="target" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-target"/>
321+
<attribute name="virtual_link_type" storage_key="link_url" source="data-link-type" virtual="true" converter="Magento_PageBuilder/js/converter/attribute/link-type"/>
322322
</attributes>
323323
</element>
324324
<element name="overlay" path=".//a/div[2]/div">
325325
<attributes>
326326
<attribute name="overlay_color" source="data-overlay-color" persist="false" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-color"/>
327327
<attribute name="overlay_transparency" source="data-overlay-color" persist="false" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-transparency"/>
328-
<attribute name="overlay_transparency" source="data-overlay-color" virtual="true" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-color-transparency"/>
328+
<attribute name="virtual_overlay_transparency" storage_key="overlay_transparency" source="data-overlay-color" virtual="true" converter="Magento_PageBuilder/js/converter/banner/attribute/overlay-color-transparency"/>
329329
</attributes>
330330
</element>
331331
<element name="desktop_image" path=".//a/div[1]">
@@ -368,7 +368,8 @@ Set the `default` attribute to "true" in an `appearance` node to set the default
368368

369369
| Attribute | Description |
370370
| ------------------- | ---------------------------------------------------------------------------------------------------------------------- |
371-
| `name` | The variable name for value in the data storage. Must be unique for the content type. |
371+
| `name` | Unique name used for configuration merging, and the default value for storage_key if none is provided. |
372+
| `storage_key` | Optional variable name for value in the data storage. If no value is provided, name will be used. |
372373
| `source` | The name of the property in the DOM. Must be in snake case. |
373374
| `converter` | Converts the value after reading or before saving to the DOM. |
374375
| `preview_converter` | Converts the value for the preview. Used for cases where the conversion logic is different between the two views. |
@@ -381,7 +382,7 @@ Set the `default` attribute to "true" in an `appearance` node to set the default
381382

382383
``` xml
383384
<style_properties>
384-
<complex_property name="margins_and_padding" reader="Magento_PageBuilder/js/property/margins" converter="Magento_PageBuilder/js/converter/style/margins"/>
385+
<complex_property name="margins" storage_key="margins_and_padding" reader="Magento_PageBuilder/js/property/margins" converter="Magento_PageBuilder/js/converter/style/margins"/>
385386
</style_properties>
386387
```
387388

‎app/code/Magento/PageBuilder/docs/how-to-add-new-content-type.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ To add configuration for a new content type, create a file under the following l
8989
<property name="border_color" source="border_color" converter="Magento_PageBuilder/js/converter/style/color"/>
9090
<property name="border_width" source="border_width" converter="Magento_PageBuilder/js/converter/style/border-width"/>
9191
<property name="border_radius" source="border_radius" converter="Magento_PageBuilder/js/converter/style/remove-px"/>
92-
<complex_property name="margins_and_padding" reader="Magento_PageBuilder/js/property/margins" converter="Magento_PageBuilder/js/converter/style/margins"/>
93-
<complex_property name="margins_and_padding" reader="Magento_PageBuilder/js/property/paddings" converter="Magento_PageBuilder/js/converter/style/paddings"/>
92+
<complex_property name="margins" storage_key="margins_and_padding" reader="Magento_PageBuilder/js/property/margins" converter="Magento_PageBuilder/js/converter/style/margins"/>
93+
<complex_property name="padding" storage_key="margins_and_padding" reader="Magento_PageBuilder/js/property/paddings" converter="Magento_PageBuilder/js/converter/style/paddings"/>
9494
</style_properties>
9595
<attributes>
9696
<attribute name="name" source="data-role"/>
@@ -294,8 +294,8 @@ Now, let's add content type that can contain other content types. Create configu
294294
<property name="border_color" source="border_color" converter="Magento_PageBuilder/js/converter/style/color"/>
295295
<property name="border_width" source="border_width" converter="Magento_PageBuilder/js/converter/style/border-width"/>
296296
<property name="border_radius" source="border_radius" converter="Magento_PageBuilder/js/converter/style/remove-px"/>
297-
<complex_property name="margins_and_padding" reader="Magento_PageBuilder/js/property/margins" converter="Magento_PageBuilder/js/converter/style/margins"/>
298-
<complex_property name="margins_and_padding" reader="Magento_PageBuilder/js/property/paddings" converter="Magento_PageBuilder/js/converter/style/paddings"/>
297+
<complex_property name="margins" storage_key="margins_and_padding" reader="Magento_PageBuilder/js/property/margins" converter="Magento_PageBuilder/js/converter/style/margins"/>
298+
<complex_property name="padding" storage_key="margins_and_padding" reader="Magento_PageBuilder/js/property/paddings" converter="Magento_PageBuilder/js/converter/style/paddings"/>
299299
</style_properties>
300300
<attributes>
301301
<attribute name="name" source="data-role"/>

‎app/code/Magento/PageBuilder/etc/content_type.xsd

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,28 @@
122122
</xs:complexType>
123123
<xs:complexType name="element">
124124
<xs:sequence>
125-
<xs:element type="style_properties" name="style_properties" minOccurs="0" maxOccurs="1"/>
126-
<xs:element type="attributes" name="attributes" minOccurs="0" maxOccurs="1"/>
125+
<xs:element type="style_properties" name="style_properties" minOccurs="0" maxOccurs="1">
126+
<xs:unique name="uniquePropertyName">
127+
<xs:annotation>
128+
<xs:documentation>
129+
Property and Complex Property name must be unique
130+
</xs:documentation>
131+
</xs:annotation>
132+
<xs:selector xpath="property|complex_property"/>
133+
<xs:field xpath="@name"/>
134+
</xs:unique>
135+
</xs:element>
136+
<xs:element type="attributes" name="attributes" minOccurs="0" maxOccurs="1">
137+
<xs:unique name="uniqueAttributeName">
138+
<xs:annotation>
139+
<xs:documentation>
140+
Attribute and Complex Attribute name must be unique
141+
</xs:documentation>
142+
</xs:annotation>
143+
<xs:selector xpath="attribute|complex_attribute"/>
144+
<xs:field xpath="@name"/>
145+
</xs:unique>
146+
</xs:element>
127147
<xs:element type="tag" name="tag" minOccurs="0" maxOccurs="1"/>
128148
<xs:element type="html" name="html" minOccurs="0" maxOccurs="1"/>
129149
<xs:element type="css" name="css" minOccurs="0" maxOccurs="1"/>
@@ -150,13 +170,15 @@
150170
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
151171
<xs:attribute type="xs:string" name="virtual" use="optional"/>
152172
<xs:attribute type="xs:string" name="persist" use="optional"/>
173+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
153174
</xs:complexType>
154175
<xs:complexType name="complex_property">
155176
<xs:attribute type="xs:string" name="name" use="required"/>
156177
<xs:attribute type="xs:string" name="reader" use="optional"/>
157178
<xs:attribute type="xs:string" name="converter" use="optional"/>
158179
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
159180
<xs:attribute type="xs:string" name="virtual" use="optional"/>
181+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
160182
</xs:complexType>
161183
<xs:complexType name="static_property">
162184
<xs:attribute type="xs:string" name="source" use="required"/>
@@ -176,6 +198,7 @@
176198
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
177199
<xs:attribute type="xs:string" name="virtual" use="optional"/>
178200
<xs:attribute type="xs:boolean" name="persist" use="optional"/>
201+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
179202
</xs:complexType>
180203
<xs:complexType name="complex_attribute">
181204
<xs:attribute type="xs:string" name="name" use="required"/>
@@ -184,6 +207,7 @@
184207
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
185208
<xs:attribute type="xs:string" name="virtual" use="optional"/>
186209
<xs:attribute type="xs:boolean" name="persist" use="optional"/>
210+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
187211
</xs:complexType>
188212
<xs:complexType name="static_attribute">
189213
<xs:attribute type="xs:string" name="source" use="required"/>

‎app/code/Magento/PageBuilder/etc/content_type_merged.xsd

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,28 @@
122122
</xs:complexType>
123123
<xs:complexType name="element">
124124
<xs:sequence>
125-
<xs:element type="style_properties" name="style_properties" minOccurs="0" maxOccurs="1"/>
126-
<xs:element type="attributes" name="attributes" minOccurs="0" maxOccurs="1"/>
125+
<xs:element type="style_properties" name="style_properties" minOccurs="0" maxOccurs="1">
126+
<xs:unique name="uniquePropertyName">
127+
<xs:annotation>
128+
<xs:documentation>
129+
Property and Complex Property name must be unique
130+
</xs:documentation>
131+
</xs:annotation>
132+
<xs:selector xpath="property|complex_property"/>
133+
<xs:field xpath="@name"/>
134+
</xs:unique>
135+
</xs:element>
136+
<xs:element type="attributes" name="attributes" minOccurs="0" maxOccurs="1">
137+
<xs:unique name="uniqueAttributeName">
138+
<xs:annotation>
139+
<xs:documentation>
140+
Attribute and Complex Attribute name must be unique
141+
</xs:documentation>
142+
</xs:annotation>
143+
<xs:selector xpath="attribute|complex_attribute"/>
144+
<xs:field xpath="@name"/>
145+
</xs:unique>
146+
</xs:element>
127147
<xs:element type="tag" name="tag" minOccurs="0" maxOccurs="1"/>
128148
<xs:element type="html" name="html" minOccurs="0" maxOccurs="1"/>
129149
<xs:element type="css" name="css" minOccurs="0" maxOccurs="1"/>
@@ -150,13 +170,15 @@
150170
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
151171
<xs:attribute type="xs:string" name="virtual" use="optional"/>
152172
<xs:attribute type="xs:string" name="persist" use="optional"/>
173+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
153174
</xs:complexType>
154175
<xs:complexType name="complex_property">
155176
<xs:attribute type="xs:string" name="name" use="required"/>
156177
<xs:attribute type="xs:string" name="reader" use="required"/>
157178
<xs:attribute type="xs:string" name="converter" use="optional"/>
158179
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
159180
<xs:attribute type="xs:string" name="virtual" use="optional"/>
181+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
160182
</xs:complexType>
161183
<xs:complexType name="static_property">
162184
<xs:attribute type="xs:string" name="source" use="required"/>
@@ -177,6 +199,7 @@
177199
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
178200
<xs:attribute type="xs:string" name="virtual" use="optional"/>
179201
<xs:attribute type="xs:boolean" name="persist" use="optional"/>
202+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
180203
</xs:complexType>
181204
<xs:complexType name="complex_attribute">
182205
<xs:attribute type="xs:string" name="name" use="required"/>
@@ -185,6 +208,7 @@
185208
<xs:attribute type="xs:string" name="preview_converter" use="optional"/>
186209
<xs:attribute type="xs:string" name="virtual" use="optional"/>
187210
<xs:attribute type="xs:boolean" name="persist" use="optional"/>
211+
<xs:attribute type="xs:string" name="storage_key" use="optional"/>
188212
</xs:complexType>
189213
<xs:complexType name="static_attribute">
190214
<xs:attribute type="xs:string" name="source" use="required"/>

0 commit comments

Comments
 (0)