From 4c44823f12bdc2daace18654c479a8a43196d600 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskii Date: Fri, 24 Nov 2023 22:29:52 +0100 Subject: [PATCH] [DataGrid] Fix cell editing adding a leading "v" on paste (#11166) Co-authored-by: Prasad Kulkarni <49803589+prasad5795@users.noreply.github.com> Co-authored-by: Olivier Tassinari --- .circleci/config.yml | 10 +-- package.json | 2 +- .../tests/cellEditing.DataGridPro.test.tsx | 3 +- .../tests/editComponents.DataGridPro.test.tsx | 14 ---- .../src/tests/rowEditing.DataGridPro.test.tsx | 4 +- .../src/components/cell/GridEditDateCell.tsx | 19 +----- .../features/editing/useGridCellEditing.ts | 32 ++++------ .../features/editing/useGridRowEditing.ts | 29 ++++----- .../src/models/api/gridEditingApi.ts | 6 +- .../src/models/params/gridEditCellParams.ts | 2 + .../src/models/params/gridRowParams.ts | 1 + .../fixtures/DataGrid/KeyboardEditNumber.tsx | 33 ++++++++++ test/e2e/index.test.ts | 64 ++++++++++++++++++- yarn.lock | 28 ++++---- 14 files changed, 148 insertions(+), 99 deletions(-) create mode 100644 test/e2e/fixtures/DataGrid/KeyboardEditNumber.tsx diff --git a/.circleci/config.yml b/.circleci/config.yml index 4b43745318f49..8665d26c7927f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -195,7 +195,7 @@ jobs: test_browser: <<: *defaults docker: - - image: mcr.microsoft.com/playwright:v1.39.0-focal + - image: mcr.microsoft.com/playwright:v1.40.0-focal environment: NODE_ENV: development # Needed if playwright is in `devDependencies` steps: @@ -228,7 +228,7 @@ jobs: test_e2e: <<: *defaults docker: - - image: mcr.microsoft.com/playwright:v1.39.0-focal + - image: mcr.microsoft.com/playwright:v1.40.0-focal environment: NODE_ENV: development # Needed if playwright is in `devDependencies` steps: @@ -241,7 +241,7 @@ jobs: test_e2e_website: <<: *defaults docker: - - image: mcr.microsoft.com/playwright:v1.39.0-focal + - image: mcr.microsoft.com/playwright:v1.40.0-focal environment: NODE_ENV: development # Needed if playwright is in `devDependencies` steps: @@ -256,7 +256,7 @@ jobs: test_regressions: <<: *defaults docker: - - image: mcr.microsoft.com/playwright:v1.39.0-focal + - image: mcr.microsoft.com/playwright:v1.40.0-focal environment: NODE_ENV: development # Needed if playwright is in `devDependencies` steps: @@ -272,7 +272,7 @@ jobs: run_danger: <<: *defaults docker: - - image: mcr.microsoft.com/playwright:v1.39.0-focal + - image: mcr.microsoft.com/playwright:v1.40.0-focal environment: NODE_ENV: development # Needed if playwright is in `devDependencies` steps: diff --git a/package.json b/package.json index a338193565950..f7b5b8142e254 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,7 @@ "@next/eslint-plugin-next": "^13.5.6", "@octokit/plugin-retry": "^6.0.1", "@octokit/rest": "^20.0.2", - "@playwright/test": "1.39.0", + "@playwright/test": "1.40.0", "@testing-library/react": "^14.0.0", "@types/babel__core": "^7.20.3", "@types/chai": "^4.3.9", diff --git a/packages/grid/x-data-grid-pro/src/tests/cellEditing.DataGridPro.test.tsx b/packages/grid/x-data-grid-pro/src/tests/cellEditing.DataGridPro.test.tsx index 7aa921856e0f9..18101263ecc38 100644 --- a/packages/grid/x-data-grid-pro/src/tests/cellEditing.DataGridPro.test.tsx +++ b/packages/grid/x-data-grid-pro/src/tests/cellEditing.DataGridPro.test.tsx @@ -186,7 +186,6 @@ describe(' - Cell editing', () => { error: false, isProcessingProps: true, changeReason: 'setEditCellValue', - unstable_updateValueOnRender: false, }); }); @@ -917,7 +916,7 @@ describe(' - Cell editing', () => { expect(spiedStartCellEditMode.lastCall.args[0]).to.deep.equal({ id: 0, field: 'currencyPair', - initialValue: 'a', + deleteValue: true, }); }); diff --git a/packages/grid/x-data-grid-pro/src/tests/editComponents.DataGridPro.test.tsx b/packages/grid/x-data-grid-pro/src/tests/editComponents.DataGridPro.test.tsx index e1d80b9bfa54c..15f9d1928c5a8 100644 --- a/packages/grid/x-data-grid-pro/src/tests/editComponents.DataGridPro.test.tsx +++ b/packages/grid/x-data-grid-pro/src/tests/editComponents.DataGridPro.test.tsx @@ -248,20 +248,6 @@ describe(' - Edit components', () => { ); }); - it('should call setEditCellValue when entering the edit mode by pressing a digit', () => { - render(); - const spiedSetEditCellValue = spyApi(apiRef.current, 'setEditCellValue'); - - const cell = getCell(0, 0); - userEvent.mousePress(cell); - fireEvent.keyDown(cell, { key: '5' }); - - expect(spiedSetEditCellValue.lastCall.args[0].id).to.equal(0); - expect(spiedSetEditCellValue.lastCall.args[0].field).to.equal('createdAt'); - expect(spiedSetEditCellValue.lastCall.args[0].debounceMs).to.equal(undefined); - expect(spiedSetEditCellValue.lastCall.args[0].value).to.be.instanceOf(Date); - }); - it('should call setEditCellValue with null when entered an empty value', () => { render(); const spiedSetEditCellValue = spyApi(apiRef.current, 'setEditCellValue'); diff --git a/packages/grid/x-data-grid-pro/src/tests/rowEditing.DataGridPro.test.tsx b/packages/grid/x-data-grid-pro/src/tests/rowEditing.DataGridPro.test.tsx index a08ebe77322c7..a4e720b3692e7 100644 --- a/packages/grid/x-data-grid-pro/src/tests/rowEditing.DataGridPro.test.tsx +++ b/packages/grid/x-data-grid-pro/src/tests/rowEditing.DataGridPro.test.tsx @@ -233,7 +233,6 @@ describe(' - Row editing', () => { error: false, isProcessingProps: true, changeReason: 'setEditCellValue', - unstable_updateValueOnRender: false, }); const args2 = column2Props.preProcessEditCellProps.lastCall.args[0]; @@ -244,7 +243,6 @@ describe(' - Row editing', () => { value: 1, error: false, isProcessingProps: true, - unstable_updateValueOnRender: false, }); }); @@ -911,7 +909,7 @@ describe(' - Row editing', () => { expect(spiedStartRowEditMode.lastCall.args[0]).to.deep.equal({ id: 0, fieldToFocus: 'currencyPair', - initialValue: 'a', + deleteValue: true, }); }); diff --git a/packages/grid/x-data-grid/src/components/cell/GridEditDateCell.tsx b/packages/grid/x-data-grid/src/components/cell/GridEditDateCell.tsx index 26d39644bd13d..0e46299cb5c01 100644 --- a/packages/grid/x-data-grid/src/components/cell/GridEditDateCell.tsx +++ b/packages/grid/x-data-grid/src/components/cell/GridEditDateCell.tsx @@ -98,8 +98,6 @@ function GridEditDateCell(props: GridEditDateCellProps) { const ownerState = { classes: rootProps.classes }; const classes = useUtilityClasses(ownerState); - const hasUpdatedEditValueOnMount = React.useRef(false); - const parseValueToDate = React.useCallback((value: string) => { if (value === '') { return null; @@ -152,24 +150,9 @@ function GridEditDateCell(props: GridEditDateCellProps) { inputRef.current!.focus(); } }, [hasFocus]); - - const meta = apiRef.current.unstable_getEditCellMeta(id, field); - - const handleInputRef = (el: HTMLInputElement) => { - inputRef.current = el; - - if (meta?.unstable_updateValueOnRender && !hasUpdatedEditValueOnMount.current) { - const inputValue = inputRef.current.value; - const parsedDate = parseValueToDate(inputValue); - setValueState({ parsed: parsedDate, formatted: inputValue }); - apiRef.current.setEditCellValue({ id, field, value: parsedDate }); - hasUpdatedEditValueOnMount.current = true; - } - }; - return ( >( (params) => { - const { id, field, reason, key, colDef } = params; + const { id, field, reason } = params; const startCellEditModeParams: GridStartCellEditModeParams = { id, field }; - if (reason === GridCellEditStartReasons.printableKeyDown) { - if (React.version.startsWith('17')) { - // In React 17, cleaning the input is enough. - // The sequence of events makes the key pressed by the end-users update the textbox directly. - startCellEditModeParams.deleteValue = true; - } else { - const initialValue = colDef.valueParser ? colDef.valueParser(key) : key; - startCellEditModeParams.initialValue = initialValue; - } - } else if (reason === GridCellEditStartReasons.deleteKeyDown) { + if ( + reason === GridCellEditStartReasons.printableKeyDown || + reason === GridCellEditStartReasons.deleteKeyDown || + reason === GridCellEditStartReasons.pasteKeyDown + ) { startCellEditModeParams.deleteValue = true; } @@ -332,18 +329,14 @@ export const useGridCellEditing = ( const { id, field, deleteValue, initialValue } = params; let newValue = apiRef.current.getCellValue(id, field); - // eslint-disable-next-line @typescript-eslint/naming-convention - let unstable_updateValueOnRender = false; if (deleteValue || initialValue) { newValue = deleteValue ? '' : initialValue; - unstable_updateValueOnRender = true; } const newProps = { value: newValue, error: false, isProcessingProps: false, - unstable_updateValueOnRender, }; updateOrDeleteFieldState(id, field, newProps); @@ -522,7 +515,8 @@ export const useGridCellEditing = ( } }, [cellModesModelProp, updateCellModesModel]); - React.useEffect(() => { + // Run this effect synchronously so that the keyboard event can impact the yet-to-be-rendered input. + useEnhancedEffect(() => { const idToIdLookup = gridRowsDataRowIdToIdLookupSelector(apiRef); // Update the ref here because updateStateToStopCellEditMode may change it later diff --git a/packages/grid/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts b/packages/grid/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts index dfc6f29644ce9..1703b26e0f9d8 100644 --- a/packages/grid/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts +++ b/packages/grid/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts @@ -1,5 +1,8 @@ import * as React from 'react'; -import { unstable_useEventCallback as useEventCallback } from '@mui/utils'; +import { + unstable_useEventCallback as useEventCallback, + unstable_useEnhancedEffect as useEnhancedEffect, +} from '@mui/utils'; import { useGridApiEventHandler, useGridApiOptionHandler, @@ -257,7 +260,6 @@ export const useGridRowEditing = ( const newParams: GridRowEditStartParams = { ...rowParams, field: params.field, - key: event.key, reason, }; apiRef.current.publishEvent('rowEditStart', newParams, event); @@ -269,20 +271,14 @@ export const useGridRowEditing = ( const handleRowEditStart = React.useCallback>( (params) => { - const { id, field, reason, key, columns } = params; + const { id, field, reason } = params; const startRowEditModeParams: GridStartRowEditModeParams = { id, fieldToFocus: field }; - if (reason === GridRowEditStartReasons.printableKeyDown) { - if (React.version.startsWith('17')) { - // In React 17, cleaning the input is enough. - // The sequence of events makes the key pressed by the end-users update the textbox directly. - startRowEditModeParams.deleteValue = !!field; - } else { - const colDef = columns.find((col) => col.field === field)!; - startRowEditModeParams.initialValue = colDef.valueParser ? colDef.valueParser(key) : key; - } - } else if (reason === GridRowEditStartReasons.deleteKeyDown) { + if ( + reason === GridRowEditStartReasons.printableKeyDown || + reason === GridRowEditStartReasons.deleteKeyDown + ) { startRowEditModeParams.deleteValue = !!field; } @@ -430,18 +426,14 @@ export const useGridRowEditing = ( } let newValue = apiRef.current.getCellValue(id, field); - // eslint-disable-next-line @typescript-eslint/naming-convention - let unstable_updateValueOnRender = false; if (fieldToFocus === field && (deleteValue || initialValue)) { newValue = deleteValue ? '' : initialValue; - unstable_updateValueOnRender = true; } acc[field] = { value: newValue, error: false, isProcessingProps: false, - unstable_updateValueOnRender, }; return acc; @@ -710,7 +702,8 @@ export const useGridRowEditing = ( } }, [rowModesModelProp, updateRowModesModel]); - React.useEffect(() => { + // Run this effect synchronously so that the keyboard event can impact the yet-to-be-rendered input. + useEnhancedEffect(() => { const idToIdLookup = gridRowsDataRowIdToIdLookupSelector(apiRef); // Update the ref here because updateStateToStopRowEditMode may change it later diff --git a/packages/grid/x-data-grid/src/models/api/gridEditingApi.ts b/packages/grid/x-data-grid/src/models/api/gridEditingApi.ts index c2e64a3b953b1..d80b04c12f531 100644 --- a/packages/grid/x-data-grid/src/models/api/gridEditingApi.ts +++ b/packages/grid/x-data-grid/src/models/api/gridEditingApi.ts @@ -19,10 +19,6 @@ export type GridRowModesModel = Record; export interface GridEditCellMeta { changeReason?: 'debouncedSetEditCellValue' | 'setEditCellValue'; - /** - * Determines if `setEditCellValue` should be called on the first render to sync the value. - */ - unstable_updateValueOnRender?: boolean; } export interface GridEditingSharedApi { @@ -88,6 +84,7 @@ export interface GridStartCellEditModeParams { /** * The initial value for the field. * If `deleteValue` is also true, this value is not used. + * @deprecated No longer needed. */ initialValue?: any; } @@ -135,6 +132,7 @@ export interface GridStartRowEditModeParams { /** * The initial value for the given `fieldToFocus`. * If `deleteValue` is also true, this value is not used. + * @deprecated No longer needed. */ initialValue?: string; } diff --git a/packages/grid/x-data-grid/src/models/params/gridEditCellParams.ts b/packages/grid/x-data-grid/src/models/params/gridEditCellParams.ts index 0fc7246d44016..cba2b7e6f99ae 100644 --- a/packages/grid/x-data-grid/src/models/params/gridEditCellParams.ts +++ b/packages/grid/x-data-grid/src/models/params/gridEditCellParams.ts @@ -32,6 +32,7 @@ enum GridCellEditStartReasons { cellDoubleClick = 'cellDoubleClick', printableKeyDown = 'printableKeyDown', deleteKeyDown = 'deleteKeyDown', + pasteKeyDown = 'pasteKeyDown', } /** @@ -45,6 +46,7 @@ export interface GridCellEditStartParams reason?: GridRowEditStartReasons; /** * If the reason is related to a keyboard event, it contains which key was pressed. + * @deprecated No longer needed. */ key?: string; } diff --git a/test/e2e/fixtures/DataGrid/KeyboardEditNumber.tsx b/test/e2e/fixtures/DataGrid/KeyboardEditNumber.tsx new file mode 100644 index 0000000000000..6d8f6a125804c --- /dev/null +++ b/test/e2e/fixtures/DataGrid/KeyboardEditNumber.tsx @@ -0,0 +1,33 @@ +import * as React from 'react'; +import { DataGrid } from '@mui/x-data-grid'; + +const baselineProps = { + rows: [{ id: 0, age: 40 }], + columns: [{ field: 'age', type: 'number', editable: true }], +}; + +export default function KeyboardEditNumber() { + const [updates, setUpdates] = React.useState([]); + return ( +
+
+ { + setUpdates((prev) => [...prev, newRow.age]); + return newRow; + }} + /> +
+
+ {updates.map((update, key) => { + return ( +
+ {typeof update} {update} +
+ ); + })} +
+
+ ); +} diff --git a/test/e2e/index.test.ts b/test/e2e/index.test.ts index c1df904d1448b..ff6f9bdc39b43 100644 --- a/test/e2e/index.test.ts +++ b/test/e2e/index.test.ts @@ -441,12 +441,74 @@ async function initializeEnvironment( ); await page.click('[role="cell"][data-field="brand"]'); - await page.type('[role="cell"][data-field="brand"]', 'p'); + await page.keyboard.press('p'); + if (browserType.name() === 'firefox') { + // In firefox the test fails without this + // It works fine when testing manually using the same firefox executable + await page.keyboard.insertText('p'); + } expect(await page.locator('[role="cell"][data-field="brand"] input').inputValue()).to.equal( 'p', ); }); + + // https://github.com/mui/mui-x/issues/9281 + it('should return a number value when editing with a digit key press', async () => { + await renderFixture('DataGrid/KeyboardEditNumber'); + + await page.click('[role="cell"][data-field="age"]'); + await page.keyboard.press('1'); + if (browserType.name() === 'firefox') { + // In firefox the test fails without this + // It works fine when testing manually using the same firefox executable + await page.keyboard.insertText('1'); + } + await page.keyboard.press('Enter'); + + expect(await page.getByTestId('new-value').textContent()).to.equal('number 1'); + }); + + // https://github.com/mui/mui-x/issues/10582 + it('should update input value when editing starts with `0` key press', async () => { + await renderFixture('DataGrid/KeyboardEditNumber'); + + await page.click('[role="cell"][data-field="age"]'); + await page.keyboard.press('0'); + if (browserType.name() === 'firefox') { + // In firefox the test fails without this + // It works fine when testing manually using the same firefox executable + await page.keyboard.insertText('0'); + } + + expect(await page.locator('[role="cell"][data-field="age"] input').inputValue()).to.equal( + '0', + ); + }); + + // https://github.com/mui/mui-x/issues/10582 + it('should update input value when editing starts with `-` key press', async () => { + await renderFixture('DataGrid/KeyboardEditNumber'); + + await page.click('[role="cell"][data-field="age"]'); + await page.keyboard.press('-'); + + expect(await page.locator('[role="cell"][data-field="age"] input').inputValue()).to.equal( + '', + ); + }); + + // https://github.com/mui/mui-x/issues/9195 + it('should not paste "v" on Ctrl+V press', async () => { + await renderFixture('DataGrid/KeyboardEditInput'); + + await page.click('[role="cell"][data-field="brand"]'); + await page.keyboard.press('Control+v'); + + expect( + await page.locator('[role="cell"][data-field="brand"] input').inputValue(), + ).not.to.equal('v'); + }); }); describe('', () => { diff --git a/yarn.lock b/yarn.lock index 14444f19754f7..1a15f30075aa0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2530,12 +2530,12 @@ picocolors "^1.0.0" tslib "^2.6.0" -"@playwright/test@1.39.0": - version "1.39.0" - resolved "https://registry.yarnpkg.com/@playwright/test/-/test-1.39.0.tgz#d10ba8e38e44104499e25001945f07faa9fa91cd" - integrity sha512-3u1iFqgzl7zr004bGPYiN/5EZpRUSFddQBra8Rqll5N0/vfpqlP9I9EXqAoGacuAbX6c9Ulg/Cjqglp5VkK6UQ== +"@playwright/test@1.40.0": + version "1.40.0" + resolved "https://registry.yarnpkg.com/@playwright/test/-/test-1.40.0.tgz#d06c506977dd7863aa16e07f2136351ecc1be6ed" + integrity sha512-PdW+kn4eV99iP5gxWNSDQCbhMaDVej+RXL5xr6t04nbKLCBwYtA046t7ofoczHOm8u6c+45hpDKQVZqtqwkeQg== dependencies: - playwright "1.39.0" + playwright "1.40.0" "@polka/url@^1.0.0-next.20": version "1.0.0-next.21" @@ -11440,17 +11440,17 @@ pkg-up@^3.1.0: dependencies: find-up "^3.0.0" -playwright-core@1.39.0: - version "1.39.0" - resolved "https://registry.yarnpkg.com/playwright-core/-/playwright-core-1.39.0.tgz#efeaea754af4fb170d11845b8da30b2323287c63" - integrity sha512-+k4pdZgs1qiM+OUkSjx96YiKsXsmb59evFoqv8SKO067qBA+Z2s/dCzJij/ZhdQcs2zlTAgRKfeiiLm8PQ2qvw== +playwright-core@1.40.0: + version "1.40.0" + resolved "https://registry.yarnpkg.com/playwright-core/-/playwright-core-1.40.0.tgz#82f61e5504cb3097803b6f8bbd98190dd34bdf14" + integrity sha512-fvKewVJpGeca8t0ipM56jkVSU6Eo0RmFvQ/MaCQNDYm+sdvKkMBBWTE1FdeMqIdumRaXXjZChWHvIzCGM/tA/Q== -playwright@1.39.0: - version "1.39.0" - resolved "https://registry.yarnpkg.com/playwright/-/playwright-1.39.0.tgz#184c81cd6478f8da28bcd9e60e94fcebf566e077" - integrity sha512-naE5QT11uC/Oiq0BwZ50gDmy8c8WLPRTEWuSSFVG2egBka/1qMoSqYQcROMT9zLwJ86oPofcTH2jBY/5wWOgIw== +playwright@1.40.0: + version "1.40.0" + resolved "https://registry.yarnpkg.com/playwright/-/playwright-1.40.0.tgz#2a1824b9fe5c4fe52ed53db9ea68003543a99df0" + integrity sha512-gyHAgQjiDf1m34Xpwzaqb76KgfzYrhK7iih+2IzcOCoZWr/8ZqmdBw+t0RU85ZmfJMgtgAiNtBQ/KS2325INXw== dependencies: - playwright-core "1.39.0" + playwright-core "1.40.0" optionalDependencies: fsevents "2.3.2"