From c703e2e7ff7c8618a654f295ed3b4dcfd95017c3 Mon Sep 17 00:00:00 2001 From: dev Date: Fri, 22 May 2026 21:40:29 +0200 Subject: [PATCH] =?UTF-8?q?fix(scroll-area):=20split=20mount-vs-visibility?= =?UTF-8?q?=20=E2=80=94=20the=20real=20type=3D'scroll'=20/=20'hover'=20bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: soma/scroll-area-scrollbar.svelte gated the entire
on `state.shouldShow`. For type='hover' / 'scroll', `shouldShow` starts false, so the bar never reached the DOM. With no DOM node: - bind:this never fired → scrollbar's `ref` stayed null - `requestFrame(cb, ref.current)` ran with a null element, so the `mounted` flag never flipped - the hover / drag listeners had no element to attach to - the recipe's `data-state` opacity transition had no element to animate - `shouldShow` could therefore never become true → deadlock Fix — split the concern: - `shouldMount` (new derived) — true whenever the axis overflows OR type='always'. Drives the `{#if}` in the part component. - `shouldShow` — drives `data-state="visible|hidden"`. The recipe transitions opacity. The bar now lives in the DOM whenever it could ever be needed, fades in/out via CSS, and stays a stable hit target for hover / click / drag. Dropped the `mounted` flag and the requestFrame mount-frame scheduling — they only existed to suppress a flash that the opacity transition handles cleanly. Also dropped the inline `border-radius: inherit` on the thumb so the recipe's `--scroll-area-thumb-radius` (the new `radius` prop) wins. Without this the prop was silently overridden to `0px`. Test updates: - removed `cancelFrame` assertion (no mount-frame to cancel) - added `shouldMount` assertion Verified in browser at /uix/components/scroll-area: - type='hover' + vertical: bar mounts hidden, fades in on root hover, fades out after delay - type='scroll' + horizontal: bar mounts hidden, fades in on scroll, thumb tracks scrollLeft, fades out after 600ms - type='always' + both: both bars + corner visible at all times - size=lg, radius=full: 12px-wide bar, fully rounded thumb Co-Authored-By: Claude Opus 4.7 (1M context) --- .../components/scroll-area-scrollbar.svelte | 16 ++++- .../scroll-area-provider.svelte.test.ts | 14 ++-- .../scroll-area-provider.svelte.ts | 67 ++++++++++--------- 3 files changed, 57 insertions(+), 40 deletions(-) diff --git a/src/uix/soma/components/scroll-area/components/scroll-area-scrollbar.svelte b/src/uix/soma/components/scroll-area/components/scroll-area-scrollbar.svelte index 21a10abf8..20293170a 100644 --- a/src/uix/soma/components/scroll-area/components/scroll-area-scrollbar.svelte +++ b/src/uix/soma/components/scroll-area/components/scroll-area-scrollbar.svelte @@ -32,7 +32,21 @@ const mergedProps = $derived(mergeProps(restProps, state.props)); -{#if state.shouldShow || forceMount} + +{#if state.shouldMount || forceMount} {#if child} {@render child({ props: mergedProps })} {:else} diff --git a/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.test.ts b/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.test.ts index e3dc25924..8d2b7afd1 100644 --- a/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.test.ts +++ b/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.test.ts @@ -193,12 +193,11 @@ describe('ScrollAreaProvider', () => { it('controls scrollbar visibility, labels and track clicks with UIX timers', async () => { const harness = installSomaHarness(); - let mountedFrame: FrameRequestCallback | undefined; - vi.spyOn(harness.dom, 'requestFrame').mockImplementation((callback) => { - mountedFrame = callback; - return 7; - }); - const cancelFrame = vi.spyOn(harness.dom, 'cancelFrame'); + // The provider no longer schedules a mount-frame — the bar stays + // in the DOM whenever the axis overflows and `data-state` drives + // opacity. We still spy on requestFrame defensively so any future + // frame-driven work in the constructor surfaces immediately. + vi.spyOn(harness.dom, 'requestFrame').mockImplementation(() => 7); const root = document.createElement('div'); const viewportEl = document.createElement('div'); const scrollbarEl = document.createElement('div'); @@ -229,9 +228,9 @@ describe('ScrollAreaProvider', () => { return { provider, scrollbar }; }); - mountedFrame?.(0); result.scrollbar.show(); expect(result.scrollbar.shouldShow).toBe(true); + expect(result.scrollbar.shouldMount).toBe(true); expect(result.scrollbar.props).toMatchObject({ id: 'scroll-area-scrollbar-y', role: 'scrollbar', @@ -262,7 +261,6 @@ describe('ScrollAreaProvider', () => { await tick(); cleanup(); await tick(); - expect(cancelFrame).toHaveBeenCalledWith(7, scrollbarEl); harness.dom.dispose(); }); diff --git a/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.ts b/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.ts index cac5f254b..0f992d634 100644 --- a/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.ts +++ b/src/uix/soma/components/scroll-area/scroll-area-provider.svelte.ts @@ -227,15 +227,14 @@ export class ScrollAreaScrollbarProvider { readonly provider: ScrollAreaProvider; - // Visibility state + // Visibility state. The bar stays mounted as soon as the + // corresponding axis overflows; `visible` only drives `data-state` + // (which the recipe transitions via opacity). Mounting eagerly is + // what lets the hover/scroll/click listeners attach and the + // transition animations play out — see scroll-area-scrollbar.svelte + // for the prior bug this avoids. visible = $state(false); private hideTimer: TimerHandle | null = null; - // `mounted` MUST be reactive — `shouldShow` reads it to suppress the - // initial-mount flash, so flipping it from false→true during the - // first frame has to invalidate the derived. Without `$state` the - // derived caches the false value and `visible` flips inside the - // `shouldShow` derivation lose the race. - private mounted = $state(false); hovering = $state(false); dragging = $state(false); @@ -249,27 +248,15 @@ export class ScrollAreaScrollbarProvider { context: ScrollAreaScrollbarProvider.ctx }); - // Skip initial-mount flash — defer mounted flag to next frame. - // MUST live inside an `$effect` so `requestFrame` (which reads - // `window`) only runs in the browser. Calling it from the - // constructor blows up SSR with `dom::window_required`. + // Lifecycle cleanup — kill any pending hide timer on unmount. $effect(() => { - const mountedFrame = this.provider.soma.dom.requestFrame(() => { - this.mounted = true; - }, opts.ref.current); - return () => { - this.provider.soma.dom.cancelFrame(mountedFrame, opts.ref.current); - this.clearHideTimer(); - }; + return () => this.clearHideTimer(); }); // Reveal the scrollbar on parent scroll for 'scroll' and 'hover' types. - // The first run of an `$effect` is a subscription-setup pass, not a real - // scroll event — without the `firstRun` guard the initial run would call - // `show()` immediately, setting `visible=true` before `mounted` flips. - // Subsequent real-scroll runs would then see `visible` already at `true` - // and the no-op assignment wouldn't invalidate `shouldShow`, leaving the - // bar stuck hidden until the hide timer expired. + // The first run of an `$effect` is a subscription-setup pass with + // scrollTop=scrollLeft=0 — skip it via `firstRun` so we don't show + // the bar on mount; only real scroll deltas should trigger. let firstRun = true; $effect(() => { void this.provider.scrollTop; @@ -314,17 +301,32 @@ export class ScrollAreaScrollbarProvider { return this.isVertical ? this.provider.hasOverflowY : this.provider.hasOverflowX; } - /** Whether the scrollbar should be displayed based on type and state. */ + /** + * Whether the scrollbar's DOM node should exist. Conservative — + * keep it mounted whenever the axis overflows (or `always`), so + * hover / scroll listeners can hit a stable target and the + * `data-state` opacity transition has an element to animate. + */ + readonly shouldMount = $derived.by(() => { + const type = this.provider.opts.type.current; + if (type === 'always') return true; + return this.hasOverflow; + }); + + /** + * Whether the scrollbar should be visible (drives `data-state` → + * the recipe transitions opacity). Mount-vs-visibility split lets + * `type='hover'` keep the bar in the DOM while fading it out. + */ readonly shouldShow = $derived.by(() => { const type = this.provider.opts.type.current; if (type === 'always') return true; if (!this.hasOverflow) return false; - // Skip initial mount to prevent flash - if (!this.mounted && (type === 'scroll' || type === 'hover')) return false; if (type === 'auto') return true; - if (type === 'hover') return this.visible; - if (type === 'scroll') return this.visible; - return false; + // 'hover' and 'scroll' fall through to the visible flag, which + // flips on the corresponding interaction (hover the root / + // scroll the viewport) and back via the hide timer. + return this.visible; }); /** Show the scrollbar (for hover/scroll modes). */ @@ -557,7 +559,10 @@ export class ScrollAreaThumbProvider { [this.isVertical ? 'top' : 'left']: `${this.thumbOffset}px`, [this.isVertical ? 'height' : 'width']: `${this.thumbSize}px`, [this.isVertical ? 'width' : 'height']: '100%', - 'border-radius': 'inherit', + // Border-radius is owned by the recipe via the + // `--scroll-area-thumb-radius` token. Setting it inline + // would `!important`-shadow the recipe and break the + // `radius` prop. '--scroll-area-thumb-width': `${this.isVertical ? '100%' : `${this.thumbSize}px`}`, '--scroll-area-thumb-height': `${this.isVertical ? `${this.thumbSize}px` : '100%'}` },