Skip to content

Commit dbc556c

Browse files
author
Hwashiang Yu
committed
MC-5810: Improve naming of the critical variables/parameters in the code and configuration
- Resolved static test failures - Correct data-content-type on non content type elements - Resolved column sorting failures
1 parent e6371fa commit dbc556c

File tree

10 files changed

+53
-34
lines changed

10 files changed

+53
-34
lines changed

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010

1111
use Magento\Framework\ObjectManager\Config\Mapper\ArgumentParser;
1212

13+
/**
14+
* Convert content type configuration data
15+
*/
1316
class Converter implements \Magento\Framework\Config\ConverterInterface
1417
{
1518
const DEFAULT_ATTRIBUTE_READER = 'Magento_PageBuilder/js/property/attribute-reader';
@@ -357,7 +360,9 @@ private function convertTag(\DOMElement $elementNode): array
357360
}
358361

359362
/**
360-
* @param \DOMElement $childNode
363+
* Converts converter data
364+
*
365+
* @param \DOMElement $appearanceNode
361366
* @return array
362367
*/
363368
private function convertConvertersData(\DOMElement $appearanceNode): array
@@ -519,11 +524,11 @@ private function isConfigNode(\DOMNode $node): bool
519524
/**
520525
* Get attribute value
521526
*
522-
* @param $attributeNode
523-
* @param $attributeName
527+
* @param \DOMElement $attributeNode
528+
* @param string $attributeName
524529
* @return string|null
525530
*/
526-
private function getAttributeValue(\DOMElement $attributeNode, $attributeName)
531+
private function getAttributeValue(\DOMElement $attributeNode, string $attributeName)
527532
{
528533
return $attributeNode->hasAttribute($attributeName)
529534
? $attributeNode->attributes->getNamedItem($attributeName)->nodeValue
@@ -545,8 +550,8 @@ private function extractVariableName(\DOMElement $node): string
545550
/**
546551
* Remove data from array
547552
*
548-
* @param $searchValue
549-
* @param $data
553+
* @param string $searchValue
554+
* @param array $data
550555
* @return array
551556
*/
552557
private function removeDataInArray(string $searchValue, array $data): array

‎app/code/Magento/PageBuilder/Test/Mftf/Section/PageBuilderEditFormSection.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
<element name="panelFieldNoAfterLabelText" type="text" selector="//div[@data-index='{{arg1}}']//span[@data-bind='text: addafter']" parameterized="true"/>
3434
<element name="panelFieldValidationError" type="input" selector="//div[contains(@class,'_insert_form')]//div[@data-index='{{arg1}}']//div[@data-index='{{arg2}}' and contains(@class,'_error')]//div[contains(@class,'')]//input[@name='{{arg2}}']" parameterized="true"/>
3535
<element name="panelFieldValidationErrorMessage" type="button" selector="//div[contains(@class,'_insert_form')]//div[@data-index='{{arg1}}']//div[contains(@class,'admin__field-control')]//input[@name='{{arg2}}']//..//../label[.='{{arg3}}']" parameterized="true"/>
36-
<element name="panelMultiSelectFieldControl" type="input" selector="aside [data-index='{{arg1}}'] [data-index='{{arg2}}'] [data-content-type='advanced-select']" parameterized="true"/>
36+
<element name="panelMultiSelectFieldControl" type="input" selector="aside [data-index='{{arg1}}'] [data-index='{{arg2}}'] [data-role='advanced-select']" parameterized="true"/>
3737
<element name="panelMultiSelectFieldControlInput" type="input" selector="aside [data-index='{{arg1}}'] [data-index='{{arg2}}'] .admin__action-multiselect-search" parameterized="true"/>
3838
<element name="panelMultiSelectFieldControlResult" type="input" selector="//aside//div[@data-index='{{arg1}}']//div[@data-index='{{arg2}}']//div[contains(@class,'action-menu-item')]//span[.='{{arg3}}']" parameterized="true"/>
3939
<element name="editFormAllRequiredFields" type="text" selector="aside.pagebuilder_modal_form_pagebuilder_modal_form_modal ._required:not([style*='display: none;'])"/>

‎app/code/Magento/PageBuilder/Test/Mftf/Section/PageBuilderProductsSection.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Page/etc/SectionObject.xsd">
1111
<section name="ProductsOnStage">
1212
<element name="base" type="text" selector="(//div[contains(@class,'pagebuilder-products')]//div[contains(@data-appearance,'grid')])[{{arg1}}]" parameterized="true"/>
13-
<element name="price" type="text" selector="//div[contains(@class,'pagebuilder-products')]//div[contains(@data-appearance,'grid')]//div[contains(@data-content-type,'priceBox')]"/>
13+
<element name="price" type="text" selector="//div[contains(@class,'pagebuilder-products')]//div[contains(@data-appearance,'grid')]//div[contains(@data-role,'priceBox')]"/>
1414
<element name="hidden" type="text" selector="(//div[contains(@class,'pagebuilder-products')]//div[contains(@data-appearance,'grid')])[{{arg1}}]/ancestor::*[contains(@class, 'pagebuilder-content-type-wrapper') and contains(@class, 'pagebuilder-content-type-hidden')]" parameterized="true"/>
1515
<element name="notHidden" type="text" selector="(//div[contains(@class,'pagebuilder-products') and not(contains(@class,'placeholder'))])[{{arg1}}]//parent::*[contains(@class, 'pagebuilder-content-type-wrapper') and not(contains(@class, 'pagebuilder-content-type-hidden'))][1]" parameterized="true"/>
1616
<element name="product" type="text" selector="(//div[contains(@class,'pagebuilder-products')]//div[contains(@data-appearance,'grid')])[{{arg1}}]//ol//li[contains(@class,'product-item')]" parameterized="true"/>
@@ -48,7 +48,7 @@
4848
</section>
4949
<section name="ProductsOnStorefront">
5050
<element name="base" type="text" selector="(//div[contains(@data-content-type,'products')])[{{arg1}}]" parameterized="true"/>
51-
<element name="price" type="text" selector="//div[contains(@data-content-type,'products')]//div[contains(@data-content-type,'priceBox')]"/>
51+
<element name="price" type="text" selector="//div[contains(@data-content-type,'products')]//div[contains(@data-role,'priceBox')]"/>
5252
<element name="hidden" type="text" selector="(//div[contains(@data-content-type,'products')])[{{arg1}}][contains(@style, 'display: none')]" parameterized="true"/>
5353
<element name="notHidden" type="text" selector="(//div[contains(@data-content-type,'products')])[{{arg1}}][not(contains(@style, 'display: none'))]" parameterized="true"/>
5454
<element name="product" type="text" selector="(//div[contains(@data-content-type,'products')])[{{arg1}}]//ol//li[contains(@class,'product-item')]" parameterized="true"/>

‎app/code/Magento/PageBuilder/Test/Mftf/Section/PageBuilderURLSection.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
<element name="SearchResultBox" type="select" selector=".action-menu._active"/>
1717
<element name="LinkItemSelected" type="text" selector="//span[text()='{{var1}}']/parent::label/parent::div[contains(@class,'_selected')]" parameterized="true"/>
1818
<element name="SearchResult" type="text" selector="//span[text()='{{var1}}']" parameterized="true"/>
19-
<element name="DropdownInput" type="input" selector="//div[@class='url-input-element-linked-element']//div[@data-content-type='selected-option']"/>
19+
<element name="DropdownInput" type="input" selector="//div[@class='url-input-element-linked-element']//div[@data-role='selected-option']"/>
2020
<element name="RemoveBtn" type="button" selector="button[data-action='remove-selected-item']"/>
2121
</section>
2222
<section name="PageBuilderURLOnStoreFrontSection">

‎app/code/Magento/PageBuilder/Test/Mftf/Test/AdminPageBuilderColumnGridTest.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@
849849
<actionGroup ref="dragContentTypeToStage" stepKey="dragColumnIntoStage">
850850
<argument name="contentType" value="PageBuilderColumnContentType"/>
851851
</actionGroup>
852-
<actionGroup ref="updateGridSize" stepKey="updateGridSizeTo3">
852+
<actionGroup ref="updateGridSize" stepKey="updateGridSizeTo4">
853853
<argument name="gridSize" value="4"/>
854854
</actionGroup>
855855
<actionGroup ref="dragContentTypeToContainer" stepKey="addColumn1">

‎app/code/Magento/PageBuilder/view/adminhtml/web/js/content-type/column-group/preview.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎app/code/Magento/PageBuilder/view/adminhtml/web/js/drag-drop/sortable.js

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type/column-group/preview.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,11 +345,10 @@ export default class Preview extends PreviewCollection {
345345
return helper;
346346
},
347347
start: (event: Event) => {
348-
const columnInstance: ContentTypeCollectionInterface = ko.dataFor($(event.target)[0]);
348+
const columnInstance = ko.dataFor($(event.target)[0]);
349349
// Use the global state as columns can be dragged between groups
350-
setDragColumn((columnInstance.containerContentType as ContentTypeCollectionInterface<ColumnPreview>));
350+
setDragColumn((columnInstance.master as ContentTypeCollectionInterface<ColumnPreview>));
351351
this.dropPositions = calculateDropPositions(this.master);
352-
353352
events.trigger("column:dragStart", {
354353
column: columnInstance,
355354
stageId: this.master.stageId,

‎app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/drag-drop/sortable.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,12 @@ function onSortUpdate(preview: Preview, event: Event, ui: JQueryUI.SortableUIPar
247247
ui.item.remove();
248248
$(this).sortable("cancel");
249249

250-
// jQuery tries to reset the state but kills KO's bindings, so we'll force a re-render on the parent
250+
// jQuery tries to reset the state but kills KO's bindings, so we'll force a re-render on the master
251251
if (ui.item.length > 0 && typeof ko.dataFor(ui.item[0]) !== "undefined") {
252-
const parent = ko.dataFor(ui.item[0]).parent as ContentTypeCollectionInterface;
253-
const children = parent.getChildren()().splice(0);
254-
parent.getChildren()([]);
255-
parent.getChildren()(children);
252+
const master = ko.dataFor(ui.item[0]).master as ContentTypeCollectionInterface;
253+
const children = master.getChildren()().splice(0);
254+
master.getChildren()([]);
255+
master.getChildren()(children);
256256
}
257257
return;
258258
}
@@ -272,7 +272,7 @@ function onSortUpdate(preview: Preview, event: Event, ui: JQueryUI.SortableUIPar
272272

273273
if (target && contentTypeInstance) {
274274
// Calculate the source and target index
275-
const sourceParent: ContentTypeCollectionInterface = contentTypeInstance.parent;
275+
const sourceParent: ContentTypeCollectionInterface = contentTypeInstance.containerContentType;
276276
const targetParent: ContentTypeCollectionInterface = getMasterProxy(target);
277277

278278
const targetIndex = $(placeholderContainer)
@@ -290,7 +290,7 @@ function onSortUpdate(preview: Preview, event: Event, ui: JQueryUI.SortableUIPar
290290

291291
moveContentType(contentTypeInstance, targetIndex, targetParent);
292292

293-
if (contentTypeInstance.parent !== targetParent) {
293+
if (contentTypeInstance.containerContentType !== targetParent) {
294294
ui.item.remove();
295295
}
296296
}

‎dev/tests/js/jasmine/tests/app/code/Magento/PageBuilder/view/frontend/web/js/widget/show-on-hover.test.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ define([
2626

2727
el = $(
2828
'<div>' +
29-
'<div class="button-selector" data-content-type="thing" data-show-overlay="1" data-show-button="1">' +
29+
'<div class="button-selector" ' +
30+
'data-content-type="thing" ' +
31+
'data-show-overlay="1" ' +
32+
'data-show-button="1">' +
3033
'<a></a>' +
3134
'</div>' +
3235
'</div>'
@@ -50,7 +53,10 @@ define([
5053

5154
el = $(
5255
'<div>' +
53-
'<div class="button-selector" data-content-type="thing" data-show-overlay="0" data-show-button="0">' +
56+
'<div class="button-selector" ' +
57+
'data-content-type="thing" ' +
58+
'data-show-overlay="0" ' +
59+
'data-show-button="0">' +
5460
'<a></a>' +
5561
'</div>' +
5662
'</div>'
@@ -74,7 +80,10 @@ define([
7480

7581
el = $(
7682
'<div>' +
77-
'<div class="button-selector" data-content-type="thing" data-show-overlay="1" data-show-button="0">' +
83+
'<div class="button-selector" ' +
84+
'data-content-type="thing" ' +
85+
'data-show-overlay="1" ' +
86+
'data-show-button="0">' +
7887
'<a></a>' +
7988
'</div>' +
8089
'</div>'
@@ -98,7 +107,10 @@ define([
98107

99108
el = $(
100109
'<div>' +
101-
'<div class="button-selector" data-content-type="thing" data-show-overlay="0" data-show-button="1">' +
110+
'<div class="button-selector" ' +
111+
'data-content-type="thing" ' +
112+
'data-show-overlay="0" ' +
113+
'data-show-button="1">' +
102114
'<a></a>' +
103115
'</div>' +
104116
'</div>'
@@ -122,7 +134,10 @@ define([
122134

123135
el = $(
124136
'<div>' +
125-
'<div class="button-selector" data-content-type="thing" data-show-overlay="1" data-show-button="1">' +
137+
'<div class="button-selector" ' +
138+
'data-content-type="thing" ' +
139+
'data-show-overlay="1" ' +
140+
'data-show-button="1">' +
126141
'<a></a>' +
127142
'</div>' +
128143
'</div>'

0 commit comments

Comments
 (0)