Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[data grid][pickers][tree-view] Fix shortcut with localization keyboard #14220

Merged
merged 5 commits into from
Oct 16, 2024
Merged

[data grid][pickers][tree-view] Fix shortcut with localization keyboard #14220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f76b916
fix copy paste cells with localization keyboard
rotembarsela Aug 15, 2024
7995105
polish
oliviertassinari Oct 8, 2024
02ac942
more fixes
oliviertassinari Oct 8, 2024
471311f
update test
oliviertassinari Oct 9, 2024
a3850fa
fix tests
oliviertassinari Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/x-data-grid-premium/src/tests/clipboard.DataGridPremium.test.tsx
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,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 @@ -901,7 +901,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 @@ -912,7 +912,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 @@ -922,7 +922,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,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 @@ -885,7 +885,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 @@ -905,7 +905,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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading