From 09f70e6862b762ea699bf19093541e94c27c3874 Mon Sep 17 00:00:00 2001 From: David Pine <7679720+IEvangelist@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:02:54 -0500 Subject: [PATCH 1/2] Harden CustomSelect against a stale open-time scroll dismissing the menu The footer CustomSelect dismisses its menu on any page scroll so the fixed listbox never detaches from its trigger. On small touch viewports, tapping the footer control first scrolls it into view; that scroll event can be delivered a frame after open() runs, even though the page already settled, so the freshly opened menu was immediately dismissed. This made the "language selector stays open while its listbox is scrolled" E2E flake on the mobile-chromium project (desktop/tablet scroll far less and never hit it). Capture the window scroll offset when the menu opens and ignore scroll events that report that same already-settled offset; only dismiss once the page actually moves underneath the menu. Existing behavior is preserved (scrolling inside the listbox keeps it open, a real page scroll and resize still close it), and a deterministic regression assertion covers the stale same-offset scroll. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/components/CustomSelect.astro | 42 ++++++++++++++----- src/frontend/tests/e2e/ui-regressions.spec.ts | 9 ++++ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/frontend/src/components/CustomSelect.astro b/src/frontend/src/components/CustomSelect.astro index cb718b68f..57651a028 100644 --- a/src/frontend/src/components/CustomSelect.astro +++ b/src/frontend/src/components/CustomSelect.astro @@ -148,6 +148,11 @@ const hasIcon = Astro.slots.has('icon'); ); let typeahead = ''; let typeaheadTimer = 0; + // Scroll offset captured when the menu opens, used to distinguish a real + // page scroll (dismiss) from a stale scroll event still in flight from the + // gesture that opened the menu (keep open). See handleWindowScroll below. + let openScrollX = 0; + let openScrollY = 0; const isOpen = () => root.hasAttribute('data-open'); @@ -233,6 +238,8 @@ const hasIcon = Astro.slots.has('icon'); function open() { if (isOpen()) return; + openScrollX = window.scrollX; + openScrollY = window.scrollY; document.dispatchEvent(new CustomEvent(openEventName, { detail: root })); listbox.hidden = false; if (typeof listbox.showPopover === 'function') { @@ -350,14 +357,29 @@ const hasIcon = Astro.slots.has('icon'); const handleOtherSelect = (event: Event) => { if (event instanceof CustomEvent && event.detail !== root) close(); }; - const handleViewportChange = (event: Event) => { - // Scrolling the page (or resizing) should dismiss the menu, but scrolling - // *inside* the fixed, overflowing listbox must not. A capture-phase scroll - // listener on window also fires for scrolls targeted at the listbox - // (e.g. wheel/touch, or option.scrollIntoView() during keyboard nav), so - // ignore any scroll that originates within the menu. + const handleViewportResize = () => close(); + const handleWindowScroll = (event: Event) => { + // Scrolling the page should dismiss the menu so the fixed listbox never + // detaches from its trigger, but two kinds of scroll must NOT close it: + // 1. Scrolls that originate *inside* the fixed, overflowing listbox + // (wheel/touch, or option.scrollIntoView() during keyboard nav). A + // capture-phase window listener also fires for those. + // 2. A scroll event still in flight from the gesture that revealed and + // opened the control. Tapping the footer selector scrolls it into + // view first; on small touch viewports that scroll event can be + // delivered a frame later — after open() — even though the page has + // already settled. Such stale events report the same offset we + // captured when opening, so only dismiss once the page actually + // moves underneath the menu (with a couple of pixels of slop to + // absorb sub-pixel rounding on high-DPI mobile viewports). const target = event.target; if (target instanceof Node && listbox.contains(target)) return; + if ( + Math.abs(window.scrollX - openScrollX) <= 2 && + Math.abs(window.scrollY - openScrollY) <= 2 + ) { + return; + } close(); }; const handleSync = () => syncFromNative(); @@ -369,8 +391,8 @@ const hasIcon = Astro.slots.has('icon'); nativeSelect.removeEventListener(syncEventName, handleSync); document.removeEventListener('pointerdown', handleDocumentPointer); document.removeEventListener(openEventName, handleOtherSelect); - window.removeEventListener('resize', handleViewportChange); - window.removeEventListener('scroll', handleViewportChange, true); + window.removeEventListener('resize', handleViewportResize); + window.removeEventListener('scroll', handleWindowScroll, true); close(); delete root.dataset.customSelectInitialized; delete root.syncCustomSelect; @@ -387,8 +409,8 @@ const hasIcon = Astro.slots.has('icon'); nativeSelect.addEventListener(syncEventName, handleSync); document.addEventListener('pointerdown', handleDocumentPointer); document.addEventListener(openEventName, handleOtherSelect); - window.addEventListener('resize', handleViewportChange, { passive: true }); - window.addEventListener('scroll', handleViewportChange, { passive: true, capture: true }); + window.addEventListener('resize', handleViewportResize, { passive: true }); + window.addEventListener('scroll', handleWindowScroll, { passive: true, capture: true }); document.addEventListener('astro:before-swap', handleBeforeSwap, { once: true }); root.syncCustomSelect = syncFromNative; diff --git a/src/frontend/tests/e2e/ui-regressions.spec.ts b/src/frontend/tests/e2e/ui-regressions.spec.ts index eb008230a..6f13a89a7 100644 --- a/src/frontend/tests/e2e/ui-regressions.spec.ts +++ b/src/frontend/tests/e2e/ui-regressions.spec.ts @@ -411,6 +411,15 @@ test('language selector stays open while its listbox is scrolled', async ({ page await expect(languageTrigger).toHaveAttribute('aria-expanded', 'true'); await expect(languageListbox).toBeVisible(); + // Regression: on small touch viewports the tap that scrolls the footer control + // into view can deliver a window `scroll` event a frame after the menu opens. + // Because it reports the same scroll offset the menu was opened at, it must be + // ignored instead of dismissing the freshly opened menu (only a real page + // scroll, below, should close it). + await page.evaluate(() => window.dispatchEvent(new Event('scroll'))); + await expect(languageTrigger).toHaveAttribute('aria-expanded', 'true'); + await expect(languageListbox).toBeVisible(); + const lastOption = options.last(); await expect(lastOption).toBeInViewport(); await lastOption.hover(); From d1c3b2e82ef527e1fe83daf7b087d2293e991aad Mon Sep 17 00:00:00 2001 From: David Pine <7679720+IEvangelist@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:23:55 -0500 Subject: [PATCH 2/2] Scope stale-scroll guard to page scrolls; close on element scrolls Address PR review feedback: - The open-time same-offset guard now only applies to page/document/window scrolls, whose position window.scrollX/Y tracks. An ancestor element scroll (e.g. a modal body) leaves window.scrollX/Y unchanged but still slides the trigger out from under the fixed listbox, so those now always close the menu instead of leaving it detached. - Extract the 2px offset tolerance into a named scrollSettleTolerancePx constant. - Add a deterministic E2E regression asserting an ancestor-element scroll dismisses the open language selector. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/components/CustomSelect.astro | 42 +++++++++++++------ src/frontend/tests/e2e/ui-regressions.spec.ts | 30 +++++++++++++ 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/src/frontend/src/components/CustomSelect.astro b/src/frontend/src/components/CustomSelect.astro index 57651a028..5b0b9f265 100644 --- a/src/frontend/src/components/CustomSelect.astro +++ b/src/frontend/src/components/CustomSelect.astro @@ -127,6 +127,11 @@ const hasIcon = Astro.slots.has('icon'); const openEventName = 'aspire:custom-select-open'; const syncEventName = 'aspire:custom-select-sync'; + // Pixels of slop allowed when comparing the live window scroll offset against + // the offset captured when the menu opened. Absorbs sub-pixel rounding on + // high-DPI mobile viewports so a settled, stale open-time scroll is treated as + // "no movement". See handleWindowScroll. + const scrollSettleTolerancePx = 2; function initializeCustomSelects() { document.querySelectorAll('[data-custom-select]').forEach((root) => { @@ -359,24 +364,37 @@ const hasIcon = Astro.slots.has('icon'); }; const handleViewportResize = () => close(); const handleWindowScroll = (event: Event) => { - // Scrolling the page should dismiss the menu so the fixed listbox never - // detaches from its trigger, but two kinds of scroll must NOT close it: + // Scrolling should dismiss the menu so the fixed listbox never detaches + // from its trigger, but some scrolls must NOT close it: // 1. Scrolls that originate *inside* the fixed, overflowing listbox // (wheel/touch, or option.scrollIntoView() during keyboard nav). A // capture-phase window listener also fires for those. - // 2. A scroll event still in flight from the gesture that revealed and - // opened the control. Tapping the footer selector scrolls it into - // view first; on small touch viewports that scroll event can be - // delivered a frame later — after open() — even though the page has - // already settled. Such stale events report the same offset we - // captured when opening, so only dismiss once the page actually - // moves underneath the menu (with a couple of pixels of slop to - // absorb sub-pixel rounding on high-DPI mobile viewports). + // 2. A stale page scroll still in flight from the gesture that + // revealed and opened the control. Tapping the footer selector + // scrolls it into view first; on small touch viewports that scroll + // event can be delivered a frame later — after open() — even though + // the page has already settled. Such stale events report the same + // offset we captured when opening, so only dismiss once the page + // actually moves underneath the menu (with a couple of pixels of + // slop to absorb sub-pixel rounding on high-DPI mobile viewports). + // + // The stale-offset guard only makes sense for page/document scrolls, + // whose position window.scrollX/Y actually tracks. An ancestor *element* + // scrolling (e.g. a modal body) leaves window.scrollX/Y unchanged yet + // still slides the trigger out from under the fixed listbox, so those + // must always close. const target = event.target; if (target instanceof Node && listbox.contains(target)) return; + const isPageScroll = + target === document || + target === window || + target === document.scrollingElement || + target === document.documentElement || + target === document.body; if ( - Math.abs(window.scrollX - openScrollX) <= 2 && - Math.abs(window.scrollY - openScrollY) <= 2 + isPageScroll && + Math.abs(window.scrollX - openScrollX) <= scrollSettleTolerancePx && + Math.abs(window.scrollY - openScrollY) <= scrollSettleTolerancePx ) { return; } diff --git a/src/frontend/tests/e2e/ui-regressions.spec.ts b/src/frontend/tests/e2e/ui-regressions.spec.ts index 6f13a89a7..37d506bcc 100644 --- a/src/frontend/tests/e2e/ui-regressions.spec.ts +++ b/src/frontend/tests/e2e/ui-regressions.spec.ts @@ -442,6 +442,36 @@ test('language selector stays open while its listbox is scrolled', async ({ page await expect(languageListbox).toBeHidden(); }); +test('language selector closes when an ancestor element scrolls', async ({ page }) => { + await page.goto('/get-started/aspire-vscode-extension/'); + await dismissCookieConsentIfVisible(page); + + const languageTrigger = page.getByRole('combobox', { name: 'Select language' }); + const languageListbox = page.locator('#footer-language-select-listbox'); + + await languageTrigger.click(); + await expect(languageListbox).toBeVisible(); + + // Regression: the open-time guard that ignores a stale, same-offset scroll keys + // off window.scrollX/Y, which do NOT move when an ancestor *element* (e.g. a + // scrollable modal body) scrolls. That scroll still slides the trigger out from + // under the position:fixed listbox, so an element scroll must always dismiss the + // menu — only page/window scrolls are eligible for the stale-offset guard. + const dispatched = await page.evaluate(() => { + const listbox = document.getElementById('footer-language-select-listbox'); + const root = listbox?.closest('[data-custom-select]'); + const ancestor = root?.parentElement; + if (!ancestor || ancestor === document.body || ancestor === document.documentElement) { + return false; + } + ancestor.dispatchEvent(new Event('scroll')); + return true; + }); + expect(dispatched).toBe(true); + await expect(languageTrigger).toHaveAttribute('aria-expanded', 'false'); + await expect(languageListbox).toBeHidden(); +}); + test('shared footer stays contained across docs page layouts', async ({ page }) => { await page.goto('/get-started/first-app/?aspire-lang=typescript'); await dismissCookieConsentIfVisible(page);