From 8c64b11b64ff9bc15fcac2b91e3496628a40c082 Mon Sep 17 00:00:00 2001 From: Pedro Ferreira <10789765+apedroferreira@users.noreply.github.com> Date: Tue, 17 Oct 2023 18:05:59 +0100 Subject: [PATCH 1/6] Attempt to fix flaky tests --- test/utils/clickCenter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/utils/clickCenter.ts b/test/utils/clickCenter.ts index 95e17fd788b..ccb09f5a917 100644 --- a/test/utils/clickCenter.ts +++ b/test/utils/clickCenter.ts @@ -1,7 +1,7 @@ import { Page, Locator, expect } from '@playwright/test'; export default async function clickCenter(page: Page, targetLocator: Locator) { - await expect(targetLocator).toBeAttached(); + await expect(targetLocator).toBeVisible(); const targetBoundingBox = await targetLocator.boundingBox(); expect(targetBoundingBox).toBeTruthy(); From 8506c0bf92af853255b4beb3efa1cac99e3fbb52 Mon Sep 17 00:00:00 2001 From: Pedro Ferreira <10789765+apedroferreira@users.noreply.github.com> Date: Tue, 17 Oct 2023 19:41:19 +0100 Subject: [PATCH 2/6] Improve Datagrid --- packages/toolpad-components/src/DataGrid.tsx | 62 ++++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/packages/toolpad-components/src/DataGrid.tsx b/packages/toolpad-components/src/DataGrid.tsx index dda1fdecef6..829d47877bd 100644 --- a/packages/toolpad-components/src/DataGrid.tsx +++ b/packages/toolpad-components/src/DataGrid.tsx @@ -43,6 +43,7 @@ import { Typography, Tooltip, Popover, + Container, } from '@mui/material'; import { getObjectKey } from '@mui/toolpad-utils/objectKey'; import { errorFrom } from '@mui/toolpad-utils/errors'; @@ -586,6 +587,7 @@ const DataGridComponent = React.forwardRef(function DataGridComponent( rowsSource, dataProviderId, onRawRowsChange, + sx, ...props }: ToolpadDataGridProps, ref: React.ForwardedRef, @@ -725,40 +727,36 @@ const DataGridComponent = React.forwardRef(function DataGridComponent( return ( -
- - -
- - - -
-
+ + + + ); }); From 3a95cfdce737c81b3e93c8020bb409f194698a9c Mon Sep 17 00:00:00 2001 From: Pedro Ferreira <10789765+apedroferreira@users.noreply.github.com> Date: Tue, 17 Oct 2023 19:42:05 +0100 Subject: [PATCH 3/6] Remove unneeded assertion --- test/utils/clickCenter.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/utils/clickCenter.ts b/test/utils/clickCenter.ts index ccb09f5a917..b70e8f436c8 100644 --- a/test/utils/clickCenter.ts +++ b/test/utils/clickCenter.ts @@ -1,8 +1,6 @@ import { Page, Locator, expect } from '@playwright/test'; export default async function clickCenter(page: Page, targetLocator: Locator) { - await expect(targetLocator).toBeVisible(); - const targetBoundingBox = await targetLocator.boundingBox(); expect(targetBoundingBox).toBeTruthy(); From 5dd55eab20dbe7fa1533f12ac3207b9956b30d35 Mon Sep 17 00:00:00 2001 From: Pedro Ferreira <10789765+apedroferreira@users.noreply.github.com> Date: Thu, 19 Oct 2023 19:40:08 +0100 Subject: [PATCH 4/6] Wait for data to load in data grid drag column test --- test/integration/data-grid/basic.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/data-grid/basic.spec.ts b/test/integration/data-grid/basic.spec.ts index 1526b56aa36..767da1b4bb3 100644 --- a/test/integration/data-grid/basic.spec.ts +++ b/test/integration/data-grid/basic.spec.ts @@ -27,6 +27,9 @@ test('Column prop updates are not lost on drag interactions', async ({ page }) = const firstGridLocator = canvasGridLocator.first(); + // Wait for data to load so that datagrid bounding box is stable + await expect(editorModel.pageRoot.getByText('Todd Breitenberg')).toBeVisible(); + await clickCenter(page, firstGridLocator); await editorModel.componentEditor.locator('button:has-text("columns")').click(); From b9fc7895b79d62ff6e6cf6f28cd6bec47e2e46cf Mon Sep 17 00:00:00 2001 From: Pedro Ferreira <10789765+apedroferreira@users.noreply.github.com> Date: Thu, 19 Oct 2023 20:19:59 +0100 Subject: [PATCH 5/6] Add suggestion from Jan --- test/utils/clickCenter.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/utils/clickCenter.ts b/test/utils/clickCenter.ts index b70e8f436c8..5716bb9415f 100644 --- a/test/utils/clickCenter.ts +++ b/test/utils/clickCenter.ts @@ -1,8 +1,13 @@ import { Page, Locator, expect } from '@playwright/test'; export default async function clickCenter(page: Page, targetLocator: Locator) { - const targetBoundingBox = await targetLocator.boundingBox(); - expect(targetBoundingBox).toBeTruthy(); + let targetBoundingBox; + await expect(async () => { + targetBoundingBox = await targetLocator.boundingBox(); + expect(targetBoundingBox).toBeTruthy(); + expect(targetBoundingBox!.width).toBeGreaterThan(0); + expect(targetBoundingBox!.height).toBeGreaterThan(0); + }).toPass(); await page.mouse.click( targetBoundingBox!.x + targetBoundingBox!.width / 2, From f7cae053c9c7d3cf7e5c88bd7a9791113f75bdba Mon Sep 17 00:00:00 2001 From: Pedro Ferreira <10789765+apedroferreira@users.noreply.github.com> Date: Thu, 19 Oct 2023 21:40:28 +0100 Subject: [PATCH 6/6] Fix remaining test, use Box instead of Container in components, revert DataGrid component changes --- packages/toolpad-components/src/Chart.tsx | 8 +-- packages/toolpad-components/src/DataGrid.tsx | 62 ++++++++++---------- packages/toolpad-components/src/Form.tsx | 9 ++- test/models/ToolpadEditor.ts | 9 ++- 4 files changed, 47 insertions(+), 41 deletions(-) diff --git a/packages/toolpad-components/src/Chart.tsx b/packages/toolpad-components/src/Chart.tsx index c4953272d97..309545e7490 100644 --- a/packages/toolpad-components/src/Chart.tsx +++ b/packages/toolpad-components/src/Chart.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; -import { Container, ContainerProps, Skeleton } from '@mui/material'; +import { Box, BoxProps, Skeleton } from '@mui/material'; import { XAxis, @@ -37,7 +37,7 @@ function getBarChartDataSeriesNormalizedYKey(dataSeries: ChartDataSeries, index: return `${dataSeries.label}-${dataSeries.yKey}-${index}`; } -interface ChartProps extends ContainerProps { +interface ChartProps extends BoxProps { data?: ChartData; loading?: boolean; error?: Error | string; @@ -94,7 +94,7 @@ function Chart({ data = [], loading, error, height, sx }: ChartProps) { const isDataVisible = !loading && !displayError; return ( - + {isDataVisible ? ( @@ -196,7 +196,7 @@ function Chart({ data = [], loading, error, height, sx }: ChartProps) { height={height} /> ) : null} - + ); } diff --git a/packages/toolpad-components/src/DataGrid.tsx b/packages/toolpad-components/src/DataGrid.tsx index 829d47877bd..dda1fdecef6 100644 --- a/packages/toolpad-components/src/DataGrid.tsx +++ b/packages/toolpad-components/src/DataGrid.tsx @@ -43,7 +43,6 @@ import { Typography, Tooltip, Popover, - Container, } from '@mui/material'; import { getObjectKey } from '@mui/toolpad-utils/objectKey'; import { errorFrom } from '@mui/toolpad-utils/errors'; @@ -587,7 +586,6 @@ const DataGridComponent = React.forwardRef(function DataGridComponent( rowsSource, dataProviderId, onRawRowsChange, - sx, ...props }: ToolpadDataGridProps, ref: React.ForwardedRef, @@ -727,36 +725,40 @@ const DataGridComponent = React.forwardRef(function DataGridComponent( return ( - - - - - + + +
+ + + +
+ ); }); diff --git a/packages/toolpad-components/src/Form.tsx b/packages/toolpad-components/src/Form.tsx index feb5e26efcc..acd105c91b9 100644 --- a/packages/toolpad-components/src/Form.tsx +++ b/packages/toolpad-components/src/Form.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Container, ContainerProps, Box, Stack, BoxProps } from '@mui/material'; +import { Box, BoxProps, Stack } from '@mui/material'; import { LoadingButton } from '@mui/lab'; import { useNode } from '@mui/toolpad-core'; import { equalProperties } from '@mui/toolpad-utils/collections'; @@ -15,7 +15,7 @@ export const FormContext = React.createContext<{ fieldValues: {}, }); -interface FormProps extends ContainerProps { +interface FormProps extends BoxProps { value: FieldValues; onChange: (newValue: FieldValues) => void; onSubmit?: (data?: FieldValues) => unknown | Promise; @@ -39,7 +39,6 @@ function Form({ mode = 'onSubmit', hasChrome = true, sx, - ...rest }: FormProps) { const form = useForm({ mode }); const { isSubmitSuccessful } = form.formState; @@ -79,7 +78,7 @@ function Form({ return ( {hasChrome ? ( - +
{children} @@ -118,7 +117,7 @@ function Form({
- + ) : ( children )} diff --git a/test/models/ToolpadEditor.ts b/test/models/ToolpadEditor.ts index 3b3af30dece..ca583607107 100644 --- a/test/models/ToolpadEditor.ts +++ b/test/models/ToolpadEditor.ts @@ -137,7 +137,12 @@ export class ToolpadEditor { await this.componentCatalog.hover(); - const pageRootBoundingBox = await this.pageRoot.boundingBox(); + let pageRootBoundingBox; + await expect(async () => { + pageRootBoundingBox = await this.pageRoot.boundingBox(); + expect(pageRootBoundingBox).toBeTruthy(); + }).toPass(); + if (!moveTargetX) { moveTargetX = pageRootBoundingBox!.x + pageRootBoundingBox!.width / 2; } @@ -148,7 +153,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); await style.evaluate((elm) => elm.parentNode?.removeChild(elm)); }