fix: do not dispatch cell-focus on mouse up outside of cell (#3587) (#…

…3594)

* fix: do not dispatch cell-focus on mouse up outside of cell

* import chrome check from test helpers

* address review comments

* add test for clicking on cell content child

* refactor to skip whole suite

Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>

packages/grid/src/vaadin-grid.js

@@ -7,7 +7,7 @@ import './vaadin-grid-column.js';
 import './vaadin-grid-styles.js';
 import { beforeNextRender } from '@polymer/polymer/lib/utils/render-status.js';
 import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
-import { isAndroid, isFirefox, isIOS, isSafari, isTouch } from '@vaadin/component-base/src/browser-utils.js';
+import { isAndroid, isChrome, isFirefox, isIOS, isSafari, isTouch } from '@vaadin/component-base/src/browser-utils.js';
 import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
 import { processTemplates } from '@vaadin/component-base/src/templates.js';
 import { Virtualizer } from '@vaadin/component-base/src/virtualizer.js';
@@ -658,13 +658,16 @@ class Grid extends ElementMixin(
     // 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 (isChrome) {
         // 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 = cellContent.contains(event.target);
+          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);

packages/grid/test/keyboard-navigation.test.js

@@ -4,6 +4,7 @@ import {
   down as mouseDown,
   fixtureSync,
   focusin,
+  isChrome,
   keyboardEventFor,
   keyDownOn,
   keyUpOn,
@@ -1889,6 +1890,45 @@ describe('keyboard navigation', () => {
 
       grid.removeEventListener('cell-focus', spy);
     });
+
+    // Separate test suite for Chrome, where we use a workaround to dispatch
+    // cell-focus on mouse up
+    (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 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;
+      });
+    });
   });
 });