Skip to content

Commit

Permalink
refactor: remove no longer needed internal-tab event workaround (#7870)
Browse files Browse the repository at this point in the history
  • Loading branch information
@web-padawan @vaadin-bot
web-padawan authored and vaadin-bot committed Sep 26, 2024
1 parent f903572 commit 9a1ae82
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 107 deletions.
1 change: 0 additions & 1 deletion packages/grid-pro/src/vaadin-grid-pro-edit-column-mixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ export const GridProEditColumnMixin = (superClass) =>
const editor = this._getEditorComponent(cell);
editor.addEventListener('focusout', this._grid.__boundEditorFocusOut);
editor.addEventListener('focusin', this._grid.__boundEditorFocusIn);
editor.addEventListener('internal-tab', this._grid.__boundCancelCellSwitch);
this._setEditorOptions(editor);
this._setEditorValue(editor, get(this.path, model.item));
editor._grid = this._grid;
Expand Down
14 changes: 2 additions & 12 deletions packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ export const InlineEditingMixin = (superClass) =>
this.__boundItemPropertyChanged = this._onItemPropertyChanged.bind(this);
this.__boundEditorFocusOut = this._onEditorFocusOut.bind(this);
this.__boundEditorFocusIn = this._onEditorFocusIn.bind(this);
this.__boundCancelCellSwitch = this._setCancelCellSwitch.bind(this);

this._addEditColumnListener('mousedown', (e) => {
// Prevent grid from resetting navigating state
Expand Down Expand Up @@ -305,8 +304,7 @@ export const InlineEditingMixin = (superClass) =>

/** @private */
_onEditorFocusOut(event) {
// Ignore focusout from internal tab event
if (this.__cancelCellSwitch || this.__shouldIgnoreFocusOut(event)) {
if (this.__shouldIgnoreFocusOut(event)) {
return;
}

Expand Down Expand Up @@ -416,20 +414,12 @@ export const InlineEditingMixin = (superClass) =>
}
}

/** @private */
_setCancelCellSwitch() {
this.__cancelCellSwitch = true;
window.requestAnimationFrame(() => {
this.__cancelCellSwitch = false;
});
}

/**
* @param {!KeyboardEvent} e
* @protected
*/
async _switchEditCell(e) {
if (this.__cancelCellSwitch || (e.defaultPrevented && e.keyCode === 9)) {
if (e.defaultPrevented && e.keyCode === 9) {
return;
}

Expand Down
28 changes: 1 addition & 27 deletions packages/grid-pro/test/keyboard-navigation.common.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from '@vaadin/chai-plugins';
import { fixtureSync, nextFrame } from '@vaadin/testing-helpers';
import { fixtureSync } from '@vaadin/testing-helpers';
import { sendKeys } from '@web/test-runner-commands';
import sinon from 'sinon';
import { createItems, dblclick, dragAndDropOver, flushGrid, getCellEditor, getContainerCell } from './helpers.js';
Expand Down Expand Up @@ -167,32 +167,6 @@ describe('keyboard navigation', () => {
expect(getCellEditor(firstCell)).to.be.ok;
});

it('should not focus cell next available for editing on Tab if `internal-tab` was fired right before it', async () => {
const firstCell = getContainerCell(grid.$.items, 1, 0);
dblclick(firstCell._content);

const editor = getCellEditor(firstCell);
editor.addEventListener('keydown', (e) => {
if (e.key === 'Tab') {
editor.dispatchEvent(new CustomEvent('internal-tab'));
}
});
await sendKeys({ press: 'Tab' });
expect(getCellEditor(firstCell)).to.be.ok;
});

it('should be possible to switch edit cell on Tab with delay after `internal-tab` was fired', async () => {
const firstCell = getContainerCell(grid.$.items, 1, 0);
dblclick(firstCell._content);

getCellEditor(firstCell).dispatchEvent(new CustomEvent('internal-tab'));
const secondCell = getContainerCell(grid.$.items, 1, 1);
await nextFrame();

await sendKeys({ press: 'Tab' });
expect(getCellEditor(secondCell)).to.be.ok;
});

it('should focus previous cell available for editing within a same row in edit mode on Shift Tab', async () => {
const firstCell = getContainerCell(grid.$.items, 1, 1);
dblclick(firstCell._content);
Expand Down
191 changes: 124 additions & 67 deletions test/integration/grid-pro-custom-editor.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,83 +2,107 @@ import { expect } from '@vaadin/chai-plugins';
import { fixtureSync, middleOfNode, nextRender } from '@vaadin/testing-helpers';
import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands';
import '@vaadin/combo-box';
import '@vaadin/custom-field';
import '@vaadin/date-picker';
import '@vaadin/date-time-picker';
import '@vaadin/grid-pro';
import '@vaadin/grid-pro/vaadin-grid-pro-edit-column.js';
import '@vaadin/text-field';
import '@vaadin/time-picker';
import { waitForOverlayRender } from '@vaadin/date-picker/test/helpers.js';
import { flushGrid, getContainerCell } from '@vaadin/grid-pro/test/helpers.js';

describe('grid-pro custom editor', () => {
let grid;
let grid, cell;

beforeEach(async () => {
function createGrid(path) {
grid = fixtureSync(`
<vaadin-grid-pro>
<vaadin-grid-pro-edit-column path="date" editor-type="custom" width="50px"></vaadin-grid-pro-edit-column>
<vaadin-grid-pro-edit-column path="status" editor-type="custom" width="50px"></vaadin-grid-pro-edit-column>
<vaadin-grid-pro-edit-column path="time" editor-type="custom" width="50px"></vaadin-grid-pro-edit-column>
<vaadin-grid-pro-edit-column path="datetime" editor-type="custom"></vaadin-grid-pro-edit-column>
<vaadin-grid-pro-edit-column path="${path}" editor-type="custom"></vaadin-grid-pro-edit-column>
</vaadin-gri-pro>
`);

const column = grid.querySelector(`[path="${path}"]`);
switch (path) {
case 'date':
column.editModeRenderer = (root, _, model) => {
root.innerHTML = `
<vaadin-date-picker value="${model.item.date}" auto-open-disabled></vaadin-date-picker>
`;
};
break;
case 'time':
column.editModeRenderer = (root, _, model) => {
root.innerHTML = `
<vaadin-time-picker value="${model.item.time}" auto-open-disabled></vaadin-time-picker>
`;
};
break;
case 'status':
column.editModeRenderer = (root, _, model) => {
root.innerHTML = `
<vaadin-combo-box
value="${model.item.status}"
items='["active", "suspended"]'
auto-open-disabled
></vaadin-combo-box>
`;
};
break;
case 'datetime':
column.editModeRenderer = (root, _, model) => {
root.innerHTML = `
<vaadin-date-time-picker value="${model.item.datetime}" auto-open-disabled>
</vaadin-date-time-picker>
`;
};
break;
case 'expires':
column.editModeRenderer = (root, _, model) => {
if (!root.firstChild) {
// NOTE: using `innerHTML` doesn't work due to the timing issue in custom-field
// See https://github.com/vaadin/web-components/issues/7871
const field = document.createElement('vaadin-custom-field');
field.appendChild(document.createElement('vaadin-text-field'));
field.appendChild(document.createElement('vaadin-text-field'));
root.appendChild(field);
}
root.firstChild.value = model.item.expires;
};
break;
default:
// Do nothing
}

grid.items = [
{ date: '1984-01-13', status: 'suspended', time: '09:00' },
{ date: '1977-07-12', status: 'active', time: '09:30' },
{ date: '1976-12-14', status: 'suspended', time: '10:00' },
{ date: '1972-12-04', status: 'active', time: '10:00' },
{ date: '1978-02-03', status: 'active', time: '10:00' },
{ date: '1984-01-13', status: 'suspended', time: '09:00', expires: '01\t25' },
{ date: '1977-07-12', status: 'active', time: '09:30', expires: '02\t26' },
{ date: '1976-12-14', status: 'suspended', time: '10:00', expires: '03\t27' },
{ date: '1972-12-04', status: 'active', time: '10:00', expires: '04\t28' },
{ date: '1978-02-03', status: 'active', time: '10:00', expires: '05\t29' },
].map((item) => {
return { ...item, datetime: `${item.date}T${item.time}` };
});

grid.querySelector('[path="date"]').editModeRenderer = (root, _, model) => {
root.innerHTML = `
<vaadin-date-picker value="${model.item.date}" auto-open-disabled>
</vaadin-date-picker>
`;
};

grid.querySelector('[path="status"]').editModeRenderer = (root, _, model) => {
if (!root.firstChild) {
const comboBox = document.createElement('vaadin-combo-box');
comboBox.autoOpenDisabled = true;
comboBox.items = ['active', 'suspended'];
root.appendChild(comboBox);
}
root.firstChild.value = model.item.status;
};

grid.querySelector('[path="time"]').editModeRenderer = (root, _, model) => {
root.innerHTML = `
<vaadin-time-picker value="${model.item.time}" auto-open-disabled></vaadin-time-picker>
`;
};

grid.querySelector('[path="datetime"]').editModeRenderer = (root, _, model) => {
root.innerHTML = `
<vaadin-date-time-picker value="${model.item.datetime}" auto-open-disabled>
</vaadin-date-time-picker>
`;
};

flushGrid(grid);
await nextRender();
});
return grid;
}

async function editFirstCell() {
cell = getContainerCell(grid.$.items, 0, 0);
cell.focus();
await sendKeys({ press: 'Enter' });
}

afterEach(async () => {
await resetMouse();
});

describe('date-picker', () => {
let cell;

beforeEach(async () => {
cell = getContainerCell(grid.$.items, 0, 0);
cell.focus();

await sendKeys({ press: 'Enter' });
grid = createGrid('date');
await nextRender();
await editFirstCell();
});

it('should apply the updated date to the cell when exiting on Tab', async () => {
Expand Down Expand Up @@ -145,13 +169,10 @@ describe('grid-pro custom editor', () => {
});

describe('combo-box', () => {
let cell;

beforeEach(async () => {
cell = getContainerCell(grid.$.items, 0, 1);
cell.focus();

await sendKeys({ press: 'Enter' });
grid = createGrid('status');
await nextRender();
await editFirstCell();
});

it('should apply the updated value to the cell when exiting on Tab', async () => {
Expand Down Expand Up @@ -181,13 +202,10 @@ describe('grid-pro custom editor', () => {
});

describe('time-picker', () => {
let cell;

beforeEach(async () => {
cell = getContainerCell(grid.$.items, 0, 2);
cell.focus();

await sendKeys({ press: 'Enter' });
grid = createGrid('time');
await nextRender();
await editFirstCell();
});

it('should apply the updated time to the cell when exiting on Tab', async () => {
Expand Down Expand Up @@ -217,13 +235,10 @@ describe('grid-pro custom editor', () => {
});

describe('date-time-picker', () => {
let cell;

beforeEach(async () => {
cell = getContainerCell(grid.$.items, 0, 3);
cell.focus();

await sendKeys({ press: 'Enter' });
grid = createGrid('datetime');
await nextRender();
await editFirstCell();
});

it('should not stop editing when switching between pickers using Tab', async () => {
Expand Down Expand Up @@ -280,4 +295,46 @@ describe('grid-pro custom editor', () => {
expect(editor.value).to.equal('1984-01-13T00:00');
});
});

describe('custom-field', () => {
beforeEach(async () => {
grid = createGrid('expires');
await nextRender();
await editFirstCell();
});

it('should not stop editing when switching between fields using mouse', async () => {
// Move focus to the second field
const { x, y } = middleOfNode(cell._content.querySelectorAll('input')[1]);
await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] });
await nextRender();
expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok;
});

it('should not stop editing when switching between fields using Tab', async () => {
// Move focus to the second field
await sendKeys({ press: 'Tab' });
await nextRender();
expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok;

// Move focus to the first field
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
await nextRender();
expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok;
});

it('should stop editing when moving focus outside the field using Tab', async () => {
// Move focus to the second field
await sendKeys({ press: 'Tab' });
await nextRender();
expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok;

// Move focus outside of the field
await sendKeys({ press: 'Tab' });
await nextRender();
expect(cell._content.querySelector('vaadin-custom-field')).to.be.not.ok;
});
});
});

0 comments on commit 9a1ae82

Please sign in to comment.