Skip to content

Commit

Permalink
[data grid][pickers][tree-view] Fix shortcut with localization keyboa…
Browse files Browse the repository at this point in the history 
…rd (#14220)

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
  • Loading branch information
@rotembarsela @oliviertassinari
rotembarsela and oliviertassinari authored Oct 16, 2024
1 parent a005076 commit 462de3e
Show file tree
Hide file tree
Showing 15 changed files with 199 additions and 81 deletions.
6 changes: 3 additions & 3 deletions packages/x-data-grid-premium/src/tests/clipboard.DataGridPremium.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe('<DataGridPremium /> - Clipboard', () => {
getData: () => pasteText,
};

fireEvent.keyDown(cell, { key: 'v', code: 'KeyV', keyCode: 86, ctrlKey: true }); // Ctrl+V
fireEvent.keyDown(cell, { key: 'v', keyCode: 86, ctrlKey: true }); // Ctrl+V
document.activeElement!.dispatchEvent(pasteEvent);
}

Expand All @@ -196,7 +196,7 @@ describe('<DataGridPremium /> - Clipboard', () => {
apiRef.current.subscribeEvent('cellEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'v', code: 'KeyV', keyCode: 86, [key]: true }); // Ctrl+V
fireEvent.keyDown(cell, { key: 'v', keyCode: 86, [key]: true }); // Ctrl+V
expect(listener.callCount).to.equal(0);
});
});
Expand All @@ -209,7 +209,7 @@ describe('<DataGridPremium /> - Clipboard', () => {
apiRef.current.subscribeEvent('rowEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'v', code: 'KeyV', keyCode: 86, [key]: true }); // Ctrl+V
fireEvent.keyDown(cell, { key: 'v', keyCode: 86, [key]: true }); // Ctrl+V
expect(listener.callCount).to.equal(0);
});
});
Expand Down
8 changes: 4 additions & 4 deletions packages/x-data-grid-pro/src/tests/cellEditing.DataGridPro.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ describe('<DataGridPro /> - Cell editing', () => {
apiRef.current.subscribeEvent('cellEditStart', listener);
const cell = getCell(0, 0);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'a' }); // A
fireEvent.keyDown(cell, { key: 'a', keyCode: 65 }); // A
expect(listener.callCount).to.equal(0);
});

