fix(ui): right resize handle no longer covers scrollbar - #465
Merged
Conversation
Commit 3ac3d5b swapped the touch area bias so side="right" extended 8px leftward into the content area, covering the 6px scrollbar. Changed to `left-0 -right-3` so the touch target has zero left encroachment and extends entirely into the sidebar panel instead. For provenance purposes, this commit was AI assisted.
backnotprop
added a commit
that referenced
this pull request
Apr 7, 2026
Wide, translucent, full-length overlay scrollbars replacing the 6px
WebKit rail that users couldn't reliably grab. Click the track to
page-animate toward the click, drag the thumb, no layout shift, Firefox
parity.
Wraps plan-mode scroll containers (main viewer, annotation panel,
sidebar, settings, export modal) in a new <OverlayScrollArea> component
backed by overlayscrollbars-react. The library handles pointer capture,
click-to-jump, hover reveal, auto-hide, RTL, touch, momentum, and
cross-browser consistency. ClickScrollPlugin registered explicitly so
`clickScroll: true` actually pages (otherwise it silently no-ops).
Plumbing:
- New ScrollViewportContext + useScrollViewport() hook so descendants
(TableOfContents, PinpointOverlay, Viewer sticky observer) can reach
the real scrolling element instead of document.querySelector('main'),
which no longer returns the scroll node after wrapping.
- New useOverlayViewport() hook — canonical ref+state+callback pattern
bridging the library viewport into components that need both
imperative access and effect re-runs when the viewport mounts.
- useActiveSection gains an optional scrollElement arg so its
IntersectionObserver root re-attaches when the viewport becomes
available (ref mutations don't retrigger effects).
Theme:
- New .os-theme-plannotator tokens sourced from existing theme CSS
variables (translucent muted-foreground for handle, transparent track
at rest). 10px at rest, 14px on hover. Color + width transitions
only — deliberately does not reintroduce transform/opacity in global
transitions, preserving the abc952f scroll-jank fix.
- Firefox `scrollbar-width: thin` + `scrollbar-color` fallback for
unwrapped surfaces (micro-scrollers, future dockview).
- print.css hides .os-scrollbar during print.
ResizeHandle (#354 regression guard):
- `side="right"` touch area retuned from `-right-3 left-0` to
`-right-3 left-3` to clear the 14px hover scrollbar. Load-bearing
comment added explicitly warning against simplification because #354
has already regressed twice.
Intentionally left native: max-h-24 inline scrollers
(EditorAnnotationCard, AgentsTab, ThemeTab), dropdown menus — a 14px
overlay scrollbar would dominate those UI elements.
Fixes #354
Follow-up to #359, #465 (both fixed #354, which kept regressing)
Preserves #253 bidirectional annotation scroll
Preserves #452 file-switch reset (plan mode portion)
For provenance purposes, this commit was AI assisted.
18 tasks
backnotprop
added a commit
that referenced
this pull request
Apr 7, 2026
* feat(ui): Zed-style overlay scrollbars for plan mode
Wide, translucent, full-length overlay scrollbars replacing the 6px
WebKit rail that users couldn't reliably grab. Click the track to
page-animate toward the click, drag the thumb, no layout shift, Firefox
parity.
Wraps plan-mode scroll containers (main viewer, annotation panel,
sidebar, settings, export modal) in a new <OverlayScrollArea> component
backed by overlayscrollbars-react. The library handles pointer capture,
click-to-jump, hover reveal, auto-hide, RTL, touch, momentum, and
cross-browser consistency. ClickScrollPlugin registered explicitly so
`clickScroll: true` actually pages (otherwise it silently no-ops).
Plumbing:
- New ScrollViewportContext + useScrollViewport() hook so descendants
(TableOfContents, PinpointOverlay, Viewer sticky observer) can reach
the real scrolling element instead of document.querySelector('main'),
which no longer returns the scroll node after wrapping.
- New useOverlayViewport() hook — canonical ref+state+callback pattern
bridging the library viewport into components that need both
imperative access and effect re-runs when the viewport mounts.
- useActiveSection gains an optional scrollElement arg so its
IntersectionObserver root re-attaches when the viewport becomes
available (ref mutations don't retrigger effects).
Theme:
- New .os-theme-plannotator tokens sourced from existing theme CSS
variables (translucent muted-foreground for handle, transparent track
at rest). 10px at rest, 14px on hover. Color + width transitions
only — deliberately does not reintroduce transform/opacity in global
transitions, preserving the abc952f scroll-jank fix.
- Firefox `scrollbar-width: thin` + `scrollbar-color` fallback for
unwrapped surfaces (micro-scrollers, future dockview).
- print.css hides .os-scrollbar during print.
ResizeHandle (#354 regression guard):
- `side="right"` touch area retuned from `-right-3 left-0` to
`-right-3 left-3` to clear the 14px hover scrollbar. Load-bearing
comment added explicitly warning against simplification because #354
has already regressed twice.
Intentionally left native: max-h-24 inline scrollers
(EditorAnnotationCard, AgentsTab, ThemeTab), dropdown menus — a 14px
overlay scrollbar would dominate those UI elements.
Fixes #354
Follow-up to #359, #465 (both fixed #354, which kept regressing)
Preserves #253 bidirectional annotation scroll
Preserves #452 file-switch reset (plan mode portion)
For provenance purposes, this commit was AI assisted.
* feat(review): overlay scrollbars for file tree, sidebar, and PR panels
Wrap the trivial code-review scroll containers in <OverlayScrollArea>:
FileTree, ReviewSidebar content area, AITab chat history, PRCommentsTab
timeline, and the ReviewPRSummary / ReviewPRChecks dock panels.
None of these components read scrollTop / scrollHeight / scrollLeft
directly — all descendant queries use containerRef.current.querySelector
and all scroll-to-target calls use element.scrollIntoView, which walks
up to the nearest scrollable ancestor (now the library viewport). No
plumbing changes required.
AITab was originally scoped to Commit B but an audit of its scroll
effects (jump-to-question + auto-scroll-to-bottom) confirmed it only
uses descendant scrollIntoView, so it ships here.
DiffViewer and LiveLogViewer follow in a separate commit — they
programmatically read/write scrollTop and need explicit viewport
plumbing via useOverlayViewport.
For provenance purposes, this commit was AI assisted.
* feat(review): overlay scrollbars for diff viewer and live logs
Wrap DiffViewer's main scroll container and LiveLogViewer's log pane in
<OverlayScrollArea>, plumbing containerRef through useOverlayViewport
so imperative scroll reads/writes and IntersectionObserver roots land
on the real library viewport, not the OverlayScrollArea host.
DiffViewer:
- `previousScrollFilePathRef` guard added: the file-switch reset
(#452) now only advances the tracking ref once the scroll actually
executed, closing a race where a file switch landing before the
library viewport attached would leave the ref stale while the
scrollTo silently no-oped.
- `viewport` added to every effect dep that reads containerRef.current
(file-switch reset, annotation scroll, search-highlight apply +
swap, scroll-to-match) so they re-run when the viewport mounts.
Without this they'd silently no-op on first paint.
- Split-view sync via @pierre/diffs unaffected — its ScrollSyncManager
attaches scroll listeners to its own internal codeDeletions /
codeAdditions elements inside the content, not the outer scroller.
LiveLogViewer:
- React onScroll replaced with a native addEventListener('scroll', ...)
inside a useEffect keyed on `viewport` because React's onScroll
doesn't bubble across the library's wrapper layers.
- Follow-tail heuristic (`scrollHeight - scrollTop - clientHeight < 40`)
and the auto-scroll-to-bottom assignment both preserved verbatim —
only the ref target changed from the raw div to the library viewport.
Preserves #452 file-switch reset
Preserves #253 bidirectional annotation scroll (diff side)
For provenance purposes, this commit was AI assisted.
* fix(ui): address PR #509 review — viewport delivery and resize handle
Two P1 bugs from the review, plus four correctness/cleanup items.
P1: OverlayScrollbars viewport was never delivered to consumers.
handleOsRef was reading osInstance() synchronously from the React ref
callback, but `defer: true` queues the library's initialization for a
later frame — at ref-callback time, the internal instance ref is still
null. With a stable useCallback the ref never re-fires, so
onViewportReady was never called with a real viewport. Every consumer
of useOverlayViewport/useScrollViewport stayed permanently null:
useActiveSection, TableOfContents.handleNavigate, Viewer sticky
detection, PinpointOverlay, DiffViewer file-switch reset, LiveLogViewer
follow-tail — all silently no-ops.
Fix: deliver the viewport via the library's own `events.initialized`
and `events.destroyed` callbacks, which fire exactly when elements are
ready and when they're torn down. `getViewport()` prefers the tracked
viewport ref over the imperative osInstance() path so late callers
still work.
P1: right-side ResizeHandle touch area was 0px wide.
Touch area is an absolute-positioned child of a w-0 parent, so actual
width = parent - left - right. With side='right' I'd set `left-3
-right-3`, which evaluates to `0 - 12 - (-12) = 0`. The annotation
panel resize handle in plan mode and both right handles in code review
had no draggable region. The visible 4px track has no event handlers,
so the only affordance was cursor-style feedback — drag did nothing.
Fix: revert to `left-0 -right-3` (12px wide, entirely right of the
boundary, no left encroachment into the adjacent scrollbar zone —
which was the original correct value before this branch). Rewrote the
comment to explain the geometry trap instead of protecting the value
that broke it.
Additional fixes:
- OverlayScrollArea: prefers-reduced-motion now reactive via
useSyncExternalStore — OS toggle mid-session propagates to mounted
instances instead of staying frozen at mount-time snapshot.
- OverlayScrollArea: `ref as never` replaced with a narrow cast to
`React.RefCallback<OverlayScrollbarsComponentRef<'div'>>` so future
signature changes get type feedback.
- PinpointOverlay: window resize listener moved above the scroll
viewport guard so it attaches unconditionally. Scroll listener still
requires the viewport (correct). Old code always registered resize
on window; new code was accidentally skipping it when viewport was
null.
- TableOfContents: `className || default` changed to `className ??
default`. An explicit empty string from a caller (SidebarContainer
passing className="" now that it wraps us in an OverlayScrollArea)
should mean "no container styling", not "use the default". The old
|| operator treated "" as falsy and applied the default, leaving
dead overflow-y-auto and unintended backdrop-blur on the nav.
- useOverlayViewport: removed redundant double cast and `?? null`
no-op in the ref setter.
For provenance purposes, this commit was AI assisted.
* fix(ui): print clipping with overlay scrollbar wrappers
When <main> is wrapped in OverlayScrollArea, the library adds its own
attribute-selector CSS rules: `[data-overlayscrollbars~="host"]` gets
`overflow: hidden !important` and `[data-overlayscrollbars-viewport]`
gets `overflow-x/y: hidden` (or scroll) with fixed viewport heights.
These beat our existing `main { overflow: visible !important }` print
override on specificity — attribute selectors outrank tag selectors
even when both use `!important`. Result: long plans printed only the
currently-visible viewport instead of flowing across pages.
Fix: add a print-scoped override that targets the library's attribute
selectors directly, setting overflow:visible, height:auto,
max-height:none, and display:block to defeat both the overflow clip
and the flex layout the library applies to host/padding wrappers.
Verified by printing a multi-page plan in the dev server.
For provenance purposes, this commit was AI assisted.
* fix(ui): persistent overlay scrollbar, remove dead reduced-motion rule
Switch autoHide from 'leave' to 'never'. The previous behavior faded
the scrollbar 800ms after the pointer left the scroll host, which felt
broken in a common interaction pattern: click a TOC entry (pointer in
the sidebar) → trackpad-scroll (pointer still in sidebar) → 800ms idle
→ scrollbar disappears. User had to hover the right edge to bring it
back on every interaction.
Persistent visibility matches the pattern used by every editor-class
technical app (VS Code, Zed, JetBrains, Xcode, Sublime) where the
scrollbar is both a position indicator and a targeting surface for
click-to-jump. Overlay scrollbars cost zero layout space, so "always
visible" has no downside.
Also removes the dead `.os-theme-plannotator .os-scrollbar` selector
from the reduced-motion block. The library applies the theme class
directly to the .os-scrollbar element itself (verified in the library
runtime source, not just CSS), so a descendant selector with a space
matches nothing. The track and handle selectors in the same block
work correctly (they really are descendants) and are preserved.
The component's own prefers-reduced-motion hook and helpers are now
unused (autoHide is unconditionally 'never') and removed. Reduced
motion is still honored for the hover color + width transitions on
track/handle via the CSS @media (prefers-reduced-motion: reduce)
block in theme.css, which the browser evaluates independently.
For provenance purposes, this commit was AI assisted.
* docs(ui): update OverlayScrollArea jsdoc to match persistent-scrollbar behavior
The component header still described the old autoHide:'leave' behavior
and implied the reduced-motion branch was in the component itself.
Neither is true after cf137bf. Rewrite the jsdoc to describe the
current behavior: always visible, no fade, reduced-motion handled via
a CSS media query in theme.css rather than a React branch.
For provenance purposes, this commit was AI assisted.
* fix(ui): recompute scrollbar on content resize (pierre/diffs expand-lines)
When pierre/diffs expanded context lines on a file whose diff previously
fit inside the viewport, the scrollbar stayed hidden until the user
manually scrolled or dragged the split-ratio handle to force a layout
recalculation.
Root cause: OverlayScrollbars' internal content observer doesn't see
the mutation because pierre/diffs renders inside a shadow DOM, and
MutationObserver does not pierce shadow DOM by default. The library's
own host-level size observer doesn't help either — the host (our
<main> / flex-1 container) has a fixed layout size that doesn't
change when content grows.
Fix: track the OverlayScrollbars instance in state and attach a
ResizeObserver to the viewport's first element child in a useEffect.
When the content's layout box grows — which happens even when the
growth originates inside a shadow tree, because layout size
propagates from shadow content to the shadow host — the observer
fires and calls `instance.update(true)` to recompute scrollbar
visibility. The call is debounced through requestAnimationFrame so
the browser commits the new layout before OverlayScrollbars reads
dimensions.
Verified manually in the compiled binary against PR #509 itself:
opening a small file, clicking pierre's expand-lines control, now
reveals the scrollbar immediately. No regression observed in normal
scroll / click-to-jump / file-switch / annotation-click paths.
For provenance purposes, this commit was AI assisted.
* docs(ui): document ResizeObserver content-resize mechanism
Add the content-resize auto-recompute behavior to OverlayScrollArea's
jsdoc header so the "what does this component do" summary is complete.
The inline comment on the effect already explains the mechanism; this
just surfaces it at the top.
For provenance purposes, this commit was AI assisted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
3ac3d5b("sidebar resize handle touch area") swapped the touch area bias so the rightResizeHandle(side="right") extended 8px left into the content area, covering the 6px scrollbar — users could see the scrollbar but couldn't grab itside="right"from-left-2 -right-1toleft-0 -right-3so the touch target has zero left encroachment and extends entirely into the sidebar panelFixes #354 (regression)
Test plan