Fix unintentional page rows being created (#3221)

mui · Feb 19, 2024 · 7cece81 · 7cece81
1 parent 14b6a6a
commit 7cece81
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 10 deletions.
diff --git a/packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/RenderOverlay.tsx b/packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/RenderOverlay.tsx
@@ -1019,7 +1019,7 @@ export default function RenderOverlay({ bridge }: RenderOverlayProps) {
             }
 
             if ([DROP_ZONE_RIGHT, DROP_ZONE_LEFT].includes(dragOverZone)) {
-              if (isOriginalParentLayout || !isDraggingOverHorizontalContainer) {
+              if (!isDraggingOverHorizontalContainer) {
                 const hasNewPageRow = isOriginalParentLayout || isOriginalParentColumn;
 
                 if (hasNewPageRow) {
@@ -1064,11 +1064,7 @@ export default function RenderOverlay({ bridge }: RenderOverlayProps) {
                 }
               }
 
-              if (
-                dragOverSlotParentProp &&
-                !isOriginalParentLayout &&
-                isDraggingOverHorizontalContainer
-              ) {
+              if (dragOverSlotParentProp && isDraggingOverHorizontalContainer) {
                 const isDraggingOverDirectionStart =
                   dragOverZone ===
                   (dragOverSlot?.flowDirection === 'row' ? DROP_ZONE_LEFT : DROP_ZONE_RIGHT);

diff --git a/test/models/ToolpadEditor.ts b/test/models/ToolpadEditor.ts
@@ -107,7 +107,13 @@ export class ToolpadEditor {
     await setTimeout(100);
   }
 
-  async dragTo(sourceLocator: Locator, moveTargetX: number, moveTargetY: number, hasDrop = true) {
+  async dragTo(
+    sourceLocator: Locator,
+    moveTargetX: number,
+    moveTargetY: number,
+    hasDrop = true,
+    steps = 10,
+  ) {
     const sourceBoundingBox = await sourceLocator.boundingBox();
 
     await this.page.mouse.move(
@@ -118,7 +124,7 @@ export class ToolpadEditor {
 
     await this.page.mouse.down();
 
-    await this.page.mouse.move(moveTargetX, moveTargetY, { steps: 10 });
+    await this.page.mouse.move(moveTargetX, moveTargetY, { steps });
 
     if (hasDrop) {
       await this.page.mouse.up();
@@ -134,6 +140,7 @@ export class ToolpadEditor {
     moveTargetX?: number,
     moveTargetY?: number,
     hasDrop = true,
+    steps?: number,
   ) {
     const style = await this.page.addStyleTag({ content: `* { transition: none !important; }` });
 
@@ -155,7 +162,7 @@ export class ToolpadEditor {
     const sourceLocator = this.getComponentCatalogItem(componentName);
     await expect(sourceLocator).toBeVisible();
 
-    await this.dragTo(sourceLocator, moveTargetX!, moveTargetY!, hasDrop);
+    await this.dragTo(sourceLocator, moveTargetX!, moveTargetY!, hasDrop, steps);
 
     await style.evaluate((elm) => elm.parentNode?.removeChild(elm));
   }

diff --git a/test/visual/components/fixture/toolpad/components/FullWidth.tsx b/test/visual/components/fixture/toolpad/components/FullWidth.tsx
@@ -0,0 +1,9 @@
+import * as React from 'react';
+import { Box } from '@mui/material';
+import { createComponent } from '@mui/toolpad/browser';
+
+function FullWidth() {
+  return <Box sx={{ width: '100%' }}>fullwidth</Box>;
+}
+
+export default createComponent(FullWidth);
diff --git a/test/visual/components/fixture/toolpad/pages/blank/page.yml b/test/visual/components/fixture/toolpad/pages/blank/page.yml
@@ -0,0 +1,6 @@
+apiVersion: v1
+kind: page
+spec:
+  id: QYjq2fJ
+  title: blank
+  display: shell
diff --git a/test/visual/components/index.spec.ts b/test/visual/components/index.spec.ts
@@ -30,7 +30,7 @@ test('rendering components in the app runtime', async ({ page, argosScreenshot }
 
 test('rendering components in the app editor', async ({ page, argosScreenshot }) => {
   const editorModel = new ToolpadEditor(page);
-  await editorModel.goto();
+  await editorModel.goToPage('components');
 
   await editorModel.waitForOverlay();
 
@@ -42,6 +42,46 @@ test('rendering components in the app editor', async ({ page, argosScreenshot })
   await argosScreenshot('with-selection');
 });
 
+test('building layouts', async ({ page, argosScreenshot }) => {
+  const editorModel = new ToolpadEditor(page);
+  await editorModel.goToPage('blank');
+
+  await editorModel.waitForOverlay();
+
+  const getNthFullWidthBoundingBox = (
+    n: number,
+  ): Promise<{ x: number; y: number; width: number; height: number } | null> =>
+    editorModel.appCanvas.getByText('fullwidth').nth(n).boundingBox();
+
+  await editorModel.dragNewComponentToCanvas('FullWidth');
+
+  await argosScreenshot('building-layout-1');
+
+  const firstFullWidthBoundingBox = await getNthFullWidthBoundingBox(0);
+
+  // Place inside right of first element
+  await editorModel.dragNewComponentToCanvas(
+    'FullWidth',
+    firstFullWidthBoundingBox!.x + (2 / 3) * firstFullWidthBoundingBox!.width,
+    firstFullWidthBoundingBox!.y + firstFullWidthBoundingBox!.height / 2,
+  );
+
+  await argosScreenshot('building-layout-2');
+
+  const secondFullWidthBoundingBox = await getNthFullWidthBoundingBox(1);
+
+  // Place outside right of second element
+  await editorModel.dragNewComponentToCanvas(
+    'FullWidth',
+    secondFullWidthBoundingBox!.x + secondFullWidthBoundingBox!.width + 12,
+    secondFullWidthBoundingBox!.y + secondFullWidthBoundingBox!.height / 2,
+    true,
+    1,
+  );
+
+  await argosScreenshot('building-layout-3');
+});
+
 test('showing grid while resizing elements', async ({ page, argosScreenshot }) => {
   const editorModel = new ToolpadEditor(page);
   await editorModel.goToPage('rows');