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.

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

refactor: replace global focusin listener with focusout check (#7858) (CP: 24.5) #7867

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 0 additions & 3 deletions packages/grid-pro/src/vaadin-grid-pro-edit-column-mixin.js
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
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

This file was deleted.

52 changes: 38 additions & 14 deletions test/integration/grid-pro-custom-editor.test.js
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