Skip to content

Commit

Permalink
refactor: replace global focusin listener with focusout check (#7858) (…
Browse files Browse the repository at this point in the history 
…#7867)

Co-authored-by: Serhii Kulykov <[email protected]>
  • Loading branch information
@vaadin-bot @web-padawan
vaadin-bot and web-padawan authored Sep 26, 2024
1 parent cb501c6 commit f903572
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 150 deletions.
3 changes: 0 additions & 3 deletions 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 @@ -282,7 +282,6 @@ export const GridProEditColumnMixin = (superClass) =>
editor.addEventListener('focusout', this._grid.__boundEditorFocusOut);
editor.addEventListener('focusin', this._grid.__boundEditorFocusIn);
editor.addEventListener('internal-tab', this._grid.__boundCancelCellSwitch);
document.body.addEventListener('focusin', this._grid.__boundGlobalFocusIn);
this._setEditorOptions(editor);
this._setEditorValue(editor, get(this.path, model.item));
editor._grid = this._grid;
Expand All @@ -300,8 +299,6 @@ export const GridProEditColumnMixin = (superClass) =>
* @protected
*/
_stopCellEdit(cell, model) {
document.body.removeEventListener('focusin', this._grid.__boundGlobalFocusIn);

this._removeEditor(cell, model);
}
};
50 changes: 31 additions & 19 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 @@ -66,7 +66,6 @@ export const InlineEditingMixin = (superClass) =>
this.__boundEditorFocusOut = this._onEditorFocusOut.bind(this);
this.__boundEditorFocusIn = this._onEditorFocusIn.bind(this);
this.__boundCancelCellSwitch = this._setCancelCellSwitch.bind(this);
this.__boundGlobalFocusIn = this._onGlobalFocusIn.bind(this);

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

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

// Schedule stop on editor component focusout
this._debouncerStopEdit = Debouncer.debounce(this._debouncerStopEdit, animationFrame, this._stopEdit.bind(this));
}

/** @private */
_onEditorFocusIn() {
this._cancelStopEdit();
}

