[brennanb2025]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​144	$\color{#cf222e}{\Huge{\mathbf{−}}}$​92	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​52
Prod	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​106	$\color{#cf222e}{\Huge{\mathbf{−}}}$​112	$\color{#cf222e}{\Huge{\mathbf{−}}}$​6

Summary

• replace the popover wheel shim's propagation/rAF workaround with a non-passive native listener and synchronous single scroll write
• remove the now-dead frame-cancellation ref plumbing
• move the palette filter wheel marker to each PopoverContent while preserving nested-list styling

Validation

• pnpm run typecheck
• npx oxlint src/renderer/src/components/ui src/renderer/src/components/cmd-j
• pnpm run audit:code-quality:type-aware
• pnpm run lint:react-doctor:changed
• pnpm run verify:localization-catalog
• npx vitest run --config config/vitest.config.ts src/renderer/src/components/ui/popover-wheel-scroll.test.tsx src/renderer/src/components/cmd-j/palette-filter-option-list.test.ts

Electron input validation remains to be run on the pushed branch.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7d332b3e-0a22-4d7e-aacb-da1abeee0c1c

📥 Commits

Reviewing files that changed from the base of the PR and between 0aa5ac0 and 7accb31.

📒 Files selected for processing (2)

• src/renderer/src/components/ui/popover-wheel-scroll.test.tsx
• src/renderer/src/components/ui/popover.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

PopoverContent now manages forwarded refs locally and uses a non-passive native wheel listener for synchronous scrolling. The previous animation-frame helper and its tests were removed. Wheel-scroll tests now verify immediate scrolling, defaultPrevented behavior, consumer cancellation, and ref cleanup. Filter popovers remove popover-scroll-content and add popover-wheel-scroll where required.

Merge Risk: ⚪ Minimal · up to 7accb

This change updates popover wheel handling and related palette markers; no actionable merge-blocking risk remains, so it is merge-ready after normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description provides a concise summary and validation commands, but it omits most required template sections. The linked issue is empty, visual proof is missing, testing details and checklist conf…	Complete the required template sections: ELI5, What Changed, Why, Linked Issue with an issue number, Visual Proof or exactly N/A with a reason, Testing details and platform coverage, AI Disclosure if applicable, Review, Agent skill upstream…
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: replacing the popover wheel handling with a non-passive wheel listener.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description provides a concise summary and validation commands, but it omits most required template sections. The linked issue is empty, visual proof is missing, testing details and checklist confirmations are incomplete, and required considerations are not documented.

Resolution

Complete the required template sections: ELI5, What Changed, Why, Linked Issue with an issue number, Visual Proof or exactly N/A with a reason, Testing details and platform coverage, AI Disclosure if applicable, Review, Agent skill upstream boundary, Notes, and the Checklist. Document the pending Electron input validation or complete it before merging.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[brennanb2025]

Electron QA (session qa-16309, CDP 9371)

Isolated throwaway profile + throwaway repos only. Attached identity: popover-wheel-followup @ brennanb2025/popover-wheel-nonpassive on http://127.0.0.1:5180/. Did not attach to the owner's Orca. No real trackpad input was taken. Every wheel number below is CDP-synthesised (playwright page.mouse.wheel = CDP Input.dispatchMouseEvent mouseWheel, or a synthetic WheelEvent as labelled).

Check 1 — workspace-cleanup Filters panel — PASS

Measured PopoverContent.popover-wheel-scroll at 471px high × 320px wide (h-[min(471px,…)]). Computed max-height was 745.5px (available height), not the 15rem/240px crush from popover-scroll-content. Inner ScrollArea viewport 420px / scrollHeight 2114. Footer (“Showing 1 of 1” + Clear filters) pinned at the bottom (footer.bottom 737.5 vs content bottom 738.5).

Check 2 — headline over-scroll ratio — PASS (ratio 1.0 at native window)

Gesture: hover the Filters ScrollArea viewport, CDP page.mouse.wheel(0, 100).

Setup	Window	Method	ΔscrollTop	ratio
Native Electron window	inner 1728×1013, dpr 1	CDP mouse.wheel δY 100	100	1.0
Repeat, same window	1728×1013	CDP mouse.wheel δY 100	100 (100→200)	1.0
Playwright setViewportSize(600,800) (not a real OS resize; outer stayed 1728×1013)	emulated 600×800	CDP mouse.wheel δY 100	200	2.0
Same emulated 600×800	600×800	synthetic WheelEvent δY 100	100	1.0

On merged main the brief recorded 200px (ratio 2.0) at default and 300px at 600px width. On this branch the comparable native-window CDP gesture is 1.0. I could not take a real 600px window measurement: window.resizeTo is a no-op and Browser.setWindowBounds is not exposed on this page CDP session. The 2.0 under setViewportSize is viewport-emulation, not the same 600px-width run as on main; the shim-only synthetic event stayed 1.0 there.

Emulated 600px (for completeness, not a real window resize):

Check 3 — descendant cancellation — PASS (shim); CDP mouse.wheel overshoots

Product surface: cmd-j PaletteFilterFieldOptions list (overflow-y: auto, 280×456) inside PopoverContent.popover-wheel-scroll. That descendant installs a non-passive listener that preventDefault()s and writes scrollTop.

• Synthetic WheelEvent δY 100 on the inner list: scrollTop 100, defaultPrevented: true, ratio 1.0. The shim did not add a second 100.
• Same surface, CDP mouse.wheel δY 100: 176 (clamped to remaining room). CDP δY 40: 160 (ratio 4.0). That is not 1:1. I am not claiming a trackpad result. The synthetic event is the one that isolates the shim from Chromium's mouseWheel scaling.

Emoji CommandList (also descendant preventDefault) matched: synthetic δY 20 → 20, defaultPrevented: true.

Check 4 — regression sweep — PASS for reached surfaces; Jira issue popover not reachable

Surface	Open	Scroll	Close
Emoji suggestion popover (nested CommandList + popover-scroll-content)	yes	yes (nested scroller 238/292)	yes (Esc / dialog Close)
cmd-j palette Filter (marker moved onto PopoverContent this PR)	yes	yes (280/456)	yes
GitHub Tasks Filters + nested Status	yes	n/a (no overflow)	yes
Linear Tasks Filters	yes	n/a (no overflow)	yes
Jira issue popover	no — throwaway profile, Jira unconnected	—	—

Check 5 — CombinedDiffViewer — PASS for the reachable popover; onWheel is not on PopoverContent

CombinedDiffViewer does not pass onWheel to PopoverContent. onWheel={markDirectScrollInput} is on .combined-diff-scroll-container. The AI-notes PopoverContent has no onWheel and was not reachable (no AI notes). The file-tree Filter diff files popover opened and closed normally (no shim class; not overflowing). Esc closed it. CDP mouse.wheel on the combined-diff scroller did not change scrollTop in this session (native path; not the shim).

Summary

Check	Result
1 Filters height / footer	PASS — 471px, footer pinned
2 Over-scroll ratio	PASS — 1.0 (CDP wheel δY 100 → 100px at 1728×1013). All numbers CDP-synthesised. No trackpad.
3 Descendant cancellation	PASS for the shim (synthetic 1.0). CDP mouse.wheel on that descendant overshot (40→160).
4 Other popovers	PASS emoji, cmd-j Filter, GitHub/Linear task filters. Jira issue popover not reachable.
5 CombinedDiffViewer	PASS file-tree filter popover. onWheel is not on PopoverContent.

[brennanb2025]

Status: review-clean, QA-clean, merging

Review loop. Two rounds. The first found a real blocker: the WeakSet bridge that carries React's synthetic wheel cancellation to the native shim was attached only when the consumer passed an onWheel/onWheelCapture prop — so a descendant's preventDefault() was never recorded, and the shim scrolled anyway. React 19 delegates wheel passively, so a descendant's prevention never reaches the native event the shim inspects; the bubble-phase handlers on PopoverContent are the only place that can observe it. Fixed in 7accb31ebc by attaching them unconditionally, with a descendant-cancellation regression test. The test was confirmed red against the conditional bridge (1 failed / 11 passed) and green after (12/12). A fresh review round then returned clean across all six attack points: deleted tests replaced by equivalent ref/cancellation coverage, React 19 ref cleanup preserved for object and callback refs, no per-mount render state, onWheel still forwarded, both marker classes still split, and nested-scroller resolution intact.

Electron QA (isolated instance, own CDP port, named session, target identity verified): 5/5 PASS. Evidence comment above.

The number this PR existed to settle: on merged main, a CDP wheel of deltaY 100 moved the Filters panel 200px. On this branch the comparable native-window gesture moves 100px — ratio 1.0.

What is not verified, stated plainly:

• No real trackpad input was taken. Every figure is CDP-synthesised. So this PR is justified as removing a provable race — the shim's stopPropagation() left native scroll allowed while its rAF write landed on top — rather than as fixing a symptom measured on hardware.
• No real 600px OS-window measurement: window.resizeTo is a no-op and Browser.setWindowBounds was not exposed on that CDP session. The 2.0 seen under Playwright viewport emulation is emulation, not a real resize; the synthetic-event control stayed 1.0 there.
• The CDP mouse.wheel overshoot on the descendant scroller (δY 40 → 160) is Chromium's own wheel scaling, isolated by the synthetic-WheelEvent control which measured exactly 1.0 with defaultPrevented: true.
• The Jira issue popover was not reachable in the QA rig; covered by code inspection only.

Relationship to #16206: that PR made the Filters panel wheel-scrollable for the first time, which is what exposed this pre-existing race from #14629. #16206 was merged but had not shipped in any tagged release, so this lands before either reaches users.