fix!: prevent focusout when closing combo-box on outside click (#7846)

Author: web-padawan

packages/combo-box/src/vaadin-combo-box-light.js

@@ -84,7 +84,6 @@ class ComboBoxLight extends ComboBoxLightMixin(ThemableMixin(PolymerElement)) {
         theme$="[[_theme]]"
         position-target="[[inputElement]]"
         no-vertical-overlap
-        restore-focus-node="[[inputElement]]"
       ></vaadin-combo-box-overlay>
     `;
   }

packages/combo-box/src/vaadin-combo-box-mixin.js

@@ -619,8 +619,6 @@ export const ComboBoxMixin = (subclass) =>
             this.inputElement.focus();
           }
         }
-
-        this._overlayElement.restoreFocusOnClose = true;
       } else {
         this._onClosed();
       }
@@ -719,9 +717,7 @@ export const ComboBoxMixin = (subclass) =>
     _onKeyDown(e) {
       super._onKeyDown(e);
 
-      if (e.key === 'Tab') {
-        this._overlayElement.restoreFocusOnClose = false;
-      } else if (e.key === 'ArrowDown') {
+      if (e.key === 'ArrowDown') {
         this._onArrowDown();
 
         // Prevent caret from moving

packages/combo-box/src/vaadin-combo-box-overlay-mixin.js

@@ -3,6 +3,7 @@
  * Copyright (c) 2015 - 2024 Vaadin Ltd.
  * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
  */
+import { isElementFocusable } from '@vaadin/a11y-base/src/focus-utils.js';
 import { PositionMixin } from '@vaadin/overlay/src/vaadin-overlay-position-mixin.js';
 
 /**
@@ -46,6 +47,20 @@ export const ComboBoxOverlayMixin = (superClass) =>
       return !eventPath.includes(this.positionTarget) && !eventPath.includes(this);
     }
 
+    /**
+     * @protected
+     * @override
+     */
+    _mouseDownListener(event) {
+      super._mouseDownListener(event);
+
+      // Prevent global mousedown event to avoid losing focus on outside click,
+      // unless the clicked element is also focusable (e.g. in date-time-picker).
+      if (this._shouldCloseOnOutsideClick(event) && !isElementFocusable(event.composedPath()[0])) {
+        event.preventDefault();
+      }
+    }
+
     /** @protected */
     _updateOverlayWidth() {
       const propPrefix = this.localName;

packages/combo-box/src/vaadin-combo-box.js

@@ -206,7 +206,6 @@ class ComboBox extends ComboBoxDataProviderMixin(
         theme$="[[_theme]]"
         position-target="[[_positionTarget]]"
         no-vertical-overlap
-        restore-focus-node="[[inputElement]]"
       ></vaadin-combo-box-overlay>
 
       <slot name="tooltip"></slot>

packages/combo-box/src/vaadin-lit-combo-box-light.js

@@ -46,7 +46,6 @@ class ComboBoxLight extends ComboBoxLightMixin(ThemableMixin(PolylitMixin(LitEle
         ?loading="${this.loading}"
         theme="${ifDefined(this._theme)}"
         .positionTarget="${this.inputElement}"
-        .restoreFocusNode="${this.inputElement}"
         no-vertical-overlap
       ></vaadin-combo-box-overlay>
     `;

packages/combo-box/src/vaadin-lit-combo-box.js

@@ -106,7 +106,6 @@ class ComboBox extends ComboBoxDataProviderMixin(
         ?loading="${this.loading}"
         theme="${ifDefined(this._theme)}"
         .positionTarget="${this._positionTarget}"
-        .restoreFocusNode="${this.inputElement}"
         no-vertical-overlap
       ></vaadin-combo-box-overlay>
 

packages/combo-box/test/interactions.common.js

@@ -13,7 +13,7 @@ import {
   tap,
   touchstart,
 } from '@vaadin/testing-helpers';
-import { sendKeys } from '@web/test-runner-commands';
+import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands';
 import sinon from 'sinon';
 import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
 import { getFirstItem, setInputValue } from './helpers.js';
@@ -161,30 +161,6 @@ describe('interactions', () => {
       expect(event.defaultPrevented).to.be.true;
     });
 
-    it('should restore focus to the input on outside click', async () => {
-      comboBox.focus();
-      comboBox.open();
-      outsideClick();
-      await aTimeout(0);
-      expect(document.activeElement).to.equal(input);
-    });
-
-    it('should restore focus to the input on toggle button click', async () => {
-      comboBox.focus();
-      comboBox.open();
-      comboBox._toggleElement.click();
-      await aTimeout(0);
-      expect(document.activeElement).to.equal(input);
-    });
-
-    it('should focus the input on outside click if not focused before opening', async () => {
-      expect(document.activeElement).to.equal(document.body);
-      comboBox.open();
-      outsideClick();
-      await aTimeout(0);
-      expect(document.activeElement).to.equal(input);
-    });
-
     // NOTE: WebKit incorrectly detects touch environment
     // See https://github.com/vaadin/web-components/issues/257
     (isTouch ? it.skip : it)('should focus the input on opening if not focused', () => {
@@ -225,14 +201,33 @@ describe('interactions', () => {
       expect(comboBox.hasAttribute('focus-ring')).to.be.true;
     });
 
-    it('should not keep focus-ring attribute after closing with outside click', () => {
-      comboBox.focus();
-      comboBox.setAttribute('focus-ring', '');
-      comboBox.open();
-      outsideClick();
-      expect(comboBox.hasAttribute('focus-ring')).to.be.false;
-      // FIXME: see https://github.com/vaadin/web-components/issues/4148
-      // expect(comboBox.hasAttribute('focus-ring')).to.be.true;
+    describe('close on click', () => {
+      afterEach(async () => {
+        await resetMouse();
+      });
+
+      it('should restore focus to the input on outside click', async () => {
+        comboBox.focus();
+        comboBox.open();
+        await sendMouse({ type: 'click', position: [200, 200] });
+        expect(document.activeElement).to.equal(input);
+      });
+
+      it('should keep focus in the input on toggle button click', async () => {
+        comboBox.focus();
+        comboBox.open();
+        const rect = comboBox._toggleElement.getBoundingClientRect();
+        await sendMouse({ type: 'click', position: [rect.x, rect.y] });
+        expect(document.activeElement).to.equal(input);
+      });
+
+      it('should keep focus-ring attribute after closing with outside click', async () => {
+        comboBox.focus();
+        comboBox.setAttribute('focus-ring', '');
+        comboBox.open();
+        await sendMouse({ type: 'click', position: [200, 200] });
+        expect(comboBox.hasAttribute('focus-ring')).to.be.true;
+      });
     });
   });
 
@@ -251,6 +246,7 @@ describe('interactions', () => {
     });
 
     it('should re-enable virtual keyboard on blur', async () => {
+      comboBox.focus();
       comboBox.open();
       comboBox.close();
       await aTimeout(0);

packages/date-time-picker/test/basic.test.js

@@ -1,6 +1,6 @@
 import { expect } from '@vaadin/chai-plugins';
 import { aTimeout, fixtureSync, focusin, focusout, nextFrame, nextRender, outsideClick } from '@vaadin/testing-helpers';
-import { sendKeys } from '@web/test-runner-commands';
+import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands';
 import sinon from 'sinon';
 import '../src/vaadin-date-time-picker.js';
 import { changeInputValue } from './helpers.js';
@@ -41,12 +41,23 @@ describe('Basic features', () => {
   let datePicker;
   let timePicker;
 
+  async function click(element) {
+    const rect = element.inputElement.getBoundingClientRect();
+    const x = Math.floor(rect.x + rect.width / 2);
+    const y = Math.floor(rect.y + rect.height / 2);
+    await sendMouse({ type: 'click', position: [x, y] });
+  }
+
   beforeEach(() => {
     dateTimePicker = fixtureSync('<vaadin-date-time-picker></vaadin-date-time-picker>');
     datePicker = getDatePicker(dateTimePicker);
     timePicker = getTimePicker(dateTimePicker);
   });
 
+  afterEach(async () => {
+    await resetMouse();
+  });
+
   it('should have default value', () => {
     expect(dateTimePicker.value).to.equal('');
   });
@@ -189,13 +200,11 @@ describe('Basic features', () => {
 
   describe('date-picker focused', () => {
     it('should remove focused attribute on time-picker click', async () => {
-      datePicker.focus();
-      datePicker.click();
+      await click(datePicker);
       await nextRender();
       expect(datePicker.hasAttribute('focused')).to.be.true;
 
-      timePicker.focus();
-      timePicker.click();
+      await click(timePicker);
       expect(datePicker.hasAttribute('focused')).to.be.false;
     });
 
@@ -207,37 +216,39 @@ describe('Basic features', () => {
       await nextRender();
       expect(datePicker.hasAttribute('focus-ring')).to.be.true;
 
-      timePicker.focus();
-      timePicker.click();
+      await click(timePicker);
       expect(datePicker.hasAttribute('focus-ring')).to.be.false;
     });
   });
 
   describe('time-picker focused', () => {
+    beforeEach(() => {
+      // Disable auto-open to make tests more reliable by only moving
+      // focus on mousedown (and not the date-picker overlay opening).
+      dateTimePicker.autoOpenDisabled = true;
+    });
+
     it('should remove focused attribute on date-picker click', async () => {
-      timePicker.focus();
-      timePicker.click();
+      await click(timePicker);
+      // Open the overlay with the keyboard
+      await sendKeys({ press: 'ArrowDown' });
       await nextRender();
       expect(timePicker.hasAttribute('focused')).to.be.true;
 
-      datePicker.focus();
-      datePicker.click();
-      await nextRender();
+      await click(datePicker);
       expect(timePicker.hasAttribute('focused')).to.be.false;
     });
 
     it('should remove focus-ring attribute on date-picker click', async () => {
       // Focus the time-picker with the keyboard
-      await sendKeys({ press: 'Tab' });
+      datePicker.focus();
       await sendKeys({ press: 'Tab' });
       // Open the overlay with the keyboard
       await sendKeys({ press: 'ArrowDown' });
       await nextRender();
       expect(timePicker.hasAttribute('focus-ring')).to.be.true;
 
-      datePicker.focus();
-      datePicker.click();
-      await nextRender();
+      await click(datePicker);
       expect(timePicker.hasAttribute('focus-ring')).to.be.false;
     });
   });

test/integration/grid-pro-custom-editor.test.js

@@ -1,6 +1,6 @@
 import { expect } from '@vaadin/chai-plugins';
 import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
-import { sendKeys } from '@web/test-runner-commands';
+import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands';
 import '@vaadin/combo-box';
 import '@vaadin/date-picker';
 import '@vaadin/grid-pro';
@@ -57,6 +57,10 @@ describe('grid-pro custom editor', () => {
     await nextRender();
   });
 
+  afterEach(async () => {
+    await resetMouse();
+  });
+
   describe('date-picker', () => {
     it('should apply the updated date to the cell when exiting on Tab', async () => {
       const cell = getContainerCell(grid.$.items, 0, 2);
@@ -107,6 +111,22 @@ describe('grid-pro custom editor', () => {
 
       expect(cell._content.textContent).to.equal('active');
     });
+
+    it('should not stop editing and update value when closing on outside click', async () => {
+      const cell = getContainerCell(grid.$.items, 0, 3);
+      cell.focus();
+
+      await sendKeys({ press: 'Enter' });
+
+      await sendKeys({ press: 'ArrowDown' });
+      await sendKeys({ type: 'active' });
+
+      await sendMouse({ type: 'click', position: [10, 10] });
+
+      const editor = cell._content.querySelector('vaadin-combo-box');
+      expect(editor).to.be.ok;
+      expect(editor.value).to.equal('active');
+    });
   });
 
   describe('time-picker', () => {
@@ -133,5 +153,21 @@ describe('grid-pro custom editor', () => {
 
       expect(cell._content.textContent).to.equal('10:00');
     });
+
+    it('should not stop editing and update value when closing on outside click', async () => {
+      const cell = getContainerCell(grid.$.items, 0, 4);
+      cell.focus();
+
+      await sendKeys({ press: 'Enter' });
+
+      await sendKeys({ press: 'ArrowDown' });
+      await sendKeys({ type: '10:00' });
+
+      await sendMouse({ type: 'click', position: [10, 10] });
+
+      const editor = cell._content.querySelector('vaadin-time-picker');
+      expect(editor).to.be.ok;
+      expect(editor.value).to.equal('10:00');
+    });
   });
 });