MC-3653: Can Drag & Drop Child Content Types Into Hidden Content Types

davemacaulay · davemacaulay · commit 6985fa3639c3 · 2018-08-13T15:25:55.000+02:00
- Block new content types being dropped into hidden content types
- Block dragging existing content types into hidden content types
- Fix display label z-index
diff --git a/app/code/Magento/PageBuilder/Test/Mftf/ActionGroup/DragAndDropActionGroup.xml b/app/code/Magento/PageBuilder/Test/Mftf/ActionGroup/DragAndDropActionGroup.xml
@@ -7,7 +7,7 @@
 -->
 
 <actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-        xsi:noNamespaceSchemaLocation="../../../../../../../dev/tests/acceptance/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Test/etc/actionGroupSchema.xsd">
+        xsi:noNamespaceSchemaLocation="../../../../../../../../dev/tests/acceptance/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Test/etc/actionGroupSchema.xsd">
     <actionGroup name="dragContentTypeToStage">
         <arguments>
             <argument name="contentType"/>
@@ -55,6 +55,22 @@
         <waitForElementVisible time="10" selector="{{PageBuilderStage.contentTypeInStageByIndex(contentType.role, contentTypeIndex)}}" stepKey="waitForContentTypeInStage" />
         <waitForPageLoad stepKey="waitForToolbarsToLoad" time="30"/>
     </actionGroup>
+    <actionGroup name="dragContentTypeToHiddenContainer">
+        <arguments>
+            <argument name="contentType"/>
+            <argument name="contentTypeIndex" defaultValue="1" type="string"/>
+            <argument name="containerTargetType"/>
+            <argument name="containerTargetIndex" defaultValue="1" type="string"/>
+            <argument name="offsetXCoordinate" defaultValue="null" type="string"/>
+            <argument name="offsetYCoordinate" defaultValue="null" type="string"/>
+        </arguments>
+        <click selector="{{PageBuilderPanel.searchPanel}}" stepKey="lostFocus"/>
+        <waitForElementVisible time="30" selector="{{PageBuilderPanel.draggableContentTypeInPanel(contentType.name)}}" stepKey="waitForContentTypeInPanel"/>
+        <dragAndDrop selector1="{{PageBuilderPanel.draggableContentTypeInPanel(contentType.name)}}" selector2="{{PageBuilderStage.contentTypeContainer(containerTargetType.role, containerTargetIndex)}}" x="{{offsetXCoordinate}}" y="{{offsetYCoordinate}}" stepKey="dropContentTypeIntoStage"/>
+        <waitForPageLoad stepKey="waitForAnimation" time="30"/>
+        <waitForElementNotVisible time="10" selector="{{PageBuilderStage.contentTypeInStageByIndex(contentType.role, contentTypeIndex)}}" stepKey="waitForContentTypeNotOnStage" />
+        <dontSeeElementInDOM selector="{{PageBuilderStage.contentTypeInStageByIndex(contentType.role, contentTypeIndex)}}" stepKey="dontSeeElementInContainer" />
+    </actionGroup>
     <actionGroup name="dragContentTypeToContainerAndExpectEditPanel">
         <arguments>
             <argument name="contentType"/>
diff --git a/app/code/Magento/PageBuilder/Test/Mftf/Test/AdminPageBuilderRowTest.xml b/app/code/Magento/PageBuilder/Test/Mftf/Test/AdminPageBuilderRowTest.xml
@@ -1752,6 +1752,17 @@
         <actionGroup ref="validateOnHoverNoOptionMenu" stepKey="verifyHeadingHasNoOptionMenuAfterSave">
             <argument name="contentType" value="PageBuilderHeadingContentType"/>
         </actionGroup>