/** @private */
_onGlobalFocusIn(e) {
__shouldIgnoreFocusOut(event) {
const edited = this.__edited;
if (edited) {
// Detect focus moving to e.g. vaadin-select-overlay
const overlay = Array.from(e.composedPath()).filter(
(node) => node.nodeType === Node.ELEMENT_NODE && /^vaadin-(?!dialog).*-overlay$/iu.test(node.localName),
)[0];

if (overlay) {
overlay.addEventListener('vaadin-overlay-outside-click', this.__boundEditorFocusOut);
this._cancelStopEdit();
const { cell, column } = this.__edited;
const editor = column._getEditorComponent(cell);

const path = event.composedPath();
const nodes = path.slice(0, path.indexOf(editor) + 1).filter((node) => node.nodeType === Node.ELEMENT_NODE);
// Detect focus moving to e.g. vaadin-select-overlay or vaadin-date-picker-overlay
if (nodes.some((el) => typeof el._shouldRemoveFocus === 'function' && !el._shouldRemoveFocus(event))) {
return true;
}
}
}

/** @private */
_onEditorFocusIn() {
this._cancelStopEdit();
}

/** @private */
_startEdit(cell, column) {
const isCellEditable = this._isCellEditable(cell);
Expand Down Expand Up @@ -446,9 +446,21 @@ export const InlineEditingMixin = (superClass) =>
// 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 });
const ignore = await new Promise((resolve) => {
editor.addEventListener(
'focusout',
(event) => {
resolve(this.__shouldIgnoreFocusOut(event));
},
{ once: true },
);
});

// Ignore focusout event after which focus stays in the field,
// e.g. Tab between date and time pickers in date-time-picker.
if (ignore) {
return;
}
} else {
e.preventDefault();
}
Expand Down
114 changes: 0 additions & 114 deletions test/integration/dialog-grid-pro.test.js
View file

This file was deleted.

52 changes: 38 additions & 14 deletions test/integration/grid-pro-custom-editor.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,21 @@ describe('grid-pro custom editor', () => {
expect(cell._content.querySelector('vaadin-date-picker')).to.be.ok;
});

it('should stop editing and update value when closing on outside click', async () => {
it('should not stop editing when clicking inside the overlay but not on focusable element', async () => {
// Open the overlay
await sendKeys({ press: 'ArrowDown' });
await waitForOverlayRender();

// Click between toolbar buttons
const overlayContent = document.querySelector('vaadin-date-picker-overlay-content');
const { x, y } = middleOfNode(overlayContent.shadowRoot.querySelector('[part="toolbar"]'));
await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] });
await nextRender();

expect(cell._content.querySelector('vaadin-date-picker')).to.be.ok;
});

it('should not stop editing and update value when closing on outside click', async () => {
// Open the overlay
await sendKeys({ press: 'ArrowDown' });
await waitForOverlayRender();
Expand All @@ -124,11 +138,9 @@ describe('grid-pro custom editor', () => {
await sendMouse({ type: 'click', position: [10, 10] });
await nextRender();

// TODO: closing occurs in `vaadin-overlay-outside-click` listener added on global `focusin`
// in grid-pro. Consider replacing it with `_shouldRemoveFocus()` check on editor `focusout`
// so that we don't stop editing on outside click, to align with the combo-box behavior.
expect(cell._content.querySelector('vaadin-date-picker')).to.be.not.ok;
expect(cell._content.textContent).to.equal('1984-01-12');
const editor = cell._content.querySelector('vaadin-date-picker');
expect(editor).to.be.ok;
expect(editor.value).to.equal('1984-01-12');
});
});

Expand Down Expand Up @@ -214,7 +226,21 @@ describe('grid-pro custom editor', () => {
await sendKeys({ press: 'Enter' });
});

it('should stop editing and update value when closing on date-picker outside click', async () => {
it('should not stop editing when switching between pickers using Tab', async () => {
// Move focus to the time-picker
await sendKeys({ press: 'Tab' });
await nextRender();
expect(cell._content.querySelector('vaadin-date-time-picker')).to.be.ok;

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

it('should not stop editing and update value when closing on date-picker outside click', async () => {
await sendKeys({ press: 'ArrowDown' });
await waitForOverlayRender();

Expand All @@ -232,16 +258,13 @@ describe('grid-pro custom editor', () => {
await sendMouse({ type: 'click', position: [10, 10] });
await nextRender();

// TODO: closing occurs in `vaadin-overlay-outside-click` listener added on global `focusin`
// in grid-pro. Consider replacing it with `_shouldRemoveFocus()` check on editor `focusout`
// so that we don't stop editing on outside click, to align with the combo-box behavior.
expect(cell._content.querySelector('vaadin-date-time-picker')).to.be.not.ok;
expect(cell._content.textContent).to.equal('1984-01-12T09:00');
const editor = cell._content.querySelector('vaadin-date-time-picker');
expect(editor).to.be.ok;
expect(editor.value).to.equal('1984-01-12T09:00');
});

it('should not stop editing and update value when closing on time-picker outside click', async () => {
// TODO: replace with Tab and add a separate test to not stop editing on Tab
cell._content.querySelector('vaadin-time-picker').focus();
await sendKeys({ press: 'Tab' });

// Open the overlay
await sendKeys({ press: 'ArrowDown' });
Expand All @@ -250,6 +273,7 @@ describe('grid-pro custom editor', () => {
await sendKeys({ press: 'ArrowDown' });

await sendMouse({ type: 'click', position: [10, 10] });
await nextRender();

const editor = cell._content.querySelector('vaadin-date-time-picker');
expect(editor).to.be.ok;
Expand Down

0 comments on commit f903572

Please sign in to comment.