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..c369d3d2d2c --- /dev/null +++ b/src/renderer/src/components/ui/popover-wheel-scroll.test.tsx @@ -0,0 +1,165 @@ +// @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
+
+ ) : ( +
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, 800, 400) + makeScrollable(viewport, 1000, 400) + + wheel(document.querySelector('[data-testid="inner"]')!, 120) + flushFrames() + + expect(viewport.scrollTop).toBe(120) + expect(content.scrollTop).toBe(0) + }) + + it('still scrolls the content itself when it is the scroller', () => { + renderPopover('popover-scroll-content', false) + const content = document.querySelector('[data-slot="popover-content"]')! + // Inline because happy-dom does not apply the stylesheet; in the app + // `.popover-scroll-content` already sets `overflow-y: auto` (main.css). + content.style.overflowY = 'auto' + 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) + }) + + it('ignores vertically clipped elements and horizontal-only scrollers', () => { + renderPopover('popover-scroll-content', true) + const content = document.querySelector('[data-slot="popover-content"]')! + const viewport = document.querySelector('[data-testid="viewport"]')! + content.style.overflowY = 'hidden' + viewport.style.overflowY = 'hidden' + viewport.style.overflowX = 'auto' + makeScrollable(content, 1000, 400) + makeScrollable(viewport, 1000, 400) + Object.defineProperty(viewport, 'scrollWidth', { value: 1000, configurable: true }) + Object.defineProperty(viewport, 'clientWidth', { value: 400, configurable: true }) + + wheel(document.querySelector('[data-testid="inner"]')!, 120) + flushFrames() + + expect(content.scrollTop).toBe(0) + expect(viewport.scrollTop).toBe(0) + }) + + it('does nothing when no element in the target chain can scroll', () => { + renderPopover('popover-scroll-content', true) + const content = document.querySelector('[data-slot="popover-content"]')! + const viewport = document.querySelector('[data-testid="viewport"]')! + content.style.overflowY = 'hidden' + viewport.style.overflowY = 'hidden' + makeScrollable(content, 1000, 400) + makeScrollable(viewport, 1000, 400) + + wheel(document.querySelector('[data-testid="inner"]')!, 120) + flushFrames() + + expect(content.scrollTop).toBe(0) + expect(viewport.scrollTop).toBe(0) + }) + + it('runs for the shim-only marker, which carries no styling', () => { + // The workspace-cleanup Filters panel needs the wheel shim but must NOT inherit + // `.popover-scroll-content`'s 15rem max-height, which would crush its 471px column. + renderPopover('popover-wheel-scroll', true) + const content = document.querySelector('[data-slot="popover-content"]')! + const viewport = document.querySelector('[data-testid="viewport"]')! + viewport.style.overflowY = 'auto' + makeScrollable(content, 400, 400) + makeScrollable(viewport, 1000, 400) + + wheel(document.querySelector('[data-testid="inner"]')!, 120) + flushFrames() + + expect(viewport.scrollTop).toBe(120) + }) +}) diff --git a/src/renderer/src/components/ui/popover.tsx b/src/renderer/src/components/ui/popover.tsx index 5523f5021ee..f76bc2aee8a 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 null +} + function PopoverContent({ className, align = 'center', @@ -55,8 +73,23 @@ function PopoverContent({ return } - const el = event.currentTarget - if (!el.classList.contains('popover-scroll-content') || el.scrollHeight <= el.clientHeight) { + const content = event.currentTarget + // Why two markers: `popover-scroll-content` also imposes a 15rem max-height and its + // own overflow, which a popover that manages its own layout (a fixed-height flex + // column over a pinned footer) must not inherit. `popover-wheel-scroll` opts into + // the shim alone. + if ( + !content.classList.contains('popover-scroll-content') && + !content.classList.contains('popover-wheel-scroll') + ) { + 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 3f4ae9e9ab9..8a48dc10c05 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 @@ -78,7 +78,12 @@ export function WorkspaceCleanupFilterBar({ {/* 471px preserves the 420px facet viewport plus the fixed footer at full height. */}