diff --git a/cypress/e2e/a11y-aria-hidden-focus.cy.ts b/cypress/e2e/a11y-aria-hidden-focus.cy.ts new file mode 100644 index 0000000..13b049a --- /dev/null +++ b/cypress/e2e/a11y-aria-hidden-focus.cy.ts @@ -0,0 +1,476 @@ +/** cSpell:ignore vscomp */ + +/** + * Focus must never sit inside an `aria-hidden="true"` subtree. + * + * Chrome refuses to apply aria-hidden when a descendant holds focus and logs "Blocked + * aria-hidden on an element because its descendant retained focus" - so the dropbox the + * component believes it hid stays exposed to assistive technology, which follows the tree, + * not the component. WAI-ARIA: aria-hidden must not be used on an ancestor of the focused + * element. + * + * Two windows produced it, both inside the popover's ~200ms hide transition, while the + * dropbox is still visible, still hit-testable and still `isOpened() === true`: + * + * 1. closeDropbox() marked the dropbox aria-hidden immediately, 200ms before it left the + * screen - fixed by deferring the attribute to afterHidePopper(); + * 2. handlers gated on isOpened() kept driving the fading dropbox - hovering an option + * focuses it (onOptionsMouseOver -> focusOption) - so focus could re-enter after the + * close and still be there when the attribute finally landed. Fixed twice over: the + * pointer is gated during the transition (isClosingTransition), and whatever a host + * still pulls in programmatically is released at hide-end (releaseFocusFromDropbox). + * + * WCAG 4.1.2 Name, Role, Value (A), 1.3.1 Info and Relationships (A), 2.4.3 Focus Order (A). + * + * Deliberately not covered: the deferred re-focus in afterRenderOptions(), which fires ~20ms after + * a render and focuses whatever still carries `.focused`. It looks like a third way in, and a case + * for it lived here briefly - but it could not fail. Once the dropbox is closed the `closed` class + * makes it `display: none`, so focus() on an option inside it is a no-op; before that, aria-hidden + * has not been applied yet. A fire-time guard there would be unreachable code, and the case that + * pretended to pin it passed whatever afterRenderOptions() did. Removed rather than left as false + * assurance. If the `closed` rule ever stops hiding the subtree, this becomes reachable and the + * guard becomes real. + */ + +import { mountVs, unmountVs, makeOptions } from '../support/mount'; + +type Violation = { kind: string; focused: string; ancestor: string; stack: string }; + +/** the sink lives on the window so re-installing the guard repoints it instead of stacking */ +type GuardHost = Window & { __ariaHiddenFocusSink?: Violation[] }; + +const describeEle = (el: Element | null) => + (el ? `${el.tagName.toLowerCase()}.${(el.getAttribute('class') || '').split(' ').join('.')}` : 'null'); + +/** + * The guard judges only the component under test: the docsify page hosting these specs has + * aria-hidden chrome of its own (sidebar, cover), and a violation there is not this + * component's defect. + */ +const isComponentNode = (el: Element | null) => !!el && !!el.closest('[class*="vscomp"]'); + +function hiddenAncestor(el: Element | null): Element | null { + let node: Element | null = el; + + while (node) { + if (node.getAttribute && node.getAttribute('aria-hidden') === 'true') { + return node; + } + + node = node.parentElement; + } + + return null; +} + +const callSite = (win: Window) => { + const raw = new (win as unknown as { Error: ErrorConstructor }).Error().stack || ''; + + return raw.split('\n').slice(2, 8).map((line) => line.trim()).join(' | '); +}; + +/** + * Reproduces what Chrome itself checks, from both directions: focus moving into an already + * hidden subtree, and aria-hidden being applied over a subtree that still holds focus. + * `Element.prototype.setAttribute` is patched rather than observed with a MutationObserver so + * the offending call site is captured synchronously - that is what pinned the root cause to + * `toggleOptionFocusedState` in the first place. + */ +function installAriaHiddenFocusGuard(win: Window, sink: Violation[]): void { + const host = win as GuardHost; + + if (host.__ariaHiddenFocusSink) { + /** already patched this window: repoint the sink rather than stacking a second patch */ + host.__ariaHiddenFocusSink = sink; + return; + } + + host.__ariaHiddenFocusSink = sink; + + win.document.addEventListener( + 'focusin', + (e) => { + const ancestor = hiddenAncestor(e.target as Element); + + if (ancestor && isComponentNode(ancestor)) { + host.__ariaHiddenFocusSink?.push({ + kind: 'focus moved into an aria-hidden subtree', + focused: describeEle(e.target as Element), + ancestor: describeEle(ancestor), + stack: callSite(win), + }); + } + }, + true, + ); + + const proto = (win as unknown as { Element: { prototype: Element } }).Element.prototype; + const nativeSetAttribute = proto.setAttribute; + + proto.setAttribute = function patchedSetAttribute(this: Element, name: string, value: string) { + nativeSetAttribute.call(this, name, value); + + /** + * String(): DomUtils.setAria() forwards the boolean `true`, not the string. The first + * version of this guard compared `value !== 'true'` and therefore never fired for any + * element the component hides - the whole "attribute applied over focus" direction was + * dead code, and deleting releaseFocusFromDropbox() left this spec green (review + * finding on PR #492). + */ + if (name !== 'aria-hidden' || String(value) !== 'true') { + return; + } + + const active = win.document.activeElement; + + if (active && active !== win.document.body && this.contains(active) && isComponentNode(this)) { + host.__ariaHiddenFocusSink?.push({ + kind: 'aria-hidden applied over the focused element', + focused: describeEle(active), + ancestor: describeEle(this), + stack: callSite(win), + }); + } + }; +} + +describe('A11y: aria-hidden is never applied over the focused element', { testIsolation: true }, () => { + const mountId = 'vs-aria-hidden-focus'; + const secondMountId = 'vs-aria-hidden-focus-2'; + const violations: Violation[] = []; + + /** + * `dropboxWrapper: 'body'` portals the dropbox out of the host element, which is the layout + * the report came from and the only one where the dropbox has a wrapper of its own to carry + * aria-hidden. Every dropbox query below therefore goes through the instance, not the DOM. + */ + const mount = (extra: Record = {}) => { + cy.viewport(1280, 800); + cy.visit('get-started'); + cy.window().then((win) => { + violations.length = 0; + installAriaHiddenFocusGuard(win, violations); + mountVs(win, mountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body', ...extra }); + }); + }; + + const dropbox = () => cy.get(`#${mountId}`).then(($ele) => cy.wrap($ele[0].virtualSelect.$dropboxWrapper)); + + const open = () => { + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + }; + + /** opening focuses the search input asynchronously (popover afterShow); wait for it */ + const waitForSearchFocus = () => { + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'search input focused after open') + .to.equal($ele[0].virtualSelect.$searchInput); + }); + }; + + /** + * Dispatch a real bubbling mouseover synchronously - no Cypress command hop, so it is + * guaranteed to land inside the hide transition it is aimed at, and no actionability check + * can reject the half-faded element (the mid-fade state is exactly what is under test). + */ + const hoverOptionNow = (vs: any, index: number) => { + const $option = vs.$dropboxContainer.querySelector(`.vscomp-option[data-index="${index}"]`); + const win = vs.$ele.ownerDocument.defaultView; + + $option.dispatchEvent(new win.MouseEvent('mouseover', { bubbles: true })); + }; + + /** the hide transition is ~200ms; wait it out so late callbacks are included */ + const assertNoViolations = () => { + cy.wait(700); + cy.then(() => { + expect(violations, JSON.stringify(violations, null, 2)).to.deep.equal([]); + }); + }; + + afterEach(() => { + cy.window().then((win) => { + unmountVs(win, mountId); + unmountVs(win, secondMountId); + }); + }); + + /** + * The reported case. Selecting in a single select closes the dropbox, but the pointer is + * still over the list while it fades out. The fading list must ignore the pointer: no new + * highlight, no aria-activedescendant on a combobox that just announced itself collapsed, + * no DOM focus pulled into a subtree about to be marked hidden - and focus must end on the + * combobox, not fall to . + */ + it('ignores the pointer over the fading list and lands focus on the combobox', () => { + mount(); + open(); + + dropbox().find('.vscomp-option[data-index="2"]').click(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + /** + * One Cypress command hop after the click, against a 200ms transition. The pin keeps + * the test honest: a closed dropbox ignores mouseover, so on a runner slow enough to + * outlive the fade the hover would silently become a no-op and the test would pass + * vacuously - this fails loudly with the reason instead. + */ + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + hoverOptionNow(vs, 3); + + expect(vs.$dropboxContainer.querySelector('.vscomp-option.focused'), 'highlight after hover').to.equal(null); + expect(vs.$wrapper.getAttribute('aria-activedescendant'), 'aria-activedescendant after hover').to.equal(null); + }); + + assertNoViolations(); + + cy.get(`#${mountId}`).then(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'focus once closed').to.equal($ele[0].virtualSelect.$wrapper); + }); + }); + + it('ignores the pointer when a multi-select closes from the toggle button', () => { + mount({ multiple: true }); + open(); + + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + hoverOptionNow(vs, 4); + + expect(vs.$dropboxContainer.querySelector('.vscomp-option.focused'), 'highlight after hover').to.equal(null); + }); + + assertNoViolations(); + }); + + /** + * The silent close: opening one instance closes every other one synchronously, straight + * through afterHidePopper() - while the first instance's search input still holds focus. + * The attribute write happens in the same script block, before the browser's style recalc + * can drop focus to , so without an explicit release aria-hidden lands over the + * focused input. shouldFocusWrapperOnClose is false here, so the release must blur, not + * steal focus back from the instance the user just opened. + */ + it('releases focus from a dropdown that another instance closes silently', () => { + mount(); + cy.window().then((win) => { + mountVs(win, secondMountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body' }); + }); + open(); + waitForSearchFocus(); + + cy.get(`#${secondMountId}`).then(($ele2) => $ele2[0].virtualSelect.openDropbox()); + + assertNoViolations(); + + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + const active = $ele[0].ownerDocument.activeElement; + + expect(vs.$dropboxWrapper.contains(active), 'focus inside the silently closed dropbox').to.equal(false); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'closed dropbox hidden').to.equal('true'); + }); + }); + + /** + * The pointer gate cannot cover host-driven focus: anything a host runs mid-fade that ends + * in focusOption() (a programmatic search, a value write) focuses the option it highlights, + * and a host can also focus() an element in the dropbox directly. The hide-end release is + * the backstop for that whole class - focus must be back on the combobox before the subtree + * is marked hidden. + * + * Driven with a direct focus() rather than through setSearchValue(): the search path + * re-renders the options (innerHTML), which destroys the focused node and races the + * deferred re-focus timer in afterRenderOptions() - where focus sits at hide-end then + * depends on which fired last. The direct call is the distilled, deterministic form of + * every path in the class. + */ + it('releases focus a host action pulled into the dropbox mid-fade', () => { + mount(); + open(); + waitForSearchFocus(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + const $option = vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]'); + + $option.focus(); + + /** precondition: DOM focus genuinely re-entered the fading dropbox */ + expect($ele[0].ownerDocument.activeElement, 'focus pulled into the fading dropbox').to.equal($option); + }); + + assertNoViolations(); + + cy.get(`#${mountId}`).then(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'focus once closed').to.equal($ele[0].virtualSelect.$wrapper); + }); + }); + + /** + * Tab from the search input is the one documented path that puts real DOM focus on an + * option while a search input exists (onKeyDown -> focusFirstVisibleOption). The outside + * click's mousedown blurs it before the close, so this pins that nothing during the hide + * transition pulls focus back into the component. + */ + it('keeps focus out of the dropbox when it closes with an option focused', () => { + mount(); + open(); + + cy.get(`#${mountId}`).pressKeys('Tab'); + dropbox().find('.vscomp-option.focused').should('exist'); + + cy.get('body').click(5, 5); + + assertNoViolations(); + + cy.window().then((win) => { + const active = win.document.activeElement as Element; + + expect(active.closest('[class*="vscomp"]'), 'focus left inside the component').to.equal(null); + }); + }); + + it('keeps focus out of the dropbox when Escape closes it mid navigation', () => { + mount({ search: false }); + open(); + + cy.get(`#${mountId}`).pressKeys('ArrowDown'); + cy.get(`#${mountId}`).pressKeys('ArrowDown'); + cy.get(`#${mountId}`).find('.vscomp-wrapper').trigger('keydown', { key: 'Escape', keyCode: 27, which: 27 }); + + assertNoViolations(); + + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'focus once closed').to.equal($ele[0].virtualSelect.$wrapper); + }); + }); + + /** + * The state behind all of the above: for as long as the component reports the dropbox as + * open - and keeps answering clicks and arrow keys on it - it must not also be telling + * assistive technology that the dropbox is not there. + */ + it('does not mark the dropbox hidden while it still reports itself open', () => { + mount(); + open(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden while still open').to.not.equal('true'); + }); + + /** and it must be hidden once the transition has actually finished */ + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden once closed').to.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('tabindex'), 'tabindex once closed').to.equal('-1'); + }); + }); + + /** + * The focus release fires `focusin` on the wrapper synchronously, mid-afterHidePopper(). A + * consumer handler reacting to it by reopening the dropdown ("restore the last dropdown" + * hosts) used to be clobbered: the hiding writes that follow the release landed on top of + * openDropbox()'s and left a visible, expanded dropdown carrying aria-hidden="true". The + * writes must stand down when the dropdown is open again by the time they run. + * + * (`afterClose` cannot reproduce this: DomUtils.dispatchEvent() defers events through + * setTimeout(0), so an afterClose handler always runs after afterHidePopper() finished.) + */ + it('does not hide a dropdown a focus handler reopens during the close', () => { + mount(); + open(); + waitForSearchFocus(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + /** attached after closeDropbox() so the close-time wrapper refocus does not trigger it */ + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + /** put focus back inside so the hide-end release has something to hand to the wrapper */ + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + vs.$wrapper.addEventListener('focusin', () => $ele[0].open?.(), { once: true }); + }); + + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'reopened').to.equal(true); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden on the reopened dropbox').to.not.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('tabindex'), 'tabindex on the reopened dropbox').to.equal('0'); + }); + + cy.then(() => { + expect(violations, JSON.stringify(violations, null, 2)).to.deep.equal([]); + }); + }); + + /** + * An open arriving mid-fade is queued behind the running hide (see + * reopen-during-hide-transition.cy.ts for the reopen behaviour itself). This is the a11y half + * of that path: the queue must hold the pointer gate up for the rest of the fade, so the list + * cannot take DOM focus back while the hide is still on its way to marking the subtree hidden. + * + * Worth pinning separately because the obvious way to write the queue - run openDropbox() + * eagerly and repair afterwards - lifts `isClosingTransition` and hands the fading list back + * to the pointer, which is exactly how focus used to end up inside an aria-hidden subtree. + */ + it('keeps the pointer gate up while an open waits behind the hide transition', () => { + mount(); + open(); + waitForSearchFocus(); + + cy.window().then((win) => { + const $ele = win.document.getElementById(mountId)!; + const vs = $ele.virtualSelect; + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition when the reopen arrives').to.equal(true); + + $ele.open?.(); + + expect(vs.isClosingTransition, 'pointer gate held while the open waits').to.equal(true); + + hoverOptionNow(vs, 3); + + expect(vs.$dropboxContainer.querySelector('.vscomp-option.focused'), 'highlight after hover').to.equal(null); + expect(vs.$dropboxContainer.contains(win.document.activeElement), 'focus after hover').to.equal(false); + }); + + assertNoViolations(); + + /** and the queued open still lands, on a dropbox that was never marked hidden underneath it */ + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden on the reopened dropbox').to.not.equal('true'); + expect(vs.$dropboxContainer.getBoundingClientRect().height, 'reopened dropbox height').to.be.greaterThan(0); + }); + }); + +}); diff --git a/cypress/e2e/a11y-close-clears-highlight.cy.ts b/cypress/e2e/a11y-close-clears-highlight.cy.ts index d121cc2..bedf49d 100644 --- a/cypress/e2e/a11y-close-clears-highlight.cy.ts +++ b/cypress/e2e/a11y-close-clears-highlight.cy.ts @@ -72,10 +72,16 @@ describe('A11y: closing the dropbox clears option navigation state', { testIsola }); /** - * Reopened in the *same tick* as the close, deliberately. Waiting for the `closed` class - * first would wait out `afterHidePopper()`, which clears the highlight on its own - so the - * case could never observe the bug it exists for. This is the user-visible symptom: reopen - * before the hide transition finishes and navigation must still start at the top. + * `openDropbox()` is called in the *same tick* as the close, deliberately. Waiting for the + * `closed` class first would wait out `afterHidePopper()`, which clears the highlight on its + * own - so the case could never observe the bug it exists for. + * + * The call itself now queues rather than reopening synchronously (a reopen mid-fade waits for + * the running hide to finish - see reopen-during-hide-transition.cy.ts), but that does not + * weaken what this pins: `isOpened()` stays true for the whole hide transition regardless, so + * the ArrowDown below reaches `focusOption()` either way, on a dropbox whose highlight + * `closeDropbox()` already cleared. What is under test is that clearing, not whether the + * queued open has landed by the time the key is pressed. */ it('starts navigation at the first option again when reopened mid hide-transition', () => { mount(); diff --git a/cypress/e2e/reopen-during-hide-transition.cy.ts b/cypress/e2e/reopen-during-hide-transition.cy.ts new file mode 100644 index 0000000..4ce5f79 --- /dev/null +++ b/cypress/e2e/reopen-during-hide-transition.cy.ts @@ -0,0 +1,422 @@ +/** cSpell:ignore vscomp popcomp */ + +/** + * A reopen that arrives while the dropbox is still closing must still open it. + * + * The popover refuses to show while its own hide transition is running: `show()` early-returns + * for as long as `pop-comp-active` is on the element, and only its `afterHide` removes that + * class. So `openDropbox()` during the ~200ms fade did all of its own work - `beforeOpen`, + * `aria-expanded="true"`, the instance back in `openInstances` - while `popper.show()` quietly + * did nothing and `afterShowPopper()` never ran. The pending hide then completed on top of it. + * + * The dropdown ended up shut with its combobox still announcing `aria-expanded="true"`, no + * `afterOpen` ever dispatched, and focus dropped to `` (the popover applies + * `display: none` before the component gets any callback, so nothing can hand it back). The + * host's `open()` was silently lost - it only looks like nothing happened, which is the worst + * shape for a bug to have. + * + * Reachable through the public `open()` API only: `toggleDropbox()` sees `isOpened() === true` + * for the whole transition, so a second click closes again rather than reopening. + */ + +import { mountVs, unmountVs, makeOptions } from '../support/mount'; + +describe('Reopening while the dropbox is still closing', { testIsolation: true }, () => { + const mountId = 'vs-reopen-during-hide'; + const secondMountId = 'vs-reopen-during-hide-2'; + + const mount = (extra: Record = {}) => { + cy.viewport(1280, 800); + cy.visit('get-started'); + cy.window().then((win) => { + mountVs(win, mountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body', ...extra }); + }); + }; + + const open = () => { + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + }; + + /** + * Close and reopen inside the same tick, so the reopen is guaranteed to land inside the hide + * transition rather than racing a Cypress command hop. `isOpened()` is asserted between the + * two to prove the transition really is still running - otherwise the case could pass by + * testing an ordinary closed-then-open sequence. + */ + const closeThenReopen = () => { + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition when the reopen arrives').to.equal(true); + $ele[0].open?.(); + }); + }; + + /** + * Settle first, then assert. + * + * Every assertion below also holds *during* the fade - `openDropbox()` sets aria-expanded and + * clears aria-hidden synchronously, and the popover has not yet applied `display: none` - so a + * retrying `should()` on its own would latch onto that transient and pass against the very bug + * this spec exists for. The wait covers the hide (~200ms) plus the show that has to follow it. + */ + const assertOpenAndVisible = () => { + cy.wait(700); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened()').to.equal(true); + expect(vs.$dropboxContainer.style.display, 'container display').to.not.equal('none'); + expect(vs.$dropboxContainer.getBoundingClientRect().height, 'rendered height').to.be.greaterThan(0); + expect(vs.$wrapper.getAttribute('aria-expanded'), 'aria-expanded').to.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden').to.not.equal('true'); + expect(vs.$dropboxWrapper.getAttribute('tabindex'), 'tabindex').to.equal('0'); + }); + }; + + /** opening focuses the search input asynchronously (popover afterShow); wait for it */ + const waitForSearchFocus = () => { + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].ownerDocument.activeElement, 'search input focused after open') + .to.equal($ele[0].virtualSelect.$searchInput); + }); + }; + + afterEach(() => { + cy.window().then((win) => { + unmountVs(win, mountId); + unmountVs(win, secondMountId); + }); + }); + + it('ends up open and on screen', () => { + mount(); + open(); + closeThenReopen(); + + assertOpenAndVisible(); + }); + + /** + * afterShowPopper() is where the dropdown finishes opening - the `focused` class, the scroll + * position, and the focus move onto the search input. A reopen that never reaches it leaves a + * dropdown nobody can type into. + */ + it('finishes the open: focus lands in the search input', () => { + mount(); + open(); + closeThenReopen(); + + assertOpenAndVisible(); + + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect($ele[0].ownerDocument.activeElement, 'focus after the reopen').to.equal(vs.$searchInput); + expect(vs.$wrapper.classList.contains('focused'), 'focused class').to.equal(true); + }); + }); + + /** + * One open() call is one open, however the reopen has to be sequenced internally. A host + * loading options on beforeOpen must not be asked twice, and afterOpen has to arrive - it is + * the only signal that the dropdown is actually usable. + */ + it('dispatches beforeOpen once and afterOpen once', () => { + mount(); + open(); + + const events: string[] = []; + + cy.get(`#${mountId}`).then(($ele) => { + ['beforeOpen', 'afterOpen', 'beforeClose', 'afterClose'].forEach((name) => { + $ele[0].addEventListener(name, () => events.push(name)); + }); + }); + + closeThenReopen(); + assertOpenAndVisible(); + + /** let any late duplicate arrive before counting - dispatchEvent defers through setTimeout */ + cy.wait(300); + cy.then(() => { + expect(events.filter((name) => name === 'beforeOpen').length, `beforeOpen (${events.join(', ')})`).to.equal(1); + expect(events.filter((name) => name === 'afterOpen').length, `afterOpen (${events.join(', ')})`).to.equal(1); + }); + }); + + /** a close arriving after the queued reopen wins - the dropdown must not spring back open */ + it('does not reopen when a close follows the reopen', () => { + mount(); + open(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + $ele[0].open?.(); + vs.closeDropbox(); + }); + + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.wait(600); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened() after the trailing close').to.equal(false); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden').to.equal('true'); + }); + }); + + /** + * A queued open is not a promise to open regardless of what happens next. The page-level close + * paths reach an instance through `VirtualSelect.openInstances`, and a queued instance is not + * in it by default - closeDropbox() removed it before the open was ever queued - so an outside + * click would sail past and the dropdown would spring open ~200ms after the user dismissed it. + */ + it('cancels a queued open when the user clicks outside', () => { + mount(); + open(); + closeThenReopen(); + + cy.get('body').click(5, 5); + + cy.wait(700); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened() after the outside click').to.equal(false); + expect(vs.$dropboxWrapper.getAttribute('aria-hidden'), 'aria-hidden').to.equal('true'); + }); + }); + + /** + * Same rule for the other page-level close: opening a second dropdown closes every other one. + * A queue that survived it would reopen ~200ms later and, through its own "close all others" + * loop, shut the dropdown the user had just opened. + */ + it('cancels a queued open when another dropdown opens', () => { + mount(); + cy.window().then((win) => { + mountVs(win, secondMountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body' }); + }); + open(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition when the reopen arrives').to.equal(true); + $ele[0].open?.(); + }); + + cy.get(`#${secondMountId}`).then(($ele2) => $ele2[0].virtualSelect.openDropbox()); + + cy.wait(800); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${secondMountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${secondMountId}`).should(($ele2) => { + expect($ele2[0].virtualSelect.isOpened(), 'the dropdown the user opened stayed open').to.equal(true); + }); + }); + + /** + * Cancelling a queued open must cancel only the open - not silently rewrite the decision the + * *original* close already made about where focus goes once its own hide finishes. + * + * onDocumentClick() and openDropbox()'s "close all other instances" loop both do + * `instanceObj.shouldFocusWrapperOnClose = false` before calling `closeDropbox()` on every + * entry in openInstances - and a queued instance is in that set (this is exactly what makes + * the previous two cases able to cancel it at all). Before this fix, `closeDropbox()`'s + * pendingOpen branch cancelled the queue but never touched that flag back, so it stayed + * corrupted for the *original* close's own afterHidePopper() - which fires later, once the + * hide that started all this actually finishes. + * + * Reproduced with a second dropdown opening (not an outside click): opening B runs + * "close all others" synchronously, at the very top of B.openDropbox() - well before B's own + * ~300ms show transition could plausibly move focus into B's search input and mask the + * corruption by overwriting document.activeElement first. The assertion reads + * document.activeElement the moment A's own hide finishes (~200ms), safely inside that + * window. + */ + it('does not let a cancelled queue corrupt where the original close sends focus', () => { + mount(); + cy.window().then((win) => { + mountVs(win, secondMountId, { options: makeOptions(50), search: true, dropboxWrapper: 'body' }); + }); + open(); + waitForSearchFocus(); + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + // A's own close: default shouldFocusWrapperOnClose (true) - this close wants focus back. + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + expect(vs.shouldFocusWrapperOnClose, 'A wants focus back, before anything cancels the queue') + .to.equal(true); + + // Something puts focus back inside A's still-visible dropbox mid-fade. + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + + // Queue a reopen of A, then cancel it by opening a second, unrelated dropdown. + $ele[0].open?.(); + }); + + cy.get(`#${secondMountId}`).then(($ele2) => $ele2[0].virtualSelect.openDropbox()); + + /** + * Read document.activeElement in a one-shot .then() the instant A's own hide finishes, not + * inside the retrying .should() above: B's own show transition also completes on its own + * ~300ms timer and steals focus to its search input, and a retry that happens to land after + * that would see B's focus instead of the value under test - passing or failing for the + * wrong reason either way. A single check right after the class appears is the only way to + * observe the moment this method actually runs. + */ + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + expect( + $ele[0].ownerDocument.activeElement, + 'focus once A\'s own close finishes - it wanted focus back, and nothing since then asked for anything else', + ).to.equal(vs.$wrapper); + }); + }); + + /** + * Two reopen routes can converge on one close: the queued open, and a consumer focus handler + * reacting to the wrapper refocus that releaseFocusFromDropbox() performs. Whichever gets there + * first, the dropdown opens once. + */ + it('dispatches beforeOpen once when a focus handler also reopens during the close', () => { + mount(); + open(); + waitForSearchFocus(); + + const events: string[] = []; + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + $ele[0].addEventListener('beforeOpen', () => events.push('beforeOpen')); + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + $ele[0].open?.(); + + /** focus inside, so the hide-end release has something to hand back and fires focusin */ + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + vs.$wrapper.addEventListener('focusin', () => $ele[0].open?.(), { once: true }); + }); + + assertOpenAndVisible(); + + cy.wait(300); + cy.then(() => { + expect(events.length, `beforeOpen count (${events.join(', ')})`).to.equal(1); + }); + }); + + /** + * A consumer focus handler that reopens the dropdown synchronously - the same pattern the + * a11y spec's "does not hide a dropdown a focus handler reopens during the close" pins - runs + * from inside releaseFocusFromDropbox(), which afterHidePopper() calls before its own + * `afterClose` dispatch. By the time that reentrant open returns, isOpened() is already true + * again - but afterClose used to fire anyway, describing a close that the same synchronous + * call stack had already undone. A host cleaning up "now that it's closed" would run that + * cleanup against a dropdown that is, in fact, open. + * + * `beforeOpen`/`afterOpen` are asserted too, so a fix that suppressed afterClose by also + * suppressing the reopen's own events would not pass this by accident. + */ + it('does not dispatch afterClose for a dropdown a focus handler reopened synchronously', () => { + mount(); + open(); + waitForSearchFocus(); + + const events: string[] = []; + + cy.get(`#${mountId}`).then(($ele) => { + const vs = $ele[0].virtualSelect; + + ['beforeOpen', 'afterOpen', 'afterClose'].forEach((name) => { + $ele[0].addEventListener(name, () => events.push(name)); + }); + + vs.closeDropbox(); + expect(vs.isOpened(), 'still mid hide-transition').to.equal(true); + + vs.$dropboxContainer.querySelector('.vscomp-option[data-index="3"]').focus(); + vs.$wrapper.addEventListener('focusin', () => $ele[0].open?.(), { once: true }); + }); + + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + expect($ele[0].virtualSelect.isOpened(), 'reopened').to.equal(true); + }); + + /** + * Retries against the live `events` array rather than a fixed wait: the reentrant open's own + * show transition and its deferred event dispatches take a variable amount of wall-clock + * time (the ~300ms show plus setTimeout(0) scheduling), and a one-shot check timed to "enough + * margin in the common case" is exactly the flaky pattern this suite has hit before. + */ + cy.wrap(null).should(() => { + expect(events, `events (${events.join(', ')})`).to.deep.equal(['beforeOpen', 'afterOpen']); + }); + }); + + /** + * Every other case here mounts with `dropboxWrapper: 'body'` (the portalled layout the + * reported Chrome warning came from), which gives the dropbox its own wrapper element and + * makes `isFocusInsideDropbox()` take its `$dropboxWrapper` branch. The default, + * non-portalled layout takes that helper's `$dropboxContainer` fallback instead - the same + * queue, on the layout most consumers actually use. + * + * Not `assertOpenAndVisible()`: `$dropboxWrapper` is only ever assigned when `dropboxWrapper` + * is set (renderDropbox() leaves it undefined otherwise), and the aria-hidden/tabindex writes + * that helper checks are themselves no-ops on this layout - DomUtils.setAria()/setAttr() both + * guard on a falsy element. There is nothing to carry aria-hidden without a wrapper of its + * own; what this layout still needs is the reopen itself. + */ + it('also queues and replays the open on the default, non-portalled layout', () => { + mount({ dropboxWrapper: 'self' }); + open(); + closeThenReopen(); + + cy.wait(700); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('not.have.class', 'closed'); + cy.get(`#${mountId}`).should(($ele) => { + const vs = $ele[0].virtualSelect; + + expect(vs.isOpened(), 'isOpened()').to.equal(true); + expect(vs.$dropboxContainer.style.display, 'container display').to.not.equal('none'); + expect(vs.$dropboxContainer.getBoundingClientRect().height, 'rendered height').to.be.greaterThan(0); + expect(vs.$wrapper.getAttribute('aria-expanded'), 'aria-expanded').to.equal('true'); + expect($ele[0].ownerDocument.activeElement, 'focus after the reopen').to.equal(vs.$searchInput); + }); + }); + + /** the ordinary path must not be routed through the queue */ + it('still opens immediately when no hide is running', () => { + mount(); + open(); + + cy.get(`#${mountId}`).find('.vscomp-toggle-button').click(); + cy.get(`#${mountId}`).find('.vscomp-wrapper').should('have.class', 'closed'); + + cy.get(`#${mountId}`).then(($ele) => { + $ele[0].open?.(); + + /** synchronous: an open with nothing to wait for must not be deferred */ + expect($ele[0].virtualSelect.isOpened(), 'open applied in the same tick').to.equal(true); + }); + + assertOpenAndVisible(); + }); +}); diff --git a/src/virtual-select.js b/src/virtual-select.js index 2b6a02c..c63eb29 100644 --- a/src/virtual-select.js +++ b/src/virtual-select.js @@ -889,7 +889,17 @@ export class VirtualSelect { onOptionsMouseOver(e) { const $ele = e.target.closest('.vscomp-option'); - if ($ele && this.isOpened()) { + /** + * isClosingTransition: isOpened() stays true for the ~200ms of the popover's hide + * transition, so without the extra gate a pointer resting over the fading list kept + * driving it - re-highlighting options, re-writing aria-activedescendant on a combobox + * that had just announced itself collapsed, and (because taking the highlight takes DOM + * focus) pulling focus back into a subtree about to be marked aria-hidden. A closing + * dropbox ignores the pointer. Clicks and arrow keys are deliberately not gated - that + * would change what mid-fade interactions *do*, where this only stops a passive hover + * from mutating state; see AI-32 in ACTION-ITEMS.md. + */ + if ($ele && this.isOpened() && !this.isClosingTransition) { if (this.shouldSkipOptionInNavigation($ele)) { this.removeOptionFocus(); } else { @@ -1394,6 +1404,10 @@ export class VirtualSelect { this.uniqueId = this.getUniqueId(); this.shouldFocusWrapperOnClose = true; // Initialize focus management property this.isClosing = false; + /** true only for the ~200ms a popover-backed close is actually running - see closeDropbox() */ + this.isClosingTransition = false; + /** an open that arrived mid hide-transition and is waiting for it - see openDropbox() */ + this.pendingOpen = false; /** true from closeDropbox() until the next openDropbox() - see closeDropbox() */ this.isSilentServerSearch = false; this.ariaSetSize = 0; @@ -2983,6 +2997,78 @@ export class VirtualSelect { openDropbox(isSilent) { // Set this instance as the last interacted one immediately VirtualSelect.lastInteractedInstance = this; + + /** + * Queue the open when the popover is still running its hide transition, and let + * afterHidePopper() replay it once the popover is idle again. + * + * PopoverComponent.show() early-returns for as long as `pop-comp-active` is on the element, + * and only its own afterHide removes that class - so opening during the ~200ms fade used to + * do all of this method's work while popper.show() quietly did nothing. afterShowPopper() + * never ran (no `focused` class, no focus moved into the list, no afterOpen), and the hide + * that was still pending then completed on top of it: the dropdown ended up shut with its + * combobox announcing aria-expanded="true". The host's open() was silently lost. + * + * Both state conditions are required. isClosingTransition alone would strand every later + * open if it were ever left set with no hide actually pending; isShown() alone is true of a + * dropdown that is simply already open, where re-opening must stay a no-op rather than a + * queued one. Together they mean exactly "a hide is running and the popover will refuse to + * show". + * + * `!isSilent` is not a fourth safety check, it is the scope of the problem: a silent open + * never calls dropboxPopover.show() at all (see the branch at the end of this method - it + * sets display directly and runs afterShowPopper() synchronously), so the refusal this + * queue works around cannot happen to it. A silent open landing mid-hide would still be + * clobbered, but by something else - the stale afterHidePopper() adding `closed` and + * aria-hidden on top of it - and it would need its own fix, not this one. Deliberately not + * written: no caller passes isSilent to this method, in-tree or through the public + * $ele.open(), and none ever has. Guarding an unreachable path here would be untested code + * defending against a call that does not exist. + * + * The visible cost, measured rather than assumed: the dropbox completes its fade to + * opacity 0 and then fades back in, so a reopen takes hideDuration + showDuration before + * the list is on screen again. It is an opacity dip, not a layout flash - `display: none` + * is never painted, because the replay's removeClass('closed') and the popper restoring + * `inline-flex` both land in the same synchronous block as the hide that set them. + * + * That is a deliberate trade, not a limitation. PopperComponent.show() does start with + * clearTimeout(hideDurationTimeout), so the hide *is* cancellable and the dip could be + * removed by calling `this.dropboxPopover.popper.show({ resetPosition: true })` directly. + * What stands in the way is only PopoverComponent.show()'s own `if (this.isShown()) return` + * one layer above it. Reaching past that means depending on two levels of undocumented + * plugin internals - and every defect this queue exists to fix came from mis-modelling this + * plugin's internal state in the first place. No user gesture can even reach the dip: + * toggleDropbox() reads isOpened() as true for the whole transition, so a second click + * closes again rather than reopening, and only a programmatic close()-then-open() gets + * here. A cosmetic gain on that path is not worth the coupling. + */ + if (!isSilent && this.isClosingTransition && this.dropboxPopover && this.dropboxPopover.isShown()) { + this.pendingOpen = true; + + /** + * Registered as open even though nothing is on screen yet, because that is how the + * page-level closes find an instance: onDocumentClick() and the "close all others" loop + * below both iterate openInstances, and the close that started this hide had already + * removed it. Without this the queue outlives an outside click or a second dropdown + * opening, and the dropdown springs open ~200ms after the user dismissed it - taking the + * one they had just opened down with it, through this very loop. closeDropbox() knows a + * queued instance is not really open and only cancels the queue. + * + * Both of those loops also do `instanceObj.shouldFocusWrapperOnClose = false` immediately + * before calling closeDropbox() on every entry they find - correct for the dropdown they + * are actually closing, wrong for a merely-queued one, whose real close is a separate, + * still-pending hide that already made its own decision about where focus should land. + * Snapshotting it here lets closeDropbox()'s cancel branch undo that mutation rather than + * leaving it to silently overwrite a decision an unrelated, later event has no business + * revisiting. + */ + this.queuedShouldFocusWrapperOnClose = this.shouldFocusWrapperOnClose; + VirtualSelect.openInstances.add(this); + + return; + } + + this.pendingOpen = false; let originalTransition = ''; // Disable transitions for programmatic opening if (!isSilent) { @@ -3015,6 +3101,9 @@ export class VirtualSelect { */ this.isSilentServerSearch = false; + /** a reopen during a still-running hide transition puts the pointer back in charge */ + this.isClosingTransition = false; + DomUtils.setAttr(this.$dropboxWrapper, 'tabindex', '0'); DomUtils.setAria(this.$dropboxWrapper, 'hidden', false); @@ -3079,6 +3168,30 @@ export class VirtualSelect { } closeDropbox(isSilent) { + /** + * An instance whose open is only queued behind a running hide is not open: that hide is + * already doing the closing, so a close aimed at it has nothing to do but cancel the queue. + * + * Before isSilentClose is touched, deliberately. The afterHidePopper() still pending belongs + * to the close that started the hide; overwriting the flag here would hand it this call's + * silentness instead, and its afterClose would go missing (or appear when it should not). + * Returning early also keeps this from dispatching a second beforeClose for a dropdown that + * is already closing. + * + * shouldFocusWrapperOnClose is restored to what it was when the open was queued, undoing + * whatever this call's caller just set it to. onDocumentClick() and the "close all others" + * loop below both write `false` to it immediately before calling closeDropbox() - a decision + * that belongs to the close THIS call would have performed, not to the hide that is actually + * still running for this instance and will read the flag itself once it finishes. + */ + if (this.pendingOpen) { + this.pendingOpen = false; + this.shouldFocusWrapperOnClose = this.queuedShouldFocusWrapperOnClose; + VirtualSelect.openInstances.delete(this); + + return; + } + this.isSilentClose = isSilent; // Remove from open instances @@ -3095,9 +3208,7 @@ export class VirtualSelect { // Return focus to wrapper only when no other meaningful element currently has focus const active = document.activeElement; - const withinComponent = - (active && this.$wrapper.contains(active)) || - (this.hasDropboxWrapper && active && this.$dropboxWrapper.contains(active)); + const withinComponent = (active && this.$wrapper.contains(active)) || this.isFocusInsideDropbox(active); const shouldRefocus = this.shouldFocusWrapperOnClose && VirtualSelect.lastInteractedInstance === this && @@ -3130,17 +3241,39 @@ export class VirtualSelect { this.setActiveDescendant(''); } + /** + * Taking the dropbox out of the tab order happens here, but hiding it from assistive + * technology is left to afterHidePopper() - where the rest of the closed state lands, the + * `closed` class and with it the `display: none` that actually takes the dropbox off the + * screen. + * + * aria-hidden used to be set here too, so for the ~200ms of the popover's hide transition + * the dropbox was marked absent while still visible, still hit-testable and still + * `isOpened() === true`. Every handler gated on isOpened() therefore kept running against a + * subtree already declared hidden - and onOptionsMouseOver() -> focusOption() moves DOM + * focus onto the option it highlights. Chrome refuses to apply aria-hidden over the focused + * element ("Blocked aria-hidden on an element because its descendant retained focus"), so + * the dropbox stayed exposed anyway: the component and the accessibility tree disagreed, + * and a screen reader follows the tree. Selecting an option in a single select was enough + * to hit it - the pointer is still over the list while it fades out. + * + * isClosingTransition covers the transition itself: without it the pointer kept driving + * the fading list - re-highlighting options, re-writing the aria-activedescendant this + * close just cleared onto a combobox already announcing itself collapsed, and pulling DOM + * focus back in. What the pointer gate cannot stop (a host focusing into the dropbox, or + * highlight paths it drives programmatically), releaseFocusFromDropbox() releases at + * hide-end, before the attribute lands. For those 200ms the list genuinely is still on + * screen, so exposing it to AT until it leaves matches what a sighted user sees. tabindex + * stays here because it was never part of the conflict - it does not block a programmatic + * focus() - and moving it would change when the dropbox leaves the tab order. + */ if (this.dropboxPopover && !isSilent) { + this.isClosingTransition = true; this.dropboxPopover.hide(); DomUtils.setAttr(this.$dropboxWrapper, 'tabindex', '-1'); - DomUtils.setAria(this.$dropboxWrapper, 'hidden', true); - DomUtils.setAttr(this.$dropboxContainerTop, 'tabindex', '-1'); - DomUtils.setAria(this.$dropboxContainerTop, 'hidden', true); - DomUtils.setAttr(this.$dropboxContainerBottom, 'tabindex', '-1'); - DomUtils.setAria(this.$dropboxContainerBottom, 'hidden', true); } else { this.afterHidePopper(); } @@ -3191,7 +3324,12 @@ export class VirtualSelect { afterHidePopper() { const isSilent = this.isSilentClose; + /** read before anything can queue a new one, and cleared either way - see openDropbox() */ + const shouldReopen = this.pendingOpen; + this.isSilentClose = false; + this.pendingOpen = false; + this.isClosingTransition = false; DomUtils.removeClass(this.$allWrappers, 'focused'); this.removeOptionFocus(); @@ -3203,14 +3341,69 @@ export class VirtualSelect { DomUtils.addClass(this.$allWrappers, 'closed'); - if (!isSilent) { + /** + * After the closed class, so a focus handler reacting to the wrapper refocus observes a + * dropdown that is really closed, and before the aria-hidden writes below - nothing may + * hold focus inside the subtree when that attribute lands, or Chrome refuses it and the + * dropbox stays exposed to AT. Within this synchronous block the browser has not yet + * recalculated style, so document.activeElement still reports the element inside the + * dropbox even though the closed class will eventually drop focus to - which is + * exactly the focus loss the release turns into a deliberate hand-back. + * + * (This ordering protects the *reader* of activeElement, not afterClose's own dispatch - + * DomUtils.dispatchEvent() defers every event through setTimeout(0), so a consumer's + * afterClose handler itself always runs after this method has finished and cannot reenter + * it.) + */ + this.releaseFocusFromDropbox(); + + /** + * !isOpened() because the focus() call inside the release above can, by itself, reopen the + * dropdown: it fires a synchronous native focusin, and a consumer handler reacting to it by + * calling open() runs to completion (including removing the `closed` class) before control + * returns here. Dispatching afterClose after that would describe a dropdown the same + * synchronous call stack had already reopened - stale the instant it fires, and ordered + * before the reopen's own beforeOpen/afterOpen only because dispatchEvent's setTimeout(0) + * calls preserve scheduling order, not because it happened first. The replay guard just + * below makes the identical check for the identical reason. + */ + if (!isSilent && !this.isOpened()) { DomUtils.dispatchEvent(this.$ele, 'afterClose'); } // Reset for next close this.shouldFocusWrapperOnClose = true; - // Restore accessibility attributes that were inadvertently removed + /** + * The hide has finished, so the popover is idle and will accept a show again: replay the + * open that arrived while it was still running. Placed before the guard below rather than + * after it, so the reopen goes through the one path that already knows not to hide a + * dropdown that is open - openDropbox() leaves isOpened() true, and the guard stands the + * hiding writes down for it exactly as it does for a synchronous focus-handler reopen. + * + * isOpened() because both reopen routes can converge on one close: a consumer focus handler + * reacting to the refocus in releaseFocusFromDropbox() above may already have reopened the + * dropdown synchronously. Replaying on top of that would run a second openDropbox() and + * dispatch a second beforeOpen for a single open. + */ + if (shouldReopen && !this.isDestroyed && !this.isOpened()) { + this.openDropbox(); + } + + /** + * Stand down if the dropdown is open again by the time these writes would run - either the + * replay above, or a focus handler that reopened it synchronously from the wrapper refocus. + * openDropbox() has already made the dropbox visible and focusable in both cases, and + * hiding it now would leave it on screen but absent from the accessibility tree. The next + * close re-applies all of this through its own afterHidePopper(). + * + * A state read rather than a record of what happened, and evaluated last, so it cannot + * disagree with the reopen paths the way an "did someone call open()" flag would. + */ + if (this.isOpened()) { + return; + } + DomUtils.setAttr(this.$dropboxWrapper, 'tabindex', '-1'); DomUtils.setAria(this.$dropboxWrapper, 'hidden', true); @@ -3388,6 +3581,77 @@ export class VirtualSelect { this.toggleFocusedProp(null); } + /** + * Whether the node is inside the dropbox proper - the subtree that is hidden on close. + * + * The portalled wrapper when there is one (`dropboxWrapper` option): it is the element that + * carries aria-hidden and everything in it goes away on close. The container otherwise: it + * lives inside $wrapper, whose toggle button and value display survive a close, so testing + * against $wrapper here would wrongly treat "focus on the combobox itself" as focus that + * needs releasing. + * + * Deliberately narrower than closeDropbox()'s within-component test (which also counts the + * combobox, to decide whether the user's focus deserves restoring), and different again from + * the Escape containment in onKeyDown() (which resolves the element hosting the keydown + * listener). The three answer different questions and are not interchangeable. + * + * @param {Element | null} $node + * @returns {boolean} + */ + isFocusInsideDropbox($node) { + const $root = this.$dropboxWrapper || this.$dropboxContainer; + + return !!$node && $root.contains($node); + } + + /** + * Take DOM focus out of the dropbox at the moment it is actually hidden. + * + * closeDropbox()'s own refocus runs when the close is *requested*; during the hide + * transition focus can re-enter the dropbox (a host focusing into it, or any host-driven + * path ending in focusOption(), which focuses the option it highlights). This runs from + * afterHidePopper(), immediately before the subtree is marked aria-hidden, and hands focus + * back to the combobox (WCAG 2.4.3 Focus Order) - re-asserting the decision the close-time + * refocus already made before something mid-fade overrode it. Without it, the closed + * class's display:none silently drops that focus to . + * + * Two deliberate divergences from closeDropbox()'s refocus guards: + * - no lastInteractedInstance / isSilent conditions: by hide-end those describe the close + * that *started* the transition, not where focus is now. The only question left is "is + * focus about to be trapped inside a hidden subtree" - if it is, it cannot stay there, + * whoever caused the close. + * - shouldFocusWrapperOnClose decides *where* focus goes, not *whether* it moves: when the + * close was caused by focus moving elsewhere (another dropdown opening, an outside + * click), the element is only blurred, so this instance does not steal focus from + * wherever it now belongs. + * + * No "has this instance been reopened" guard, deliberately. It reads as the safe thing to + * add - a stale hide should not yank focus out of a dropdown the user is looking at - but a + * reopen that genuinely put the dropbox back on screen cannot have a hide still pending + * against it: the popover clears `pop-comp-active` before calling back, so a successful + * show() means the hide already finished, and an open arriving before that is queued rather + * than applied (see openDropbox). All such a guard could do is suppress the release on a + * path where the dropbox is being hidden anyway. The caller decides whether the dropbox is + * being hidden; this only makes sure nothing is focused inside it when that happens. + * + * preventScroll because this runs ~200ms after the user's action - if they have scrolled in + * the meantime, focus restoration must not scroll the combobox back into view (the + * deferred re-focus in afterRenderOptions() makes the same call). + */ + releaseFocusFromDropbox() { + const $active = document.activeElement; + + if (!this.isFocusInsideDropbox($active)) { + return; + } + + if (this.shouldFocusWrapperOnClose) { + this.$wrapper.focus({ preventScroll: true }); + } else { + $active.blur(); + } + } + selectOption($ele, { event } = {}) { if (!$ele) { return;