From 72d53bad4c6e60daee1d23445200ec488659e2ba Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Thu, 30 Nov 2023 09:42:09 +0200 Subject: [PATCH] Revert "fix: focus the closest focusable on overlay mousedown (#6850) (#6856)" (#6886) This reverts commit ee01a1ae03e28b38778789d2ecbc95f426ddeab1. --- packages/component-base/src/focus-utils.d.ts | 8 -- packages/component-base/src/focus-utils.js | 40 --------- .../component-base/test/focus-utils.test.js | 69 +------------- packages/overlay/src/vaadin-overlay.js | 15 ---- packages/overlay/test/overlay.test.js | 89 +++---------------- 5 files changed, 12 insertions(+), 209 deletions(-) diff --git a/packages/component-base/src/focus-utils.d.ts b/packages/component-base/src/focus-utils.d.ts index d437b1993c..9217b4f95e 100644 --- a/packages/component-base/src/focus-utils.d.ts +++ b/packages/component-base/src/focus-utils.d.ts @@ -49,11 +49,3 @@ export declare function isElementFocused(element: HTMLElement): boolean; * The method traverses nodes in shadow DOM trees too if any. */ export declare function getFocusableElements(element: HTMLElement): HTMLElement[]; - -/** - * Returns a closest focusable ancestor of an element, - * or an element itself in case if it's focusable. - * - * The method traverses nodes in shadow DOM trees too if any. - */ -export declare function getClosestFocusable(element: HTMLElement): HTMLElement | undefined; diff --git a/packages/component-base/src/focus-utils.js b/packages/component-base/src/focus-utils.js index af29a8fecf..98138f3a0e 100644 --- a/packages/component-base/src/focus-utils.js +++ b/packages/component-base/src/focus-utils.js @@ -260,43 +260,3 @@ export function getFocusableElements(element) { } return focusableElements; } - -/** - * Returns an ancestor for the given node. - * - * @param {Node} node - * @return {Node} - * @private - */ -function getAncestor(node) { - if (node.nodeType === Node.ELEMENT_NODE && node.assignedSlot) { - return node.assignedSlot; - } - - if (node instanceof ShadowRoot) { - return node.host; - } - - return node.parentNode; -} - -/** - * Returns a closest focusable ancestor of an element, - * or an element itself in case if it's focusable. - * - * The method traverses nodes in shadow DOM trees too if any. - * - * @param {HTMLElement} element - * @return {HTMLElement | undefined} - */ -export function getClosestFocusable(element) { - let current = element; - - while (current !== document.body) { - if (current.nodeType === Node.ELEMENT_NODE && isElementFocusable(current)) { - return current; - } - - current = getAncestor(current); - } -} diff --git a/packages/component-base/test/focus-utils.test.js b/packages/component-base/test/focus-utils.test.js index 10f60269cf..948bc88b8a 100644 --- a/packages/component-base/test/focus-utils.test.js +++ b/packages/component-base/test/focus-utils.test.js @@ -1,7 +1,6 @@ import { expect } from '@esm-bundle/chai'; -import { defineCE, fixtureSync, mousedown, tabKeyDown } from '@vaadin/testing-helpers'; +import { fixtureSync, mousedown, tabKeyDown } from '@vaadin/testing-helpers'; import { - getClosestFocusable, getFocusableElements, isElementFocusable, isElementFocused, @@ -222,70 +221,4 @@ describe('focus-utils', () => { expect(isKeyboardActive()).to.be.false; }); }); - - describe('getClosestFocusable', () => { - let element, host; - - const hostTag = defineCE( - class extends HTMLElement { - constructor() { - super(); - this.attachShadow({ mode: 'open' }); - this.shadowRoot.innerHTML = ` -
- - -
- - `; - } - }, - ); - - beforeEach(() => { - element = fixtureSync(` -
-
- <${hostTag}> - -
Content
-
Footer - -
-
- `); - host = element.querySelector(`${hostTag}`); - }); - - it('should return element itself in case if it is focusable', () => { - const outerInput = element.querySelector('#outer'); - expect(getClosestFocusable(outerInput)).to.equal(outerInput); - - const innerInput = host.shadowRoot.querySelector('#inner'); - expect(getClosestFocusable(innerInput)).to.equal(innerInput); - }); - - it('should return parent element in case if it is focusable', () => { - expect(getClosestFocusable(host)).to.equal(host.parentElement); - }); - - it('should return element in shadow DOM for a slotted element', () => { - const content = element.querySelector('#content'); - const focusable = host.shadowRoot.querySelector('[tabindex="0"]'); - expect(getClosestFocusable(content)).to.equal(focusable); - }); - - it('should return element outside shadow DOM for a slotted element', () => { - const footer = element.querySelector('[slot="footer"]'); - expect(getClosestFocusable(footer)).to.equal(host.parentElement); - }); - - it('should return element outside shadow DOM for a shadow root', () => { - expect(getClosestFocusable(host.shadowRoot)).to.equal(host.parentElement); - }); - - it('should return undefined if no focusable ancestor is found', () => { - expect(getClosestFocusable(element)).to.be.undefined; - }); - }); }); diff --git a/packages/overlay/src/vaadin-overlay.js b/packages/overlay/src/vaadin-overlay.js index d25cd3ca6b..dc805d0566 100644 --- a/packages/overlay/src/vaadin-overlay.js +++ b/packages/overlay/src/vaadin-overlay.js @@ -11,7 +11,6 @@ import { isIOS } from '@vaadin/component-base/src/browser-utils.js'; import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js'; import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js'; import { FocusTrapController } from '@vaadin/component-base/src/focus-trap-controller.js'; -import { getClosestFocusable } from '@vaadin/component-base/src/focus-utils.js'; import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; /** @@ -377,10 +376,6 @@ class Overlay extends ThemableMixin(DirMixin(ControllerMixin(PolymerElement))) { this.$.backdrop.addEventListener('click', () => {}); this.addController(this.__focusTrapController); - - this.addEventListener('mousedown', (e) => { - this._onMouseDown(e); - }); } /** @private */ @@ -991,16 +986,6 @@ class Overlay extends ThemableMixin(DirMixin(ControllerMixin(PolymerElement))) { this.style.zIndex = zIndex; this.__zIndex = zIndex || parseFloat(getComputedStyle(this).zIndex); } - - /** @private */ - _onMouseDown(event) { - const target = event.target; - const focusable = getClosestFocusable(target.focusElement || target); - if (focusable) { - event.preventDefault(); - focusable.focus(); - } - } } customElements.define(Overlay.is, Overlay); diff --git a/packages/overlay/test/overlay.test.js b/packages/overlay/test/overlay.test.js index 22b154afc6..d2ec854420 100644 --- a/packages/overlay/test/overlay.test.js +++ b/packages/overlay/test/overlay.test.js @@ -1,17 +1,5 @@ import { expect } from '@esm-bundle/chai'; -import { - click, - enterKeyDown, - escKeyDown, - fixtureSync, - makeMouseEvent, - middleOfNode, - mousedown, - mouseup, - nextRender, - oneEvent, -} from '@vaadin/testing-helpers'; -import { resetMouse, sendMouse } from '@web/test-runner-commands'; +import { click, enterKeyDown, escKeyDown, fixtureSync, mousedown, mouseup, oneEvent } from '@vaadin/testing-helpers'; import sinon from 'sinon'; import '@vaadin/text-field/vaadin-text-field.js'; import '../vaadin-overlay.js'; @@ -209,25 +197,16 @@ describe('vaadin-overlay', () => { let parent, overlay, overlayPart, backdrop; beforeEach(async () => { - parent = document.createElement('div'); - overlay = fixtureSync('', parent); - overlay.renderer = (root) => { - if (!root.firstChild) { - const div = document.createElement('div'); - div.textContent = 'overlay content'; - root.appendChild(div); - - const btn = document.createElement('button'); - btn.textContent = 'Button'; - - const wrapper = document.createElement('p'); - // Mimic the DelegateFocusMixin logic - wrapper.focusElement = btn; - wrapper.appendChild(btn); - - root.appendChild(wrapper); - } - }; + parent = fixtureSync(` +
+ + + +
+ `); + overlay = parent.children[0]; overlayPart = overlay.$.overlay; backdrop = overlay.$.backdrop; overlay._observer.flush(); @@ -420,52 +399,6 @@ describe('vaadin-overlay', () => { expect(overlay.opened).to.be.true; }); }); - - describe('mousedown on content', () => { - afterEach(async () => { - await resetMouse(); - }); - - it('should not move focus to body on clicking the content element', async () => { - const div = overlay.querySelector('div'); - const { x, y } = middleOfNode(div); - - await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] }); - await nextRender(); - - expect(document.activeElement).to.be.equal(overlay); - }); - - it('should move focus to focusElement if the click target has one', async () => { - const p = overlay.querySelector('p'); - const { x, y } = middleOfNode(p); - - await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] }); - await nextRender(); - - expect(document.activeElement).to.be.equal(p.querySelector('button')); - }); - - it('should prevent default on content mousedown', () => { - const div = overlay.querySelector('div'); - const event = makeMouseEvent('mousedown', middleOfNode(div), div); - expect(event.defaultPrevented).to.be.true; - }); - - it('should focus an overlay part on content mousedown', () => { - const div = overlay.querySelector('div'); - const spy = sinon.spy(overlayPart, 'focus'); - mousedown(div); - expect(spy.calledOnce).to.be.true; - }); - - it('should focus a focusable content element, if any', () => { - const button = overlay.querySelector('button'); - const spy = sinon.spy(button, 'focus'); - mousedown(button); - expect(spy.calledOnce).to.be.true; - }); - }); }); describe('position and sizing', () => {