Skip to content

fix(ui): opening a dialog shifts a fixed header right by half the scrollbar #1144

Description

@vivek7405

Problem

Opening a <ui-dialog> or <ui-alert-dialog> shifts a position: fixed header to the right by half the scrollbar width. Visible on webjs.dev/ui itself: open the alert-dialog demo and the top navbar jumps.

The scroll lock removes the page scrollbar and compensates by padding the BODY:

packages/ui/packages/registry/components/dialog.ts:174-178
packages/ui/packages/registry/components/alert-dialog.ts:116-120

  const scrollbarWidth = window.innerWidth - document.documentElement.clientWidth;
  document.body.style.overflow = 'hidden';
  if (scrollbarWidth > 0) document.body.style.paddingRight = `${scrollbarWidth}px`;

A position: fixed element is laid out against the initial containing block (the viewport), NOT the body's padding box, so it never receives that compensation. Verified in a real browser on /ui/alert-dialog: with padding-right: 15px applied to body exactly as lockScroll() does, the fixed header's width is unchanged at 1600px while in-flow content is inset.

So on open:

  1. overflow: hidden removes the scrollbar, widening the viewport by its width.
  2. Body padding holds in-flow content still.
  3. The fixed header widens with the viewport, and its centred inner container slides right by half the scrollbar width.

This hits any app with a pinned header, which is the pattern WebJs explicitly recommends (position: fixed, never sticky, per #610 and references/styling.md). So the kit's own guidance and the kit's own dialog disagree.

Reproduces only where the scrollbar takes layout width. Classic scrollbars on Windows and Linux, and macOS with "Show scroll bars: Always". Headless Chromium uses overlay scrollbars, so window.innerWidth - documentElement.clientWidth is 0 there and the whole path is inert. That is why no existing browser test caught it, and it is the thing to get right when writing one.

Design / approach

Two candidates.

A. Publish the compensation as a CSS custom property. During the lock, set --wj-scrollbar-compensation: <width>px on document.documentElement alongside the existing body padding, and clear it on unlock. A fixed element opts in with padding-right: var(--wj-scrollbar-compensation, 0px). Small, backwards compatible, no layout change when no dialog is open. Costs an opt-in line in every app that pins a header, so it needs documenting in references/styling.md next to the fixed-header guidance, and the kit cannot fix an app that does not opt in.

B. Reserve the gutter permanently with scrollbar-gutter: stable on the root in themes/index.css. Then locking never changes the viewport width and no compensation is needed anywhere. This is the modern answer and it fixes every fixed element with no per-app opt-in. Two things to check before taking it: the existing JS compensation must be skipped or it double-compensates (with a stable gutter the innerWidth - clientWidth probe still reports a non-zero width), and reserving the gutter shows a permanent blank strip on short pages in some browsers, which is a visual change to every app that adopts the theme.

Recommend B with A as the escape hatch, but confirm the double-compensation interaction empirically before committing, since the probe's behaviour under scrollbar-gutter is the crux and is easy to get wrong by reasoning alone.

Implementation notes (for the implementing agent)

Where to edit:

  • packages/ui/packages/registry/components/dialog.ts, lockScroll() / unlockScroll() around L170-190.
  • packages/ui/packages/registry/components/alert-dialog.ts, the same pair around L112-132. The two are deliberately kept in lockstep and NOT shared, so webjs ui add alert-dialog stays self-contained. Whatever lands must land in both, identically.
  • packages/ui/packages/registry/themes/index.css if taking option B.
  • .agents/skills/webjs/references/styling.md, the fixed-header section, if taking option A.
  • website/app/layout.ts:348 (class="site-top fixed inset-x-0 top-0 z-20") is the in-repo consumer to verify against.

Landmines / gotchas:

  • The refcount. Both files track scrollLockCount so nested dialogs unlock in order. Any new state must be refcounted the same way or a nested close resets it early.
  • Do not verify in headless Chromium alone. Overlay scrollbars make the bug unreproducible; the assertion needs a forced classic scrollbar or a simulated body padding, and a test that passes under overlay scrollbars proves nothing.
  • useUnifiedScrollbar style flags differ across the browser matrix, so check WebKit as well as Chromium and Firefox.

Invariants to respect:

  • packages/ui/AGENTS.md invariant 2, no third-party runtime dependencies. Hand-roll it.
  • packages/ui/AGENTS.md invariant 5, API parity with shadcn. This is presentation and lifecycle, not API, but record a deliberate divergence in invariant 5 if the fix departs from what shadcn ships.
  • Repo root AGENTS.md invariant 11 for any prose added.

Tests + docs surfaces:

  • packages/ui/test/components/browser/ui-overlay.test.js is where dialog behaviour is asserted. A test must force a measurable scrollbar rather than assuming one exists.
  • .agents/skills/webjs/references/styling.md for the fixed-header guidance, and references/components.md if the lock contract becomes part of the public surface.

Acceptance criteria

  • Opening and closing a dialog or alert-dialog leaves a position: fixed header exactly where it was, on a browser with classic scrollbars
  • In-flow page content still does not shift either
  • Nested dialogs still unlock correctly, with the compensation released only on the outermost close
  • A browser test asserts the fixed-element case and genuinely fails when the fix is reverted, rather than passing vacuously under overlay scrollbars
  • dialog.ts and alert-dialog.ts stay in lockstep

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions