MotirBuilding in public
MOTIR · moooon
onMotir
You’re viewing a public project. Anyone can view it — no account needed. Sign in to submit, upvote, or comment on requests.View-only — you can’t edit work items
MOTIR-2491

Sweep the `Modal.Body`-bypass class app-wide — measure every `<Modal>` call site that owns no scroll container, fix the ones that clip, and add a guard so there is no fourth instance

To Do
Description

Type · code (sweep + structural guard) · Parent · root sibling (the two known instances, MOTIR-462 and MOTIR-2488, are both root-level) · Discovered in · the MOTIR-2488 diagnosis, which found the class had already been fixed once at a single site with no sweep.

This card is a MEASUREMENT first, a fix second

It is deliberately NOT "wrap every modal in Modal.Body". Most of the call sites below are short confirm dialogs that fit in any viewport, and wrapping them changes rendered markup for no reason. The deliverable is: know which ones clip, fix those, and make the next one impossible to ship silently.

The defect class

Modal's panel caps its own height and clips — packages/design-system/src/components/ui/Modal.tsx:45:

flex max-h-[90vh] flex-col overflow-hidden

Modal.Body (Modal.tsx:238-256) owns the flex min-h-0 flex-1 flex-col overflow-y-auto recipe that makes that survivable. A call site whose children are a bare <div> / <form> instead gets a flex item with min-height: auto, which cannot shrink below its content — so the overflow is clipped by the panel and no scrollbar appears anywhere. Whatever sits at the bottom (usually the footer, i.e. the primary action) becomes unreachable by any means.

Two confirmed instances, both found by a human using the product:

  • MOTIR-462 — the complete-sprint success modal's burndown, clipped off the bottom. Fixed at that one site.
  • MOTIR-2488 — the create-API-token modal's Cancel + Create token footer, clipped entirely out of view. Fixed at that one site.

Steps

  1. Enumerate the current call sites — do NOT trust a count copied from this card; re-derive it:

    for f in $(grep -rlE "<Modal($|[ >/])" --include="*.tsx" app components | sort -u); do
      grep -q "Modal\.Body" "$f" || echo "$f"
    done
    
  2. Measure, don't guess. For each site, render the modal at a realistic short viewport (1280×700 is what MOTIR-2488's regression test uses) and compare the content height against the panel's 90vh. A site whose content fits at 700px in its TALLEST reachable state is fine as-is — record it as measured-and-fine rather than silently skipping it.

    "Tallest reachable state" is the part that hides instances: MOTIR-2488 only clipped once the account had ≥2 organizations, a shape no fixture rendered. For each candidate ask what makes it taller — a conditional row, a long list, a validation error, a locale with longer strings — and measure THAT.

  3. Fix the ones that clip with the established pattern (app/(authed)/_components/CreateIssueModal.tsx:222-227): the form/div becomes the flex column, fields go in Modal.Body, footer pinned beside it as a sibling and — for a form — still INSIDE the <form> so type="submit" keeps working.

  4. Add the guard. A structural test asserting that any <Modal> call site over a size threshold uses Modal.Body, or a lint rule, or a shared Modal.Form compound that makes the correct shape the only shape. Prefer removing the footgun over documenting it: the primitive's doc comment ALREADY warns about this failure and it still shipped three times.

Known candidates as of 2026-08-08

The tall ones worth measuring first — each renders a variable-length body: BoardConfigEditor, WorkflowEditor, WidgetConfigModal, AutomationSettings, DiscoveryOnboarding, PublicOverviewEditor. The remainder of the ~40 hits are mostly short confirm dialogs (DeleteSprintDialog, RenameSprintDialog, ArchiveProjectModal, RevokeTokenDialog, …).

Scope BOUNDARY

Sweeps motir-core's own call sites and may add a guard. It does NOT redesign Modal, does NOT change Modal.Body's scroll recipe, and does NOT touch the two already-fixed sites.

Acceptance criteria

  • The enumeration command above is re-run at implementation time and its output recorded on the card — the list, not a count.
  • Every site in that output is classified: fixed, or measured and fits with the viewport and the tallest-state it was measured in named.
  • Each site that clipped has a Playwright assertion using toBeInViewport() on its primary action at a viewport ≤700px tall, failing before its fix and passing after. toBeVisible() is NOT sufficient — a clipped element still has a bounding box and still answers every role query, which is why both prior instances passed their own test suites.
  • A guard exists that fails CI when a new tall <Modal> call site ships without a scroll container, and it is demonstrated failing against a deliberately-broken fixture before being committed.
  • The existing modal E2E coverage still passes unchanged.

Context refs

  • packages/design-system/src/components/ui/Modal.tsx — the panel cap and the Modal.Body recipe.
  • app/(authed)/_components/CreateIssueModal.tsx:222-227 — the canonical correct shape.
  • tests/e2e/api-tokens.spec.tsMOTIR-2488's regression test; the multi-org + short-viewport recipe to copy.
  • Prior instances: MOTIR-462, MOTIR-2488.