From 2d252a4857c162577d60e110f1b7edd6e4aaa529 Mon Sep 17 00:00:00 2001
From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Date: Sun, 23 Aug 2026 19:14:13 -0700
Subject: [PATCH] fix(popover): wheel-scroll a popover whose scroller is
nested, not the content
The workspace-cleanup Filters panel cannot be scrolled with a wheel. Only the
scrollbar drag and focus-scroll work. Two independent Electron QA passes measured
it on main: wheel events over that popover arrive defaultPrevented, panel
scrollTop 0 -> 0, while the candidate list inside the dialog subtree scrolls
normally.
Cause: the popover portals outside the Radix dialog subtree, so
react-remove-scroll's scroll-lock cancels wheel there. #14629 added a shim for
exactly this, but it scrolls `event.currentTarget` -- the PopoverContent -- and
only when that element is itself overflowing. The Filters panel is a flex column
holding a ScrollArea above a pinned footer, so PopoverContent is overflow-hidden
and the real scroller is a descendant. The shim looked at the wrong element and
returned early.
The shim now resolves the nearest scrollable element between the wheel target and
the content, inclusive of both, so it handles the nested-viewport shape as well as
the flat one. Still opt-in via `popover-scroll-content`; popovers without the
class are untouched. The cleanup Filters panel opts in.
This is the unfixed half of the original report. #14629 fixed the panel being
clipped; the wheel -- what you actually reach for -- stayed dead.
The nested test fails against the shipped shim and the other two stay green, so it
reproduces the bug without redefining existing behaviour.
---
.../ui/popover-wheel-scroll.test.tsx | 110 ++++++++++++++++++
src/renderer/src/components/ui/popover.tsx | 30 ++++-
.../workspace-cleanup-filter-bar.tsx | 5 +-
3 files changed, 142 insertions(+), 3 deletions(-)
create mode 100644 src/renderer/src/components/ui/popover-wheel-scroll.test.tsx
diff --git a/src/renderer/src/components/ui/popover-wheel-scroll.test.tsx b/src/renderer/src/components/ui/popover-wheel-scroll.test.tsx
new file mode 100644
index 00000000000..79a50f5b46c
--- /dev/null
+++ b/src/renderer/src/components/ui/popover-wheel-scroll.test.tsx
@@ -0,0 +1,110 @@
+// @vitest-environment happy-dom
+import { act } from 'react'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { Popover, PopoverContent, PopoverTrigger } from './popover'
+
+let root: Root | null = null
+let container: HTMLDivElement
+
+/** happy-dom reports 0 for layout, so scroll geometry has to be defined per element. */
+function makeScrollable(el: HTMLElement, scrollHeight: number, clientHeight: number): void {
+ Object.defineProperty(el, 'scrollHeight', { value: scrollHeight, configurable: true })
+ Object.defineProperty(el, 'clientHeight', { value: clientHeight, configurable: true })
+}
+
+function wheel(el: HTMLElement, deltaY: number): void {
+ act(() => {
+ el.dispatchEvent(new WheelEvent('wheel', { deltaY, bubbles: true, cancelable: true }))
+ })
+}
+
+function renderPopover(contentClassName: string, nested: boolean): void {
+ container = document.createElement('div')
+ document.body.appendChild(container)
+ root = createRoot(container)
+ act(() => {
+ root!.render(
+
+ open
+
+ {nested ? (
+
+ ) : (
+ tall
+ )}
+
+
+ )
+ })
+}
+
+describe('PopoverContent wheel shim', () => {
+ let rafCallbacks: FrameRequestCallback[]
+
+ beforeEach(() => {
+ root = null
+ // The shim defers the scroll write to rAF; capture and flush it explicitly.
+ rafCallbacks = []
+ vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => {
+ rafCallbacks.push(cb)
+ return rafCallbacks.length
+ })
+ vi.stubGlobal('cancelAnimationFrame', () => {})
+ })
+
+ afterEach(() => {
+ if (root) {
+ act(() => root!.unmount())
+ }
+ document.body.replaceChildren()
+ vi.unstubAllGlobals()
+ })
+
+ const flushFrames = (): void => {
+ const pending = rafCallbacks
+ rafCallbacks = []
+ for (const cb of pending) {
+ cb(0)
+ }
+ }
+
+ it('scrolls a nested viewport when the content itself cannot scroll', () => {
+ // The workspace-cleanup Filters panel: a flex column whose PopoverContent is
+ // overflow-hidden, with a ScrollArea viewport above a pinned footer.
+ renderPopover('popover-scroll-content', true)
+ const content = document.querySelector('[data-slot="popover-content"]')!
+ const viewport = document.querySelector('[data-testid="viewport"]')!
+ makeScrollable(content, 400, 400)
+ makeScrollable(viewport, 1000, 400)
+
+ wheel(document.querySelector('[data-testid="inner"]')!, 120)
+ flushFrames()
+
+ expect(viewport.scrollTop).toBe(120)
+ })
+
+ it('still scrolls the content itself when it is the scroller', () => {
+ renderPopover('popover-scroll-content', false)
+ const content = document.querySelector('[data-slot="popover-content"]')!
+ makeScrollable(content, 1000, 400)
+
+ wheel(document.querySelector('[data-testid="inner"]')!, 120)
+ flushFrames()
+
+ expect(content.scrollTop).toBe(120)
+ })
+
+ it('leaves popovers that did not opt in alone', () => {
+ renderPopover('', true)
+ const viewport = document.querySelector('[data-testid="viewport"]')!
+ makeScrollable(viewport, 1000, 400)
+
+ wheel(document.querySelector('[data-testid="inner"]')!, 120)
+ flushFrames()
+
+ expect(viewport.scrollTop).toBe(0)
+ })
+})
diff --git a/src/renderer/src/components/ui/popover.tsx b/src/renderer/src/components/ui/popover.tsx
index 5523f5021ee..449b50edeb3 100644
--- a/src/renderer/src/components/ui/popover.tsx
+++ b/src/renderer/src/components/ui/popover.tsx
@@ -18,6 +18,24 @@ function PopoverAnchor(props: React.ComponentProps
}
+/**
+ * Nearest scrollable element between the wheel target and the popover content,
+ * inclusive of both. Returns null when nothing in that chain can scroll.
+ */
+function resolvePopoverScroller(target: EventTarget | null, content: HTMLElement): HTMLElement | null {
+ let node = target instanceof Node ? target : null
+ while (node && node !== content.parentNode) {
+ if (node instanceof HTMLElement && node.scrollHeight > node.clientHeight) {
+ const overflowY = getComputedStyle(node).overflowY
+ if (overflowY === 'auto' || overflowY === 'scroll') {
+ return node
+ }
+ }
+ node = node.parentNode
+ }
+ return content.scrollHeight > content.clientHeight ? content : null
+}
+
function PopoverContent({
className,
align = 'center',
@@ -55,8 +73,16 @@ function PopoverContent({
return
}
- const el = event.currentTarget
- if (!el.classList.contains('popover-scroll-content') || el.scrollHeight <= el.clientHeight) {
+ const content = event.currentTarget
+ if (!content.classList.contains('popover-scroll-content')) {
+ return
+ }
+
+ // Why resolve rather than use currentTarget: a popover whose content is a flex
+ // column with a nested viewport (a ScrollArea above a pinned footer) is itself
+ // overflow-hidden, so the scroller is a descendant under the pointer.
+ const el = resolvePopoverScroller(event.target, content)
+ if (!el) {
return
}
diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx
index 1fcfa2049dc..cedfd293516 100644
--- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx
+++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx
@@ -70,7 +70,10 @@ export function WorkspaceCleanupFilterBar({
{/* 471px preserves the 420px facet viewport plus the fixed footer at full height. */}