Expand All @@ -935,7 +935,7 @@ describe('<DataGridPro /> - Cell editing', () => {
apiRef.current.subscribeEvent('cellEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'a', [key]: true }); // for example Ctrl + A, copy
fireEvent.keyDown(cell, { key: 'a', keyCode: 65, [key]: true }); // for example Ctrl + A, copy
expect(listener.callCount).to.equal(0);
});
});
Expand All @@ -946,7 +946,7 @@ describe('<DataGridPro /> - Cell editing', () => {
apiRef.current.subscribeEvent('cellEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'a', shiftKey: true }); // Print A in uppercase
fireEvent.keyDown(cell, { key: 'a', keyCode: 65, shiftKey: true }); // Print A in uppercase
expect(listener.callCount).to.equal(1);
});

Expand All @@ -956,7 +956,7 @@ describe('<DataGridPro /> - Cell editing', () => {
apiRef.current.subscribeEvent('cellEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'v', ctrlKey: true }); // Ctrl+V
fireEvent.keyDown(cell, { key: 'v', keyCode: 86, ctrlKey: true }); // Ctrl+V
expect(listener.callCount).to.equal(1);
});

Expand Down
6 changes: 3 additions & 3 deletions packages/x-data-grid-pro/src/tests/rowEditing.DataGridPro.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ describe('<DataGridPro /> - Row editing', () => {
apiRef.current.subscribeEvent('rowEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'a', [key]: true });
fireEvent.keyDown(cell, { key: 'a', keyCode: 65, [key]: true });
expect(listener.callCount).to.equal(0);
});
});
Expand All @@ -947,7 +947,7 @@ describe('<DataGridPro /> - Row editing', () => {
apiRef.current.subscribeEvent('rowEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'a', shiftKey: true });
fireEvent.keyDown(cell, { key: 'a', keyCode: 65, shiftKey: true });
expect(listener.callCount).to.equal(1);
});

Expand All @@ -967,7 +967,7 @@ describe('<DataGridPro /> - Row editing', () => {
apiRef.current.subscribeEvent('rowEditStart', listener);
const cell = getCell(0, 1);
fireUserEvent.mousePress(cell);
fireEvent.keyDown(cell, { key: 'v', ctrlKey: true });
fireEvent.keyDown(cell, { key: 'v', keyCode: 86, ctrlKey: true });
expect(listener.callCount).to.equal(1);
});

Expand Down
10 changes: 2 additions & 8 deletions packages/x-data-grid/src/hooks/features/clipboard/useGridClipboard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useGridApiOptionHandler, useGridNativeEventListener } from '../../utils
import { gridFocusCellSelector } from '../focus/gridFocusStateSelector';
import { serializeCellValue } from '../export/serializers/csvSerializer';
import type { DataGridProcessedProps } from '../../../models/props/DataGridProps';
import { isCopyShortcut } from '../../../utils/keyboardUtils';

function writeToClipboardPolyfill(data: string) {
const span = document.createElement('span');
Expand Down Expand Up @@ -74,14 +75,7 @@ export const useGridClipboard = (

const handleCopy = React.useCallback(
(event: KeyboardEvent) => {
if (
!(
(event.ctrlKey || event.metaKey) &&
event.key.toLowerCase() === 'c' &&
!event.shiftKey &&
!event.altKey
)
) {
if (!isCopyShortcut(event)) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/x-data-grid/src/hooks/features/rowSelection/useGridRowSelection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ export const useGridRowSelection = (
return;
}

if (event.key === 'a' && (event.ctrlKey || event.metaKey)) {
if (String.fromCharCode(event.keyCode) === 'A' && (event.ctrlKey || event.metaKey)) {
event.preventDefault();
selectRows(apiRef.current.getAllRowIds(), true);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/x-data-grid/src/internals/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export {
getActiveElement,
isEventTargetInPortal,
} from '../utils/domUtils';
export { isNavigationKey, isPasteShortcut } from '../utils/keyboardUtils';
export { isNavigationKey, isPasteShortcut, isCopyShortcut } from '../utils/keyboardUtils';
export * from '../utils/utils';
export { exportAs } from '../utils/exportAs';
export * from '../utils/getPublicApiRef';
Expand Down
24 changes: 18 additions & 6 deletions packages/x-data-grid/src/utils/keyboardUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,25 @@ export const isHideMenuKey = (key: React.KeyboardEvent['key']) => key === 'Tab'
// In theory, on macOS, ctrl + v doesn't trigger a paste, so the function should return false.
// However, maybe it's overkill to fix, so let's be lazy.
export function isPasteShortcut(event: React.KeyboardEvent) {
if (
return (
(event.ctrlKey || event.metaKey) &&
event.key.toLowerCase() === 'v' &&
// We can't use event.code === 'KeyV' as event.code assumes a QWERTY keyboard layout,
// for example, it would be another letter on a Dvorak physical keyboard.
// We can't use event.key === 'v' as event.key is not stable with key modifiers and keyboard layouts,
// for example, it would be ה on a Hebrew keyboard layout.
// https://github.com/w3c/uievents/issues/377 could be a long-term solution
String.fromCharCode(event.keyCode) === 'V' &&
!event.shiftKey &&
!event.altKey
) {
return true;
}
return false;
);
}

// Checks if the keyboard event corresponds to the copy shortcut (CTRL+C or CMD+C) across different localization keyboards.
export function isCopyShortcut(event: KeyboardEvent): boolean {
return (
(event.ctrlKey || event.metaKey) &&
String.fromCharCode(event.keyCode) === 'C' &&
!event.shiftKey &&
!event.altKey
);
}
48 changes: 36 additions & 12 deletions ...ickers-pro/src/SingleInputDateRangeField/tests/editing.SingleInputDateRangeField.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(view.getActiveSection(0), { key: 'a', ctrlKey: true });
fireEvent.keyDown(view.getActiveSection(0), {
key: 'a',
keyCode: 65,
ctrlKey: true,
});

fireEvent.keyDown(view.getSectionsContainer(), { key: 'Delete' });
expectFieldValueV7(view.getSectionsContainer(), 'MMMM YYYY – MMMM YYYY');
Expand All @@ -40,7 +44,7 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(input, { key: 'a', ctrlKey: true });
fireEvent.keyDown(input, { key: 'a', keyCode: 65, ctrlKey: true });

fireEvent.keyDown(input, { key: 'Delete' });
expectFieldValueV6(input, 'MMMM YYYY – MMMM YYYY');
Expand All @@ -60,7 +64,11 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
expectFieldValueV7(view.getSectionsContainer(), 'January YYYY – MMMM YYYY');

// Select all sections
fireEvent.keyDown(view.getActiveSection(0), { key: 'a', ctrlKey: true });
fireEvent.keyDown(view.getActiveSection(0), {
key: 'a',
keyCode: 65,
ctrlKey: true,
});

fireEvent.keyDown(view.getSectionsContainer(), { key: 'Delete' });
expectFieldValueV7(view.getSectionsContainer(), 'MMMM YYYY – MMMM YYYY');
Expand All @@ -83,7 +91,7 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
expectFieldValueV6(input, 'January YYYY – MMMM YYYY');

// Select all sections
fireEvent.keyDown(input, { key: 'a', ctrlKey: true });
fireEvent.keyDown(input, { key: 'a', keyCode: 65, ctrlKey: true });

fireEvent.keyDown(input, { key: 'Delete' });
expectFieldValueV6(input, 'MMMM YYYY – MMMM YYYY');
Expand All @@ -102,7 +110,11 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(view.getActiveSection(0), { key: 'a', ctrlKey: true });
fireEvent.keyDown(view.getActiveSection(0), {
key: 'a',
keyCode: 65,
ctrlKey: true,
});

fireEvent.keyDown(view.getSectionsContainer(), { key: 'Delete' });
expect(onChangeV7.callCount).to.equal(0);
Expand All @@ -122,7 +134,7 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(input, { key: 'a', ctrlKey: true });
fireEvent.keyDown(input, { key: 'a', keyCode: 65, ctrlKey: true });

fireEvent.keyDown(input, { key: 'Delete' });
expect(onChangeV6.callCount).to.equal(0);
Expand Down Expand Up @@ -262,7 +274,11 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(view.getActiveSection(0), { key: 'a', ctrlKey: true });
fireEvent.keyDown(view.getActiveSection(0), {
key: 'a',
keyCode: 65,
ctrlKey: true,
});

view.pressKey(null, '');
expectFieldValueV7(view.getSectionsContainer(), 'MMMM YYYY – MMMM YYYY');
Expand All @@ -280,7 +296,7 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(input, { key: 'a', ctrlKey: true });
fireEvent.keyDown(input, { key: 'a', keyCode: 65, ctrlKey: true });

fireEvent.change(input, { target: { value: '' } });
expectFieldValueV6(input, 'MMMM YYYY – MMMM YYYY');
Expand All @@ -300,7 +316,11 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
expectFieldValueV7(view.getSectionsContainer(), 'January YYYY – MMMM YYYY');

// Select all sections
fireEvent.keyDown(view.getActiveSection(0), { key: 'a', ctrlKey: true });
fireEvent.keyDown(view.getActiveSection(0), {
key: 'a',
keyCode: 65,
ctrlKey: true,
});

view.pressKey(null, '');
expectFieldValueV7(view.getSectionsContainer(), 'MMMM YYYY – MMMM YYYY');
Expand All @@ -323,7 +343,7 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
expectFieldValueV6(input, 'January YYYY – MMMM YYYY');

// Select all sections
fireEvent.keyDown(input, { key: 'a', ctrlKey: true });
fireEvent.keyDown(input, { key: 'a', keyCode: 65, ctrlKey: true });

fireEvent.change(input, { target: { value: '' } });
expectFieldValueV6(input, 'MMMM YYYY – MMMM YYYY');
Expand All @@ -342,7 +362,11 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(view.getActiveSection(0), { key: 'a', ctrlKey: true });
fireEvent.keyDown(view.getActiveSection(0), {
key: 'a',
keyCode: 65,
ctrlKey: true,
});

view.pressKey(null, '');
expect(onChangeV7.callCount).to.equal(0);
Expand All @@ -362,7 +386,7 @@ describe('<SingleInputDateRangeField /> - Editing', () => {
view.selectSection('month');

// Select all sections
fireEvent.keyDown(input, { key: 'a', ctrlKey: true });
fireEvent.keyDown(input, { key: 'a', keyCode: 65, ctrlKey: true });

fireEvent.change(input, { target: { value: 'Delete' } });
expect(onChangeV6.callCount).to.equal(0);
Expand Down
Loading

0 comments on commit 462de3e

Please sign in to comment.