Implement scrollbars end to end - #489
Merged
Merged
Conversation
…t off) blitz previously never painted scrollbars: an overflow: auto/scroll container gave no visual indication that it scrolls or where within the content the viewport sits. Draw an overlay thumb (no reserved gutter, so no layout change) per overflowing axis: always for overflow: scroll, only when the content overflows for overflow: auto, never for hidden/clip (programmatic-only scrolling). Geometry derives from Layout::scroll_width/scroll_height and node.scroll_offset; the thumb is drawn unscrolled, above the clipped content, inside the opacity/filter layer. Like other overlay scrollbar UIs (macOS, Firefox on Linux), thumbs only appear while the scroll container is hovered or scrolled away from the origin. Beyond matching platform conventions this keeps thumbs out of static reftest screenshots: an always-visible variant regressed 10 WPT overflow reftests whose reference pages cannot scroll; the hover/scroll rule regresses none. Everything sits behind an off-by-default `scrollbars` cargo feature on blitz-paint, mirrored in the blitz and dioxus-native umbrella crates like the other paint features: zero code and zero behavior change unless enabled. Pixel-level tests in packages/blitz-html/tests/scrollbars.rs (the dev-dependency enables the feature).
Thumb geometry moves from blitz-paint into a shared helper on Node (scrollbar_thumb / wants_scrollbar / scrollbar_drag_ratio) so painting and hit testing cannot drift; blitz-paint now consumes it. Pointer handling gains a DragMode::ScrollbarDrag state: on pointerdown the layout ancestor chain of the hit node is walked for a scroll container whose thumb contains the pointer (the hit test returns the content under the overlay thumb). While dragging, pointer movement maps thumb px to content px via the track ratio and scrolls through the existing clamped scroll_by path (negated: scroll_by uses wheel-delta semantics). Scrollbar drags are not dispatched to the page, and the existing DragMode handling suppresses the synthetic click on release. The document also tracks which thumb the pointer is over (hovered_scrollbar, repainting on change), and the painter renders three thumb states: normal, hovered, and dragged — a dragged thumb also stays visible if the pointer leaves the container mid-drag. The interaction respects the `scrollbars` feature: blitz-dom gains the feature (enabled by blitz-paint/scrollbars), gated at the single thumb- resolution chokepoint (scrollbar_thumb_at), so with the feature off no pointer event is ever claimed for a thumb that is never painted. Tests in packages/blitz-html/tests/scrollbar_drag.rs drive the EventDriver with synthetic pointer events: thumb drags scroll proportionally and clamp at the track end; content drags don't scroll; hover and drag visibly restyle the thumb.
UMCEKO
commented
Jul 3, 2026
UMCEKO
left a comment
UMCEKO
left a comment
Contributor
Author
There was a problem hiding this comment.
Fixed some parts where some functionality was duplicated, rest of the code quality lgtm but idk the conventions of this codebase so there might be some things I could've missed.
Member
|
Are the scrollbars supposed to be so thin? Possibly scale factor ought to be accounted for? (or possibly you've just chose a very thin width): 
|
DO NOT MERGE as-is. stylo 0.18 gates scrollbar-color/scrollbar-width to engine="gecko"; the next commit needs them under the servo engine. This patches to a 0.18.0-identical rev whose only delta gates them behind the layout.unimplemented servo pref instead (the servo/stylo#413 approach; blitz already enables that pref). Replace this commit with a plain stylo version bump once a release ships the change.
scrollbar-color: auto keeps the default gray overlay thumb states; author colors are used exactly at rest, lightened towards white for hover/active, and a track strip is painted when a track color is given. scrollbar-width: none suppresses the scrollbar entirely (painting and thumb-drag hit testing — the check lives in Node::wants_scrollbar); thin uses a 4px thumb instead of 6px. NOTE: requires stylo with the css-scrollbars-1 properties enabled for the servo engine (currently a [patch.crates-io] to a local stylo where the two-line engine gate is removed from longhands.toml — pending upstreaming to servo/stylo). Not pushable until that lands.
Audited against Blink/Chromium (ScrollbarThemeOverlay, NativeThemeAura/ Base, PaintLayerScrollableArea) and adopted what applies: - Author-color hover/active states blend towards whichever pole (black or white) has contrast headroom against the color itself, to a target contrast ratio (1.8 hovered / 1.3 active — Chromium's BlendForMinContrast approach). The previous unidirectional lighten gave invisible feedback for near-white scrollbar-color values. Fully transparent author colors stay untouched, also matching Chromium. - Default thumbs get a thin contrast stroke (fill + stroke, as Chromium paints overlay thumbs) so they stay visible over content close to the fill color, and the default palette follows the viewport color scheme (lighter thumb + dark stroke on dark scheme). - Sub-pixel thumb displacements round up to a whole pixel so any nonzero scroll visibly moves the thumb (Chromium's ThumbPosition rounding). - The bespoke ScrollbarThumb struct is replaced with kurbo::Rect (already a blitz-dom dependency): contains() and scale_from_origin() come free. - FIXME on the thumb hit-test documenting that CSS transforms are not accounted for (a blitz-wide pointer-math limitation, wants a shared transform-aware page->local conversion rather than a scrollbar special case). Deliberate divergences from Chromium, for the record: no drag snap-back (that is Windows theme policy; macOS themes do not snap), no hover thumb-thickening or fade animation yet (needs repaint-timer infra), and overlay track parts remain non-interactive (which Chromium's overlay HitTest also enforces — track click-to-page is classic-scrollbar behavior). Tests: white author thumb still signals hover/drag; default thumb edge carries the stroke; dark scheme paints a distinct thumb.
nicoburns
reviewed
Jul 3, 2026
Review response: relative_luminance is already provided by the color crate (OpaqueColor::relative_luminance), and border.rs already had the same WCAG contrast_ratio. contrast_ratio moves to color.rs (shared with border.rs) and blend_for_contrast joins it, built on the color crate's luminance and lerp_rect instead of hand-rolled equivalents.
Contributor
Author
let me compare it against chromium brb |
Review response: the thumbs were thin. Scale was already applied (DIP constants scaled at paint, the same model as Chromium's scale_from_dip); the constants themselves were small. Chromium's overlay scrollbars (ui/native_theme/overlay_scrollbar_constants.h) use a 10 DIP thumb when hovered/pressed, 1 DIP stroke, 2 DIP edge padding, and a ~32 DIP minimum thumb length, thinning to 0.4x while idle. Since these thumbs only appear in interactive states (hovered or scrolled), adopt the interactive metrics: thumb 6 -> 10 (thin 4 -> 6), minimum length 16 -> 32. Idle-vs-hover thickness animation remains a follow-up.
Contributor
Author
|
Yeah chromium's thumbnail was 10 px thick, min length was 32 px, fixed accordingly in new commit to be in par with chromium. |
Member
|
A suggestion for being able to land this sooner:
Then we could land most of the implementation, and just keep a small diff in a PR that switches in the real style types. (I expect we'll be able to land the PR in Stylo relatively soon, but Stylo releases are monthly and one just went out today so it'll probably be ~a month until that filter down into Blitz) |
Contributor
Author
I'll look into it tmr |
Review suggestion, so the PR can land without waiting on a stylo release: blitz-dom defines its own ScrollbarWidth/ScrollbarColor mirrors of the stylo types, and Node::scrollbar_width()/ scrollbar_color() accessors return hardcoded defaults with the stylo reads left as TODOs (servo/stylo#413 is approved; releases are monthly). The TEMP stylo git patch is gone — the branch builds against crates.io stylo 0.18 again. The five tests that drive the properties through CSS are #[ignore]d with the blocking reason. The switch-over PR is then a small diff: implement the two accessors from the computed style, and un-ignore the tests.
Contributor
Author
|
Nvm couldn't sleep, blitz-dom now has its own ScrollbarWidth/ScrollbarColor mirrors. Stylo reads are left as todos. |
UMCEKO
marked this pull request as ready for review
July 3, 2026 22:00
nicoburns
requested changes
Jul 4, 2026
nicoburns
left a comment
nicoburns
left a comment
Member
There was a problem hiding this comment.
This mostly looks good, but I have some comments.
Comment on lines
+1184
to
+1186
| /// The scrollport (padding box) in (unscaled) CSS px relative to the | ||
| /// node's border-box origin. Taffy has content-box helpers but none for | ||
| /// the padding box. |
Member
There was a problem hiding this comment.
We probably ought to add such helpers to Taffy at some point (but this is low priority).
(I'm also hoping to unify Taffy and Kurbo's geometry type representations at some point (linebender/kurbo#513))
Contributor
Author
There was a problem hiding this comment.
Yeah taffy doesn't have any helpers for the padding box. Happy to upstream a helper to taffy if you'd like tho.
Review round 2:
- Scrollbars are identified by a ScrollbarRef { node_id, axis } struct
(axis: taffy::AbsoluteAxis) instead of (usize, bool) tuples and
`horizontal: bool` parameters.
- All scrollbar code on Node moves to a dedicated node/scrollbar.rs.
- The painter resolves hover/drag state once per scene (fields on
BlitzDomPainter) instead of once per scroll container.
- Thumb hit-testing is integrated into the single top-down Node::hit
descent via an accumulator (mirroring how Blink's hit test carries
the scrollbar on HitTestResult), replacing the per-node upward walk
that recomputed absolute_position per ancestor (O(depth^2)). One
traversal per pointermove now resolves hit node + hovered thumb, and
since the descent already applies inverse transforms per node, thumb
hit regions are correct inside transformed ancestors, resolving the
old FIXME.
…rolled Replaces the non-standard "visible whenever scrolled away from the origin" rule with Chromium's overlay visibility model: - Scrolling shows a container's scrollbars at full opacity; they hold for 500ms after the last activity, then fade out over 200ms (Chromium's overlay timings, ui/native_theme/ overlay_scrollbar_constants.cc). Opacity is a pure function of time-since-last-activity, so no timer infrastructure is needed: BaseDocument::is_animating() keeps frames rendering until pending fade-outs finish, mirroring how the fling animation drives frames. - The pointer resting on a visible thumb (or dragging it) holds the scrollbars shown; leaving the thumb or releasing the drag restarts the fade delay (cc/input/scrollbar_animation_controller.cc). - Faded-out thumbs are not interactive: they don't capture pointer events, and pointer moves never fade scrollbars back in (only scrolling shows them), matching Chromium's Fluent overlay behavior. Tests scroll through the scroll API (which registers activity) instead of writing scroll offsets directly, and cover the new semantics: hover-doesn't-summon, hidden-thumb-doesn't-capture, stale offsets paint nothing, and the opacity ramp itself as a unit test.
nicoburns
approved these changes
Jul 5, 2026
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
2 participants
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.
Implements scrollbar support end to end as per the w3 spec and compared against Blink/Chromium for any edge cases.
Generated via Claude Fable, code quality reviewed manually. PR Desc was handwritten.
Changes made:
scrollbar-colorandscrollbar-width(thin/none) are supportedscrollbarscargo feature, off by defaultNotes
Out of scope / deliberately skipped
scrollbar-gutter