+        <!-- Verify we cannot drop content types into hidden containers -->
+        <comment userInput="Verify we cannot drop content types into hidden containers" stepKey="commentDropIntoContainer"/>
+        <actionGroup ref="expandPageBuilderPanelGroup" stepKey="expandPageBuilderPanelGroup1">
+            <argument name="group" value="PageBuilderHeadingContentType"/>
+        </actionGroup>
+        <actionGroup ref="dragContentTypeToHiddenContainer" stepKey="dragHeaderToRow2">
+            <argument name="contentType" value="PageBuilderHeadingContentType"/>
+            <argument name="contentTypeIndex" value="2"/>
+            <argument name="containerTargetType" value="PageBuilderRowContentType"/>
+            <argument name="containerTargetIndex" value="1"/>
+        </actionGroup>
         <!-- Validate store front -->
         <comment userInput="Validate content type is disabled on storefront" stepKey="commentValidateHiddenStorefront"/>
         <openNewTab stepKey="openNewTab" />
diff --git a/app/code/Magento/PageBuilder/Test/Mftf/Test/AdminPageBuilderTabGroupTests.xml b/app/code/Magento/PageBuilder/Test/Mftf/Test/AdminPageBuilderTabGroupTests.xml
@@ -896,6 +896,17 @@
         <actionGroup ref="validateOnHoverNoOptionMenu" stepKey="verifyHeadingHasNoOptionMenuAfterSave">
             <argument name="contentType" value="PageBuilderHeadingContentType"/>
         </actionGroup>
+        <!-- Verify we cannot drop content types into hidden containers -->
+        <comment userInput="Verify we cannot drop content types into hidden containers" stepKey="commentDropIntoContainer"/>
+        <actionGroup ref="expandPageBuilderPanelGroup" stepKey="expandPageBuilderPanelGroup1">
+            <argument name="group" value="PageBuilderHeadingContentType"/>
+        </actionGroup>
+        <actionGroup ref="dragContentTypeToHiddenContainer" stepKey="dragHeaderToTabs2">
+            <argument name="contentType" value="PageBuilderHeadingContentType"/>
+            <argument name="contentTypeIndex" value="2"/>
+            <argument name="containerTargetType" value="PageBuilderTabsContentType"/>
+            <argument name="containerTargetIndex" value="1"/>
+        </actionGroup>
         <!-- Validate store front -->
         <comment userInput="Validate content type is disabled on storefront" stepKey="commentValidateHiddenStorefront"/>
         <openNewTab stepKey="openNewTab" />
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/css/source/content-type/_preview.less b/app/code/Magento/PageBuilder/view/adminhtml/web/css/source/content-type/_preview.less
@@ -174,7 +174,7 @@
         transition: opacity 200ms, top 200ms, visibility 200ms;
         visibility: hidden;
         width: 100%;
-        z-index: 11;
+        z-index: 15;
     }
 }
 
@@ -596,4 +596,9 @@
         opacity: 1;
         visibility: visible;
     }
+    .pagebuilder-drop-indicator,
+    .pagebuilder-content-type.type-container.empty-container > .content-type-container:before {
+        opacity: 0 !important;
+        visibility: hidden !important;
+    }
 }
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/js/collection.js b/app/code/Magento/PageBuilder/view/adminhtml/web/js/collection.js
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/js/content-type-collection.js b/app/code/Magento/PageBuilder/view/adminhtml/web/js/content-type-collection.js
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/js/content-type/column-group/preview.js b/app/code/Magento/PageBuilder/view/adminhtml/web/js/content-type/column-group/preview.js
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/js/drag-drop/sortable.js b/app/code/Magento/PageBuilder/view/adminhtml/web/js/drag-drop/sortable.js
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/collection.ts b/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/collection.ts
@@ -4,8 +4,11 @@
  */
 
 import ko from "knockout";
+import ContentTypeCollectionInterface from "./content-type-collection.d";
+import ContentTypeInterface from "./content-type.d";
 import { moveArrayItemIntoArray, removeArrayItem } from "./utils/array";
 
+
 /**
  * @api
  */
