Skip to content

Commit 085465c

Browse files
committed
Fix: header menu scroll-race that caused drill-up button to vanish in CI
Increase the scroll-to-close guard delay in VirtualizedTableRenderer from 150 ms to 400 ms and the pre-click drain wait in open_header_menu from 300 ms to 500 ms. Under a 4-worker CI run the scroll event fired by scrollIntoView routinely arrives 300–400 ms after the call, well past the old 150 ms guard, closing the menu and stranding locator.evaluate for its full 30 s timeout. Also add an explicit to_be_visible(15 s) guard inside click_menu_item so any future regression surfaces as a clear assertion error instead of a silent hang. Made-with: Cursor
1 parent 789f9e3 commit 085465c

2 files changed

Lines changed: 20 additions & 10 deletions

File tree

e2e_playwright/pivot_table_interactions_test.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,15 @@
3030
ROW_HEADERS_REVENUE_DESC = ["South", "North", "West", "East"]
3131

3232

33-
def click_menu_item(locator) -> None:
34-
"""Click a menu item reliably even when the popover clips its viewport."""
33+
def click_menu_item(locator, timeout: int = 15000) -> None:
34+
"""Click a menu item reliably even when the popover clips its viewport.
35+
36+
Waits for the item to be visible before evaluating so that if the menu
37+
closed between open_header_menu and this call (e.g. due to a stray scroll
38+
event on a loaded CI runner) we get a clear assertion error quickly rather
39+
than a 30 s Locator.evaluate hang.
40+
"""
41+
expect(locator).to_be_visible(timeout=timeout)
3542
locator.evaluate(
3643
"el => { el.scrollIntoView({ block: 'center', inline: 'nearest' }); el.click(); }"
3744
)
@@ -111,19 +118,20 @@ def open_header_menu(page: Page, trigger_locator, menu_test_id: str):
111118
Scroll and click are intentionally separated by a short wait. Even with
112119
``behavior:'instant'``, browsers dispatch scroll events asynchronously — in
113120
Firefox and Chromium they can fire 200 ms+ after the scrollIntoView call.
114-
Our React scroll-to-close listener has a 150 ms grace period after the menu
121+
Our React scroll-to-close listener has a 400 ms grace period after the menu
115122
opens, but if the scroll event arrives later than that it closes the menu
116123
immediately. By scrolling first, waiting for those events to drain, and
117124
only then clicking to open the menu, we ensure no scroll events are
118-
in-flight when the listener registers.
125+
in-flight when the listener registers. 500 ms comfortably outlasts delayed
126+
scroll events on loaded CI runners.
119127
"""
120128
expect(trigger_locator).to_be_attached(timeout=5000)
121129
menu = page.get_by_test_id(menu_test_id)
122130
for attempt in range(2):
123131
trigger_locator.evaluate(
124132
"el => el.scrollIntoView({ block: 'center', inline: 'nearest', behavior: 'instant' })"
125133
)
126-
page.wait_for_timeout(300) # let scroll events drain before opening menu
134+
page.wait_for_timeout(500) # let scroll events drain before opening menu
127135
trigger_locator.evaluate("el => el.click()")
128136
try:
129137
expect(menu).to_be_visible(timeout=5000)

streamlit_pivot/frontend/src/renderers/VirtualizedTableRenderer.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -537,10 +537,12 @@ const VirtualizedTableRenderer: FC<VirtualizedTableRendererProps> = ({
537537
// listen in the capture phase at the document root to catch scroll on the
538538
// wrapper, any descendant overflow container, or the window itself.
539539
//
540-
// A 150 ms guard delay prevents stale scroll events that were queued
541-
// synchronously during the click-to-open sequence (e.g. scrollIntoView
542-
// followed by click) from immediately re-closing the menu before the user
543-
// has had a chance to interact with it.
540+
// A guard delay prevents stale scroll events that were queued during the
541+
// click-to-open sequence (e.g. scrollIntoView followed by click) from
542+
// immediately re-closing the menu before the user has had a chance to
543+
// interact with it. 400 ms is chosen to comfortably outlast the browser's
544+
// async scroll-event dispatch on both fast desktops and loaded CI runners;
545+
// it is imperceptibly short from a UX perspective.
544546
useEffect(() => {
545547
if (!menuTarget) return;
546548
let listener: (() => void) | null = null;
@@ -551,7 +553,7 @@ const VirtualizedTableRenderer: FC<VirtualizedTableRendererProps> = ({
551553
passive: true,
552554
capture: true,
553555
});
554-
}, 150);
556+
}, 400);
555557
return () => {
556558
clearTimeout(timerId);
557559
if (listener)

0 commit comments

Comments
 (0)