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

Jump to bottom

fix: wait for editor focusout on Tab to get updated value #7822

Merged
merged 5 commits into from
Sep 24, 2024
Merged

fix: wait for editor focusout on Tab to get updated value #7822

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
64ad712
fix: wait for editor focusout on Tab to get updated value
web-padawan Sep 19, 2024
a214f89
test: add integration test with date-picker as cell editor
web-padawan Sep 19, 2024
674b92c
test: add integration test with combo-box as cell editor
web-padawan Sep 19, 2024
3cb1752
test: use single beforeEach to reduce duplication
web-padawan Sep 19, 2024
16eb0b4
test: add integration test for time-picker in grid-pro
web-padawan Sep 24, 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
22 changes: 19 additions & 3 deletions packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,10 @@ export const InlineEditingMixin = (superClass) =>

/** @private */
_onEditorFocusOut() {
// Ignore focusout from internal tab event
if (this.__cancelCellSwitch) {
return;
}
Comment on lines +309 to +312
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the flag was only checked on keydown in _switchEditCell method. However, now when we wait for the native focusout it is necessary to also check it here, otherwise the debouncer will stop editing.

// Schedule stop on editor component focusout
this._debouncerStopEdit = Debouncer.debounce(this._debouncerStopEdit, animationFrame, this._stopEdit.bind(this));
}
Expand Down Expand Up @@ -424,7 +428,7 @@ export const InlineEditingMixin = (superClass) =>
* @param {!KeyboardEvent} e
* @protected
*/
_switchEditCell(e) {
async _switchEditCell(e) {
if (this.__cancelCellSwitch || (e.defaultPrevented && e.keyCode === 9)) {
return;
}
Expand All @@ -434,11 +438,23 @@ export const InlineEditingMixin = (superClass) =>
const editableColumns = this._getEditColumns();
const { cell, column, model } = this.__edited;

this._stopEdit();
e.preventDefault();
// Prevent vaadin-grid handler from being called
e.stopImmediatePropagation();

const editor = column._getEditorComponent(cell);

// Do not prevent Tab to allow native input blur and wait for it,
// unless the keydown event is from the edit cell select overlay.
if (e.key === 'Tab' && editor && editor.contains(e.target)) {
await new Promise((resolve) => {
editor.addEventListener('focusout', () => resolve(), { once: true });
});
} else {
e.preventDefault();
}

this._stopEdit();

// Try to find the next editable cell
let nextIndex = model.index;
let nextColumn = column;
Expand Down
1 change: 1 addition & 0 deletions packages/grid-pro/test/edit-column-lit.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import './not-animated-styles.js';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have "select column" tests in this suite, I though it would be good to disable select animation.

import '../theme/lumo/vaadin-lit-grid-pro.js';
import '../theme/lumo/vaadin-lit-grid-pro-edit-column.js';
import './edit-column.common.js';
1 change: 1 addition & 0 deletions packages/grid-pro/test/edit-column-polymer.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import './not-animated-styles.js';
import '../theme/lumo/vaadin-grid-pro.js';
import '../theme/lumo/vaadin-grid-pro-edit-column.js';
import './edit-column.common.js';
132 changes: 78 additions & 54 deletions packages/grid-pro/test/edit-column.common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from '@vaadin/chai-plugins';
import { enter, esc, fixtureSync, focusin, focusout, isIOS, nextFrame, tab } from '@vaadin/testing-helpers';
import { enter, esc, fixtureSync, focusin, focusout, nextFrame } from '@vaadin/testing-helpers';
import { sendKeys } from '@web/test-runner-commands';
import sinon from 'sinon';
import {
createItems,
Expand All @@ -20,8 +21,8 @@ function indexNameRenderer(root, _, { index, item }) {
}

describe('edit column', () => {
(isIOS ? describe.skip : describe)('select column', () => {
let grid, textCell, selectCell, checkboxCell;
describe('select column', () => {
let grid, textCell, selectCell, ageCell;

beforeEach(async () => {
grid = fixtureSync(`
Expand All @@ -37,7 +38,7 @@ describe('edit column', () => {
flushGrid(grid);
textCell = getContainerCell(grid.$.items, 1, 0);
selectCell = getContainerCell(grid.$.items, 1, 1);
checkboxCell = getContainerCell(grid.$.items, 1, 2);
ageCell = getContainerCell(grid.$.items, 1, 2);
await nextFrame();
});

Expand All @@ -47,27 +48,31 @@ describe('edit column', () => {
await nextFrame();

// Press Tab to edit the select cell
tab(document.activeElement);
await sendKeys({ press: 'Tab' });
expect(getCellEditor(selectCell)).to.be.ok;
await nextFrame();

// Press Tab to edit the checkbox cell
tab(document.activeElement);
expect(getCellEditor(checkboxCell)).to.be.ok;
// Press Tab to edit the age cell
await sendKeys({ press: 'Tab' });
expect(getCellEditor(ageCell)).to.be.ok;
});

it('should focus previous cell available for editing in edit mode on Shift Tab', async () => {
dblclick(checkboxCell._content);
expect(getCellEditor(checkboxCell)).to.be.ok;
dblclick(ageCell._content);
expect(getCellEditor(ageCell)).to.be.ok;
await nextFrame();

// Press Shift + Tab to edit the select cell
tab(document.activeElement, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
expect(getCellEditor(selectCell)).to.be.ok;
await nextFrame();

// Press Shift + Tab to edit the text cell
tab(document.activeElement, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
expect(getCellEditor(textCell)).to.be.ok;
});
});
Expand Down Expand Up @@ -113,7 +118,7 @@ describe('edit column', () => {
});

describe('horizontal scrolling to cell', () => {
let grid, input;
let grid;

beforeEach(() => {
grid = fixtureSync(`
Expand All @@ -130,26 +135,24 @@ describe('edit column', () => {
flushGrid(grid);
});

it('should scroll to the right on tab when editable cell is outside the viewport', () => {
it('should scroll to the right on tab when editable cell is outside the viewport', async () => {
const firstCell = getContainerCell(grid.$.items, 1, 0);
dblclick(firstCell._content);
input = getCellEditor(firstCell);
tab(input);

await sendKeys({ press: 'Tab' });

expect(grid.$.table.scrollLeft).to.be.at.least(100);
});

it('should scroll to the left on tab when editable cell is outside the viewport', (done) => {
it('should scroll to the left on tab when editable cell is outside the viewport', async () => {
const firstCell = getContainerCell(grid.$.items, 1, 1);
dblclick(firstCell._content);

setTimeout(() => {
input = getCellEditor(firstCell);
tab(input, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });

expect(grid.$.table.scrollLeft).to.closeTo(1, 1);
done();
});
expect(grid.$.table.scrollLeft).to.closeTo(1, 1);
});
});

Expand Down Expand Up @@ -429,78 +432,91 @@ describe('edit column', () => {
});

describe('with Tab', () => {
it('should skip non-editable cells when navigating with Tab', () => {
it('should skip non-editable cells when navigating with Tab', async () => {
let cell = getContainerCell(grid.$.items, 1, 2);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;

tab(cell);
await sendKeys({ press: 'Tab' });
cell = getContainerCell(grid.$.items, 1, 3);
expect(getCellEditor(cell)).to.be.ok;

// Should skip non-editable row
tab(cell);
await sendKeys({ press: 'Tab' });
cell = getContainerCell(grid.$.items, 3, 1);
expect(getCellEditor(cell)).to.be.ok;

tab(cell);
await sendKeys({ press: 'Tab' });
cell = getContainerCell(grid.$.items, 3, 2);
expect(getCellEditor(cell)).to.be.ok;
});

it('should skip non-editable cells when navigating with Shift-Tab', () => {
it('should skip non-editable cells when navigating with Shift-Tab', async () => {
let cell = getContainerCell(grid.$.items, 3, 2);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;

tab(cell, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
cell = getContainerCell(grid.$.items, 3, 1);
expect(getCellEditor(cell)).to.be.ok;

// Should skip non-editable rows
tab(cell, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
cell = getContainerCell(grid.$.items, 1, 3);
expect(getCellEditor(cell)).to.be.ok;

tab(cell, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
cell = getContainerCell(grid.$.items, 1, 2);
expect(getCellEditor(cell)).to.be.ok;
});

it('should skip cells that become non-editable after editing current cell', () => {
it('should skip cells that become non-editable after editing current cell', async () => {
// Edit status in row 2 to be completed, so none of the cells in this
// row should be editable anymore
let cell = getContainerCell(grid.$.items, 1, 1);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
const input = getCellEditor(cell);
input.value = 'completed';
tab(cell);
await sendKeys({ press: 'Tab' });

// Should skip to row 4
cell = getContainerCell(grid.$.items, 3, 1);
expect(getCellEditor(cell)).to.be.ok;
});

it('should stop editing and focus last edited cell if there are no more editable cells with Tab', () => {
it('should stop editing and focus last edited cell if there are no more editable cells with Tab', async () => {
const cell = getContainerCell(grid.$.items, 3, 3);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.be.ok;

tab(cell);
await sendKeys({ press: 'Tab' });
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.not.be.ok;
const target = cell._focusButton || cell;
expect(grid.shadowRoot.activeElement).to.equal(target);
expect(grid.hasAttribute('navigating')).to.be.true;
});

it('should stop editing and focus last edited cell if there are no more editable cells with Shift-Tab', () => {
it('should stop editing and focus last edited cell if there are no more editable cells with Shift-Tab', async () => {
const cell = getContainerCell(grid.$.items, 1, 1);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.be.ok;

tab(cell, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.not.be.ok;
const target = cell._focusButton || cell;
expect(grid.shadowRoot.activeElement).to.equal(target);
Expand All @@ -513,46 +529,54 @@ describe('edit column', () => {
grid.enterNextRow = true;
});

it('should skip non-editable cells when navigating with Enter', () => {
it('should skip non-editable cells when navigating with Enter', async () => {
let cell = getContainerCell(grid.$.items, 1, 1);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;

enter(cell);
await sendKeys({ press: 'Enter' });
cell = getContainerCell(grid.$.items, 3, 1);
expect(getCellEditor(cell)).to.be.ok;
});

it('should skip non-editable cells when navigating with Shift-Enter', () => {
it('should skip non-editable cells when navigating with Shift-Enter', async () => {
let cell = getContainerCell(grid.$.items, 3, 1);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;

enter(cell, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Enter' });
await sendKeys({ up: 'Shift' });
cell = getContainerCell(grid.$.items, 1, 1);
expect(getCellEditor(cell)).to.be.ok;
});

it('should stop editing and focus last edited cell if there are no more editable cells with Enter', () => {
it('should stop editing and focus last edited cell if there are no more editable cells with Enter', async () => {
const cell = getContainerCell(grid.$.items, 3, 1);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.be.ok;

enter(cell);
await sendKeys({ press: 'Enter' });
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.not.be.ok;
const target = cell._focusButton || cell;
expect(grid.shadowRoot.activeElement).to.equal(target);
expect(grid.hasAttribute('navigating')).to.be.true;
});

it('should stop editing and focus last edited cell if there are no more editable cells with Shift-Enter', () => {
it('should stop editing and focus last edited cell if there are no more editable cells with Shift-Enter', async () => {
const cell = getContainerCell(grid.$.items, 1, 1);
enter(cell);
cell.focus();
await sendKeys({ press: 'Enter' });
expect(getCellEditor(cell)).to.be.ok;
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.be.ok;

enter(cell, ['shift']);
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Enter' });
await sendKeys({ up: 'Shift' });
expect(grid.querySelector('vaadin-grid-pro-edit-text-field')).to.not.be.ok;
const target = cell._focusButton || cell;
expect(grid.shadowRoot.activeElement).to.equal(target);
Expand Down
Loading
Loading