@@ -19,9 +22,9 @@ export default class Collection {
     /**
      * Return the children of the current element
      *
-     * @returns {KnockoutObservableArray<ContentTypeInterface>}
+     * @returns {KnockoutObservableArray<ContentTypeInterface | ContentTypeCollectionInterface>}
      */
-    public getChildren(): KnockoutObservableArray<any> {
+    public getChildren(): KnockoutObservableArray<ContentTypeInterface | ContentTypeCollectionInterface> {
         return this.children;
     }
 
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type-collection.ts b/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type-collection.ts
@@ -34,19 +34,19 @@ export default class ContentTypeCollection extends ContentType implements Conten
     /**
      * Return the children of the current element
      *
-     * @returns {KnockoutObservableArray<ContentTypeInterface>}
+     * @returns {KnockoutObservableArray<ContentTypeInterface | ContentTypeCollectionInterface>}
      */
-    public getChildren(): KnockoutObservableArray<ContentTypeInterface> {
+    public getChildren(): KnockoutObservableArray<ContentTypeInterface | ContentTypeCollectionInterface> {
         return this.collection.getChildren();
     }
 
     /**
      * Add a child into the observable array
      *
-     * @param child
-     * @param index
+     * @param {ContentTypeInterface | ContentTypeCollectionInterface} child
+     * @param {number} index
      */
-    public addChild(child: ContentTypeInterface, index?: number): void {
+    public addChild(child: ContentTypeInterface | ContentTypeCollectionInterface, index?: number): void {
         child.parent = this;
         this.collection.addChild(child, index);
 
@@ -71,11 +71,11 @@ export default class ContentTypeCollection extends ContentType implements Conten
      *
      * @param children
      */
-    public setChildren(children: KnockoutObservableArray<ContentTypeInterface>) {
+    public setChildren(children: KnockoutObservableArray<ContentTypeInterface | ContentTypeCollectionInterface>) {
         this.collection.setChildren(children);
     }
 
-    get children() {
+    get children(): KnockoutObservableArray<ContentTypeInterface | ContentTypeCollectionInterface> {
         return this.collection.getChildren();
     }
 }
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type/column-group/preview.ts b/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type/column-group/preview.ts
@@ -542,6 +542,10 @@ export default class Preview extends PreviewCollection {
     private initMouseMove(group: JQuery): void {
         let intersects: boolean = false;
         $(document).on("mousemove touchmove", (event: JQueryEventObject) => {
+            if (group.parents(".pagebuilder-content-type-hidden").length > 0) {
+                return;
+            }
+
             const groupPosition = this.getGroupPosition(group);
 
             // If we're handling a touch event we need to pass through the page X & Y
diff --git a/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/drag-drop/sortable.ts b/app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/drag-drop/sortable.ts
@@ -134,7 +134,9 @@ let placeholderContainer: Element;
  * @param {JQueryUI.SortableUIParams} ui
  */
 function onSort(preview: Preview, event: Event, ui: JQueryUI.SortableUIParams) {
-    if ($(this).sortable("option", "disabled")) {
+    if ($(this).sortable("option", "disabled") ||
+        ui.placeholder.parents(".pagebuilder-content-type-hidden").length > 0
+    ) {
         ui.placeholder.hide();
     } else {
         ui.placeholder.show();
@@ -183,7 +185,7 @@ function onSortReceive(preview: Preview, event: Event, ui: JQueryUI.SortableUIPa
     const contentTypeConfig = getDraggedContentTypeConfig();
     if (contentTypeConfig) {
         // If the sortable instance is disabled don't complete this operation
-        if ($(this).sortable("option", "disabled")) {
+        if ($(this).sortable("option", "disabled") || $(this).parents(".pagebuilder-content-type-hidden").length > 0) {
             return;
         }
 
@@ -232,8 +234,17 @@ function onSortReceive(preview: Preview, event: Event, ui: JQueryUI.SortableUIPa
  */
 function onSortUpdate(preview: Preview, event: Event, ui: JQueryUI.SortableUIParams) {
     // If the sortable instance is disabled don't complete this operation
-    if ($(this).sortable("option", "disabled")) {
+    if ($(this).sortable("option", "disabled") || ui.item.parents(".pagebuilder-content-type-hidden").length > 0) {
         ui.item.remove();
+        $(this).sortable("cancel");
+
+        // jQuery tries to reset the state but kills KO's bindings, so we'll force a re-render on the parent
+        if (ui.item.length > 0 && typeof ko.dataFor(ui.item[0]) !== "undefined") {
+            const parent = ko.dataFor(ui.item[0]).parent as ContentTypeCollectionInterface;
+            const children = parent.getChildren()().splice(0);
+            parent.getChildren()([]);
+            parent.getChildren()(children);
+        }
         return;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -174,7 +174,7 @@`
`174`	`174`	`transition: opacity 200ms, top 200ms, visibility 200ms;`
`175`	`175`	`visibility: hidden;`
`176`	`176`	`width: 100%;`
`177`		`- z-index: 11;`
	`177`	`+ z-index: 15;`
`178`	`178`	`}`
`179`	`179`	`}`
`180`	`180`
`@@ -596,4 +596,9 @@`
`596`	`596`	`opacity: 1;`
`597`	`597`	`visibility: visible;`
`598`	`598`	`}`
	`599`	`+ .pagebuilder-drop-indicator,`
	`600`	`+ .pagebuilder-content-type.type-container.empty-container > .content-type-container:before {`
	`601`	`+ opacity: 0 !important;`
	`602`	`+ visibility: hidden !important;`
	`603`	`+ }`
`599`	`604`	`}`
Original file line number	Diff line number	Diff line change
`@@ -34,19 +34,19 @@ export default class ContentTypeCollection extends ContentType implements Conten`
`34`	`34`	`/**`
`35`	`35`	`* Return the children of the current element`
`36`	`36`	`*`
`37`		`- * @returns {KnockoutObservableArray<ContentTypeInterface>}`
	`37`	`+ * @returns {KnockoutObservableArray<ContentTypeInterface \| ContentTypeCollectionInterface>}`
`38`	`38`	`*/`
`39`		`- public getChildren(): KnockoutObservableArray<ContentTypeInterface> {`
	`39`	`+ public getChildren(): KnockoutObservableArray<ContentTypeInterface \| ContentTypeCollectionInterface> {`
`40`	`40`	`return this.collection.getChildren();`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`/**`
`44`	`44`	`* Add a child into the observable array`
`45`	`45`	`*`
`46`		`- * @param child`
`47`		`- * @param index`
	`46`	`+ * @param {ContentTypeInterface \| ContentTypeCollectionInterface} child`
	`47`	`+ * @param {number} index`
`48`	`48`	`*/`
`49`		`- public addChild(child: ContentTypeInterface, index?: number): void {`
	`49`	`+ public addChild(child: ContentTypeInterface \| ContentTypeCollectionInterface, index?: number): void {`
`50`	`50`	`child.parent = this;`
`51`	`51`	`this.collection.addChild(child, index);`
`52`	`52`
`@@ -71,11 +71,11 @@ export default class ContentTypeCollection extends ContentType implements Conten`
`71`	`71`	`*`
`72`	`72`	`* @param children`
`73`	`73`	`*/`
`74`		`- public setChildren(children: KnockoutObservableArray<ContentTypeInterface>) {`
	`74`	`+ public setChildren(children: KnockoutObservableArray<ContentTypeInterface \| ContentTypeCollectionInterface>) {`
`75`	`75`	`this.collection.setChildren(children);`
`76`	`76`	`}`
`77`	`77`
`78`		`- get children() {`
	`78`	`+ get children(): KnockoutObservableArray<ContentTypeInterface \| ContentTypeCollectionInterface> {`
`79`	`79`	`return this.collection.getChildren();`
`80`	`80`	`}`
`81`	`81`	`}`