FE core: OverlayScrollbars — macOS-like overlay scrollbars on all platforms#1487
FE core: OverlayScrollbars — macOS-like overlay scrollbars on all platforms#1487romanivan-flamingo wants to merge 3 commits into
Conversation
… on all platforms OverlayScrollArea (per-container) + GlobalOverlayScrollbars (body) built on overlayscrollbars-react: native scrolling kept, ODS-themed thumb, autoHide on idle. Fine-pointer devices only — touch keeps native scrollbars. Storybook: UI/OverlayScrollArea. Co-Authored-By: Claude Fable 5 <[email protected]>
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughChangesOverlay scrollbar integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant useFinePointer
participant OverlayScrollArea
participant OverlayScrollbars
Browser->>useFinePointer: Detect fine-pointer input
useFinePointer->>OverlayScrollArea: Return pointer capability
OverlayScrollArea->>OverlayScrollbars: Initialize configured overlay scrolling
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx`:
- Around line 26-35: Update useFinePointer and the overlay scroll container so
the host element remains stable across pointer detection, initializing or
destroying OverlayScrollbars in place instead of swapping or remounting the
container after mount. Preserve the existing native behavior when overlay
scrollbars are inactive, and deep-merge the nested scrollbars options so
caller-provided values retain ODS_SCROLLBAR_OPTIONS defaults.
- Around line 75-79: Update the options construction in
OverlayScrollbarsComponent so options.scrollbars is merged with
ODS_SCROLLBAR_OPTIONS.scrollbars instead of being replaced by caller-provided
values. Preserve all other top-level option overrides and ensure the default
theme and auto-hide settings remain when options.scrollbars is partial.
In `@openframe-frontend-core/src/styles/overlay-scrollbars-theme.css`:
- Around line 5-7: Replace the hardcoded scrollbar dimensions in the overlay
scrollbar theme variables, including --os-size, --os-padding-perpendicular, and
--os-padding-axis and the additional flagged values, with existing ODS
spacing/radius tokens or newly defined semantic scrollbar tokens. Preserve the
current sizing behavior while ensuring no raw pixel values remain.
In `@openframe-frontend-core/src/styles/vendor-overlayscrollbars.css`:
- Around line 1-3: Exclude the vendored overlayscrollbars stylesheet from
Stylelint using the project’s existing file-level or Stylelint configuration
mechanism. Keep the upstream-generated CSS unchanged, preserving the wholesale
replacement workflow described in its header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 713def7e-50ad-43b8-82a6-cec562b02ab5
📒 Files selected for processing (7)
openframe-frontend-core/package.jsonopenframe-frontend-core/src/components/ui/index.tsopenframe-frontend-core/src/components/ui/overlay-scroll-area.tsxopenframe-frontend-core/src/stories/OverlayScrollArea.stories.tsxopenframe-frontend-core/src/styles/index.cssopenframe-frontend-core/src/styles/overlay-scrollbars-theme.cssopenframe-frontend-core/src/styles/vendor-overlayscrollbars.css
| return ( | ||
| <OverlayScrollbarsComponent | ||
| className={className} | ||
| options={{ ...ODS_SCROLLBAR_OPTIONS, ...options }} | ||
| {...props} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the component and related scrollbar defaults.
git ls-files 'openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx' \
'openframe-frontend-core/src/components/ui/*' \
'openframe-frontend-core/src/**/*' | sed -n '1,200p'
echo '--- outline ---'
ast-grep outline openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx --view expanded || true
echo '--- file excerpt ---'
nl -ba openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx | sed -n '1,180p'
echo '--- search for ODS_SCROLLBAR_OPTIONS ---'
rg -n "ODS_SCROLLBAR_OPTIONS|scrollbars:" openframe-frontend-core/src -SRepository: flamingo-stack/openframe-oss-lib
Length of output: 13076
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check OverlayScrollbars option semantics in local docs if present.
rg -n "scrollbars|autoHide|theme" . -g '!*node_modules*' -g '!*.lock' | sed -n '1,200p'Repository: flamingo-stack/openframe-oss-lib
Length of output: 26875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx | sed -n '1,220p'
echo '---'
rg -n "const ODS_SCROLLBAR_OPTIONS|ODS_SCROLLBAR_OPTIONS\\s*=|scrollbars\\s*:" openframe-frontend-core/src -SRepository: flamingo-stack/openframe-oss-lib
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx')
text = path.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 1 <= i <= 180:
print(f"{i:4}: {line}")
PYRepository: flamingo-stack/openframe-oss-lib
Length of output: 4808
Deep-merge options.scrollbars here. options={{ ...ODS_SCROLLBAR_OPTIONS, ...options }} replaces the nested scrollbar defaults when callers pass options.scrollbars, dropping the ODS theme and auto-hide settings. openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx:75-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/components/ui/overlay-scroll-area.tsx` around
lines 75 - 79, Update the options construction in OverlayScrollbarsComponent so
options.scrollbars is merged with ODS_SCROLLBAR_OPTIONS.scrollbars instead of
being replaced by caller-provided values. Preserve all other top-level option
overrides and ensure the default theme and auto-hide settings remain when
options.scrollbars is partial.
| --os-size: 8px; | ||
| --os-padding-perpendicular: 2px; | ||
| --os-padding-axis: 2px; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace raw scrollbar dimensions with ODS tokens.
These hand-written 8px, 2px, 4px, and 33px values violate the source-style contract. Reference existing spacing/radius tokens or introduce semantic scrollbar tokens.
As per coding guidelines: “Do not use hardcoded styles… raw hex colors, pixel sizes, and font shorthands are not allowed.” <coding_guidelines>
Also applies to: 14-14, 18-18
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/styles/overlay-scrollbars-theme.css` around lines
5 - 7, Replace the hardcoded scrollbar dimensions in the overlay scrollbar theme
variables, including --os-size, --os-padding-perpendicular, and
--os-padding-axis and the additional flagged values, with existing ODS
spacing/radius tokens or newly defined semantic scrollbar tokens. Preserve the
current sizing behavior while ensuring no raw pixel values remain.
Source: Coding guidelines
| /* Vendored from [email protected] (styles/overlayscrollbars.css). | ||
| Do not edit — replace wholesale on package upgrade. ODS theme lives in | ||
| ./overlay-scrollbars-theme.css */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Exclude the immutable vendor stylesheet from Stylelint.
The vendored copy currently triggers four Stylelint errors at Lines 200, 235, 286, and 350. Since the header requires wholesale upstream replacement, add a file-level/stylelint configuration exclusion instead of modifying generated rules.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/styles/vendor-overlayscrollbars.css` around lines
1 - 3, Exclude the vendored overlayscrollbars stylesheet from Stylelint using
the project’s existing file-level or Stylelint configuration mechanism. Keep the
upstream-generated CSS unchanged, preserving the wholesale replacement workflow
described in its header.
Source: Linters/SAST tools
…containers OverlayScrollArea now renders its own stable viewport div (passed to OverlayScrollbars via elements.viewport): children stay its direct children, refs/onScroll/observer logic work unchanged, overlay handles live on the non-scrolling host. Swapped: DrawerBody, Modal/ModalV2 content, PageContainer detail scroller, notification drawer list, time-tracker panel, chat sidebar/ticket list/history, guide & mingo welcome columns, chat message list. Co-Authored-By: Claude Fable 5 <[email protected]>
Description
OverlayScrollArea (per-container) + GlobalOverlayScrollbars (body) built on overlayscrollbars-react: native scrolling kept, ODS-themed thumb, autoHide on idle. Fine-pointer devices only — touch keeps native scrollbars. Storybook: UI/OverlayScrollArea.
Task
Settle elevator scrollbars: remove custom track styling, fix layout jumps
Summary by CodeRabbit