Skip to content

web: pin Connection Info modal header while body scrolls#1755

Merged
cliffhall merged 1 commit into
v2/mainfrom
v2/1754-conninfo-sticky-header
Jul 24, 2026
Merged

web: pin Connection Info modal header while body scrolls#1755
cliffhall merged 1 commit into
v2/mainfrom
v2/1754-conninfo-sticky-header

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1754

Follow-up to #1698 / #1753. The Connection Info modal had the same pattern the settings modals had before #1698 — a custom header Group (title + close) inside a Stack in the modal body, using the default <Modal> with no scrollAreaComponent. With a long server instructions section (e.g. the everything server in a typical config), the connection details overflow the viewport, so the whole modal scrolled and the header (title + close) scrolled out of view.

Fix

Switched ConnectionInfoModal.tsx to the compound Modal.Root / Modal.Overlay / Modal.Content / Modal.Header / Modal.Body API, moving the header row into Modal.Header (which Mantine renders position: sticky; top: 0) and setting scrollAreaComponent={ScrollArea.Autosize} so only the body scrolls. The fade-down transition is already supplied app-wide by ThemeModalRoot (added in #1753), so no transitionProps literal is needed. Props-only, no CSS.

Verification

Storybook ConnectionInfoModal (WithOAuth, ~620px viewport) via Playwright. Measured in-page: the body scroll container is scrollHeight 1172 × clientHeight 558 (scrolls internally), header position: sticky flush to the dialog top after scrolling.

Scrolled to top Scrolled down
Header above the first section. Body scrolled to Server Instructions / OAuth Details — the same header stays pinned.

More detail in pr-screenshots/README.md.

Tests

Existing ConnectionInfoModal unit tests pass unchanged (5 tests); the file stays above the ≥90 coverage gate. npm run ci passes (validate → coverage → smoke → Storybook).

🤖 Generated with Claude Code

https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

Same fix as #1698, applied to the Connection Info modal, which had the same
pattern: a custom header Group (title + close) inside a Stack in the modal body,
using the default `<Modal>` with no `scrollAreaComponent`. With a long server
`instructions` section (e.g. the everything server) the details overflow the
viewport and the whole modal scrolled, taking the header out of view.

Switch to the compound `Modal.Root`/`Content`/`Header`/`Body` API, move the
header into `Modal.Header` (sticky by design) and set
`scrollAreaComponent={ScrollArea.Autosize}` so only the body scrolls. The
fade-down transition is supplied app-wide by `ThemeModalRoot` (added in #1753),
so no `transitionProps` literal is needed here.

Closes #1754

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Jul 24, 2026
@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 47s —— View job


Code Review — Connection Info sticky header


Verdict: LGTM. ✅ Clean, low-risk, props-only change that faithfully mirrors the pattern established for the settings modals in #1698/#1753.

What's good

  • Correct structural fix. Moving the header into Modal.Header (which Mantine renders position: sticky; top: 0) plus scrollAreaComponent={ScrollArea.Autosize} confines overflow to Modal.Body — the right mechanism, and identical to ServerSettingsModal.tsx:241-297.
  • Transition claim verified. The comment says the fade-down transition comes from ThemeModalRoot app-wide; confirmed in src/theme/Modal.ts:25-29 (Modal.Root.extend({ defaultProps: { transitionProps: { transition: "fade-down", duration: 200 } } })). No per-call-site transitionProps literal needed — consistent with the sibling modals.
  • Accessibility preserved. Modal.Title still wires aria-labelledby, and CloseButton keeps its explicit aria-label="Close". The withCloseButton={false} prop is correctly dropped (the compound API has no built-in close button to suppress).
  • Imports are tidy. Stack removed (no longer used), ScrollArea added — no dangling imports.
  • Tests unchanged & still meaningful. All 5 existing tests (opened=false, content render, oauth forwarding, close-button click, Escape) exercise the new structure without modification. w="100%" on the header Group is the expected addition so justify="space-between" spans the full header width.

Minor notes (non-blocking, no change requested)

  • The ConnectionInfoContent component doesn't need to change, and doesn't — the scroll container is now the Modal.Body, so the content stays purely presentational. Good separation.
  • No new test asserts the sticky/scroll behavior, but that's consistent with web: pin Server/Client Settings modal header while body scrolls #1753 (jsdom/happy-dom can't measure layout, and the behavior is Mantine's, verified via the Storybook + Playwright screenshots in the PR body). Reasonable to rely on the existing coverage.

Nothing to fix here — the PR does exactly what it says and matches the precedent it cites.
· branch v2/1754-conninfo-sticky-header

@cliffhall

Copy link
Copy Markdown
Member Author

Thanks for the review! Both notes are non-blocking with no change requested — ConnectionInfoContent stays purely presentational (the scroll container is now Modal.Body), and the sticky/scroll behavior isn't observable in happy-dom, so the Storybook + Playwright measurements stand as the proof, consistent with #1753. No changes.

@cliffhall cliffhall linked an issue Jul 24, 2026 that may be closed by this pull request
@cliffhall
cliffhall merged commit e788bcb into v2/main Jul 24, 2026
3 checks passed
@cliffhall
cliffhall deleted the v2/1754-conninfo-sticky-header branch July 24, 2026 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connection Info modal: header scrolls out of view

1 participant