Skip to content

Commit

Permalink
fix: do not dispatch cell-focus on mouse up outside of cell (#2215)
Browse files Browse the repository at this point in the history
* fix: do not dispatch cell-focus on mouse up outside of cell

* add test for cell focus from shadow DOM

* revert changes to other test setup
  • Loading branch information
sissbruecker authored Apr 26, 2022
1 parent 04e0429 commit 7601b0c
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/vaadin-grid.html
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,12 @@

static get properties() {
return {
/** @private */
_chrome: {
type: Boolean,
value: /Chrome/.test(navigator.userAgent) && /Google Inc/.test(navigator.vendor)
},

/** @private */
_safari: {
type: Boolean,
Expand Down Expand Up @@ -607,13 +613,16 @@
// focusable slot wrapper, that is why cells are not focused with
// mousedown. Workaround: listen for mousedown and focus manually.
cellContent.addEventListener('mousedown', () => {
if (window.chrome) {
if (this._chrome) {
// Chrome bug: focusing before mouseup prevents text selection, see http://crbug.com/771903
const mouseUpListener = () => {
if (!cellContent.contains(this.getRootNode().activeElement)) {
const mouseUpListener = (event) => {
// If focus is on element within the cell content — respect it, do not change
const contentContainsFocusedElement = cellContent.contains(this.getRootNode().activeElement);
// Only focus if mouse is released on cell content itself
const mouseUpWithinCell = event.composedPath().indexOf(cellContent) >= 0;
if (!contentContainsFocusedElement && mouseUpWithinCell) {
cell.focus();
}
// If focus is in the cell content — respect it, do not change.
document.removeEventListener('mouseup', mouseUpListener, true);
};
document.addEventListener('mouseup', mouseUpListener, true);
Expand Down
67 changes: 67 additions & 0 deletions test/keyboard-navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,16 @@
MockInteractions.keyDownOn(target || grid.shadowRoot.activeElement, 113, [], 'F2');
}

function mouseDown(target) {
const event = new CustomEvent('mousedown', {bubbles: true, cancelable: true, composed: true});
target.dispatchEvent(event);
}

function mouseUp(target) {
const event = new CustomEvent('mouseup', {bubbles: true, cancelable: true, composed: true});
target.dispatchEvent(event);
}

function getFirstHeaderCell() {
return grid.$.header.children[0].children[0];
}
Expand Down Expand Up @@ -1914,6 +1924,63 @@

expect(spy.calledOnce).to.be.true;
});

// Separate test suite for Chrome, where we use a workaround to dispatch
// cell-focus on mouse up
const isChrome = /Chrome/.test(navigator.userAgent) && /Google Inc/.test(navigator.vendor);
(isChrome ? describe : describe.skip)('chrome', () => {
it('should dispatch cell-focus on mouse up on cell content', () => {
const spy = sinon.spy();
grid.addEventListener('cell-focus', spy);

// Mouse down and release on cell content element
const cell = getRowFirstCell(0);
mouseDown(cell._content);
mouseUp(cell._content);
expect(spy.calledOnce).to.be.true;
});

it('should dispatch cell-focus on mouse up on cell content, when grid is in shadow DOM', () => {
const spy = sinon.spy();
grid.addEventListener('cell-focus', spy);

// Move grid into a shadow DOM
const container = document.createElement('div');
document.body.appendChild(container);
container.attachShadow({mode: 'open'});
container.shadowRoot.appendChild(grid);

// Mouse down and release on cell content element
const cell = getRowFirstCell(0);
mouseDown(cell._content);
mouseUp(cell._content);
expect(spy.calledOnce).to.be.true;
});

it('should dispatch cell-focus on mouse up within cell content', () => {
const spy = sinon.spy();
grid.addEventListener('cell-focus', spy);

// Mouse down and release on cell content child
const cell = getRowFirstCell(0);
const contentSpan = document.createElement('span');
cell._content.appendChild(contentSpan);

mouseDown(contentSpan);
mouseUp(contentSpan);
expect(spy.calledOnce).to.be.true;
});

// Regression test for https://github.com/vaadin/flow-components/issues/2863
it('should not dispatch cell-focus on mouse up outside of cell', () => {
const spy = sinon.spy();
grid.addEventListener('cell-focus', spy);

mouseDown(getRowFirstCell(0)._content);
mouseUp(document.body);
expect(spy.calledOnce).to.be.false;
});
});
});
});

Expand Down

0 comments on commit 7601b0c

Please # to comment.