Skip to content

Commit 7601b0c

Browse files
authored
fix: do not dispatch cell-focus on mouse up outside of cell (#2215)
* 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
1 parent 04e0429 commit 7601b0c

File tree

2 files changed

+80
-4
lines changed

2 files changed

+80
-4
lines changed

src/vaadin-grid.html

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,12 @@
342342

343343
static get properties() {
344344
return {
345+
/** @private */
346+
_chrome: {
347+
type: Boolean,
348+
value: /Chrome/.test(navigator.userAgent) && /Google Inc/.test(navigator.vendor)
349+
},
350+
345351
/** @private */
346352
_safari: {
347353
type: Boolean,
@@ -607,13 +613,16 @@
607613
// focusable slot wrapper, that is why cells are not focused with
608614
// mousedown. Workaround: listen for mousedown and focus manually.
609615
cellContent.addEventListener('mousedown', () => {
610-
if (window.chrome) {
616+
if (this._chrome) {
611617
// Chrome bug: focusing before mouseup prevents text selection, see http://crbug.com/771903
612-
const mouseUpListener = () => {
613-
if (!cellContent.contains(this.getRootNode().activeElement)) {
618+
const mouseUpListener = (event) => {
619+
// If focus is on element within the cell content — respect it, do not change
620+
const contentContainsFocusedElement = cellContent.contains(this.getRootNode().activeElement);
621+
// Only focus if mouse is released on cell content itself
622+
const mouseUpWithinCell = event.composedPath().indexOf(cellContent) >= 0;
623+
if (!contentContainsFocusedElement && mouseUpWithinCell) {
614624
cell.focus();
615625
}
616-
// If focus is in the cell content — respect it, do not change.
617626
document.removeEventListener('mouseup', mouseUpListener, true);
618627
};
619628
document.addEventListener('mouseup', mouseUpListener, true);

test/keyboard-navigation.html

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,16 @@
230230
MockInteractions.keyDownOn(target || grid.shadowRoot.activeElement, 113, [], 'F2');
231231
}
232232

233+
function mouseDown(target) {
234+
const event = new CustomEvent('mousedown', {bubbles: true, cancelable: true, composed: true});
235+
target.dispatchEvent(event);
236+
}
237+
238+
function mouseUp(target) {
239+
const event = new CustomEvent('mouseup', {bubbles: true, cancelable: true, composed: true});
240+
target.dispatchEvent(event);
241+
}
242+
233243
function getFirstHeaderCell() {
234244
return grid.$.header.children[0].children[0];
235245
}
@@ -1914,6 +1924,63 @@
19141924

19151925
expect(spy.calledOnce).to.be.true;
19161926
});
1927+
1928+
// Separate test suite for Chrome, where we use a workaround to dispatch
1929+
// cell-focus on mouse up
1930+
const isChrome = /Chrome/.test(navigator.userAgent) && /Google Inc/.test(navigator.vendor);
1931+
(isChrome ? describe : describe.skip)('chrome', () => {
1932+
it('should dispatch cell-focus on mouse up on cell content', () => {
1933+
const spy = sinon.spy();
1934+
grid.addEventListener('cell-focus', spy);
1935+
1936+
// Mouse down and release on cell content element
1937+
const cell = getRowFirstCell(0);
1938+
mouseDown(cell._content);
1939+
mouseUp(cell._content);
1940+
expect(spy.calledOnce).to.be.true;
1941+
});
1942+
1943+
it('should dispatch cell-focus on mouse up on cell content, when grid is in shadow DOM', () => {
1944+
const spy = sinon.spy();
1945+
grid.addEventListener('cell-focus', spy);
1946+
1947+
// Move grid into a shadow DOM
1948+
const container = document.createElement('div');
1949+
document.body.appendChild(container);
1950+
container.attachShadow({mode: 'open'});
1951+
container.shadowRoot.appendChild(grid);
1952+
1953+
// Mouse down and release on cell content element
1954+
const cell = getRowFirstCell(0);
1955+
mouseDown(cell._content);
1956+
mouseUp(cell._content);
1957+
expect(spy.calledOnce).to.be.true;
1958+
});
1959+
1960+
it('should dispatch cell-focus on mouse up within cell content', () => {
1961+
const spy = sinon.spy();
1962+
grid.addEventListener('cell-focus', spy);
1963+
1964+
// Mouse down and release on cell content child
1965+
const cell = getRowFirstCell(0);
1966+
const contentSpan = document.createElement('span');
1967+
cell._content.appendChild(contentSpan);
1968+
1969+
mouseDown(contentSpan);
1970+
mouseUp(contentSpan);
1971+
expect(spy.calledOnce).to.be.true;
1972+
});
1973+
1974+
// Regression test for https://github.com/vaadin/flow-components/issues/2863
1975+
it('should not dispatch cell-focus on mouse up outside of cell', () => {
1976+
const spy = sinon.spy();
1977+
grid.addEventListener('cell-focus', spy);
1978+
1979+
mouseDown(getRowFirstCell(0)._content);
1980+
mouseUp(document.body);
1981+
expect(spy.calledOnce).to.be.false;
1982+
});
1983+
});
19171984
});
19181985
});
19191986

0 commit comments

Comments
 (0)