fix(ui): update table rendering behavior

[sneumannb5]

What

GFM tables stay contained: keyboard-focusable scroll wrapper + sticky . MarkdownTable.tsx plugs into the

slot in Markdown.tsx. Closes #319

Why

Wide/tall tables broke chat-bubble layout. Wrapper scoped to .md-table-wrap — BuilderMarkdown and other .md-view surfaces stay at their
previous default.

Test plan

• npm run lint && npx tsc --noEmit (web-ui)
• vitest run — Markdown.test.tsx green (was red on a4b9d73)
• 20-col table → horizontal scroll inside bubble
• 200-row table → sticky header keeps its divider while scrolling
• Tab into wrapper → arrow-key scroll (WCAG 2.1.1)
• Streaming mid-table → no flicker
• de + en render markdownTable.scrollRegionLabel
• BuilderMarkdown small table → content-width, no stretch

Risk

Pure web-ui/. One new i18n key. No schema / API / env. Sticky border switched to border-collapse: separate + inset-shadow divider
(sticky-collapse bug).

Changes since a4b9d73

• Dropped fullscreen modal (no portal, no framer-motion, no lucide-react, no maximize button) → resolves modal-a11y / focus-trap / inert /
  motion-token / print-button / button-overlap by elimination.
• Markdown.test.tsx → renderWithIntl (CI blocker).
• Dead i18n keys removed (ariaCloseBackdrop, openFullView, modalTitle, ariaClose).
• min-width: max-content scoped to wrapper (BuilderMarkdown regression).
• Sticky divider via inset shadow (sticky+collapsed-border bug).
• Wrapper tabIndex={0} + role="region" + aria-label.

[ConnysCode]

Review: Request changes

Solid core idea — the scroll-wrapper genuinely fixes the out-of-bounds overflow from #319, and the body-portal for the full-view modal is the correct call (assistant bubbles keep a residual transform from the lume-condense animation via animation-fill-mode: both, which would otherwise capture a non-portaled position: fixed). A few things to address before merge, one blocking.

🔴 Blocker — breaks an existing test (CI red)

Rendering a GFM table now mounts MarkdownTable → useTranslations('markdownTable'). app/_components/__tests__/Markdown.test.tsx renders a table with a plain render() (no NextIntlClientProvider), so it now throws. Verified locally on head a4b9d73:

FAIL  Markdown.test.tsx > wraps a highlightTerm occurrence in a violet span
Error: Failed to call `useTranslations` because the context from
       `NextIntlClientProvider` was not found.
 ❯ MarkdownTable app/_components/MarkdownTable.tsx:31:13
Tests  1 failed | 3 passed (4)

Fix: migrate the test to the repo's renderWithIntl (app/_lib/test-utils.tsx), and run the full vitest run (the lint/tsc checkbox is unticked).

🟠 Scope & architecture

• Scope creep vs #319. The issue asks for tables "collapsible to the sides" — the scroll wrapper delivers that. The fullscreen modal + maximize button + sticky thead are extra surface nobody requested; consider shipping the wrapper alone and splitting the modal off.
• App-wide blast radius — not just chat. <Markdown> renders in ≥6 non-test places (app/page.tsx, app/memory/page.tsx, app/routines/[id]/runs/[runId]/page.tsx, CreateIssueButton.tsx, store/InstallButton.tsx, app/store/[id]/page.tsx). The table override + the global .md-view table rules change every markdown table app-wide; a floating "open fullscreen" button now shows on e.g. static store-description tables. Scope it to chat, or own the app-wide change in the description.
• Fourth hand-rolled modal. Duplicates the dialog pattern already in AgentDetailsModal.tsx, OnboardingModal.tsx, ConfirmDialog.tsx (portal + role="dialog"/aria-modal + ESC + header + X). app/_components/ui/ is the primitives home (it has Button.tsx) but no Modal. A shared ui/Modal would stop the divergence (focus handling already differs across these four).

🧪 Tests

No test for the new ~160-LoC component despite a strong vitest convention; a smoke test (open / ESC / portal target) is cheap. (The existing table test must be fixed regardless — see Blocker.)

⚡ Perf framing

The hoisted components object is fine, but with the modal open mid-stream the table children live in two DOM subtrees and reconcile twice per token; useTranslations runs twice per table. Minor, but it cuts against the "streaming-cheap" framing.

PR description — factcheck

Claim	Reality
"Adds two i18n keys"	Four keys per locale; ariaCloseBackdrop is unused (see inline)
"tables in chat bubbles"	Applies app-wide (6 <Markdown> call-sites)
Test-plan checkboxes	All unticked; the table test is in fact red
"no schema / API / env-var touch"	✅ accurate
"framer-motion + lucide-react already in deps"	✅ accurate

Minimum to merge

1. Fix Markdown.test.tsx (→ renderWithIntl) and get CI green.
2. Remove the dead ariaCloseBackdrop key; correct the description (four keys, app-wide).
3. Decide chat-only vs app-wide and scope accordingly.

Strongly recommended: address the modal a11y (focus trap / restore, inert), the sticky-header border bug, and pull the easing from design-tokens.ts. Details inline.

Nits (non-blocking)

• globals.css:508-509 — overflow-wrap: anywhere + word-break: break-word are redundant; the latter is the legacy form and, with tabular-nums, can break long numbers mid-digit. overflow-wrap: break-word alone is gentler.
• globals.css:501-502 — min-width: max-content + width: 100% stretches even small 2-column tables to full width everywhere markdown renders.
• MarkdownTable.tsx:32 — open state survives streaming only for append-only growth; a block-structure change above the table mid-stream can remount MarkdownTable and silently close the modal. Worth scoping the "streaming-safe" claim.
• MarkdownTable.tsx:110 — the motion.div has no key, and the exit fade is dropped if MarkdownTable unmounts while open (inherent to a per-instance portal).
• MarkdownTable.tsx:97 — the ESC handler doesn't preventDefault/stopPropagation, so it propagates to any ancestor keydown handler.
• JSDoc blocks are heavier than the surrounding code's norm (harmless).

(Reviewed against head a4b9d73.)

[Weegy]

Requesting changes. Thanks for tackling the wide/tall table problem. The portal-rendered fullscreen modal and the SSR/hydration deferral are well done, and hoisting the components map to module scope is the right call for streaming.

A few items to address before merge. The first is a behavioral regression on another surface; the rest are correctness/accessibility/cleanup. Details are inline.

1. Regression (Medium): the change to the global .md-view table rule reaches BuilderMarkdown.tsx, which renders GFM tables into .md-view without the new scroll wrapper. See inline comment on globals.css.
2. Sticky header border (Medium): position: sticky + border-collapse: collapse tends to drop the header border on scroll. Please verify in-browser.
3. Maximize button overlap (Medium): the button overlays the top-right header cell and shows on every table.
4. Accessibility (Medium): the scroll container is not keyboard-focusable, and the modal has no focus trap / body scroll lock / focus restoration.
5. Dead i18n key (Low): markdownTable.ariaCloseBackdrop is unused in both locales.

Happy to pair on the CSS scoping fix.

[ConnysCode]

Review: Request changes

One item left before merge; everything else from my previous review is addressed (those threads resolved).

Must change before merge

🟠 MAJOR — the global table rules leak onto BuilderMarkdown. The .md-table-wrap scroll wrapper is added only by the table override in Markdown.tsx. The rewritten .md-view thead th { position: sticky }, .md-view table { border-collapse: separate } and the dropped my-3 / w-full are global, not wrapper-scoped. BuilderMarkdown.tsx renders into .md-view via its own <ReactMarkdown> (it overrides only the ol slot), so its tables get the new rules with no scroll container — the <thead> pins to the builder chat pane (BuilderChatPane.tsx:544, SimpleIntakePane.tsx:241, PreviewChatPane.tsx:506, all overflow-y-auto) and the table loses its margin. This is the cross-surface bleed BuilderMarkdown's own docstring warns against. Scope the rules to .md-table-wrap (.md-table-wrap thead th { position: sticky … }, and move border-collapse / my-3 under .md-table-wrap) — or correct the description and own the builder change. Inline detail below.

Already changed since the last review

• Test migrated to renderWithIntl — CI blocker gone. Verified on 4b34ab0: vitest 4/4 pass, tsc --noEmit clean, eslint clean.
• Modal dropped → focus-trap / inert / motion-token / print-button / button-overlap threads moot (resolved).
• Sticky-header border → border-collapse: separate + inset-shadow divider, as requested (resolved).
• Scroll region → tabIndex={0} + role + aria-label added (resolved).
• Dead ariaCloseBackdrop key removed (resolved).

PR description — factcheck

Claim	Reality
"Wrapper scoped to .md-table-wrap — BuilderMarkdown and other .md-view surfaces stay at their previous default"	❌ Only min-width/width are wrapper-scoped. .md-view thead th (sticky), .md-view table (border-collapse, dropped my-3/w-full) and the .md-view th/td rewrite stay global, so BuilderMarkdown tables do change.
"One new i18n key"	✅ Only markdownTable.scrollRegionLabel, present in de+en, parity passes.
"No schema / API / env"	✅ Accurate.
Test plan (unticked)	lint / tsc / vitest all pass on 4b34ab0 (verified) — tick them.

Minimum to merge

1. Scope the sticky-header rule (and border-collapse / my-3) to .md-table-wrap so non-wrapped .md-view surfaces (BuilderMarkdown) stay unchanged — or correct the description and own the builder change.

Non-blocking: role="region" → role="group" (landmark spam), and add a small MarkdownTable test (25-file vitest convention). Inline.

Nits (non-blocking)

• Markdown.tsx:134 — stale JSDoc "full view toolbar"; no toolbar anymore (modal dropped).
• MarkdownTable.tsx:27 — tabIndex={0} is unconditional, so non-scrolling tables enter the tab order. Defensible per WCAG 2.1.1; noting only.
• Border model (separate + inset-shadow) and the @media print fallback are correct — verified.

(Reviewed against head 4b34ab0.)

[sneumannb5]

changes implemented, see latest commit

[ConnysCode]

Review: Approve

All blocking and major findings from my two prior reviews are addressed and verified on head e3ba05a. Approving.

Already changed since the last review

• MAJOR — the .md-view → BuilderMarkdown CSS leak is fixed. The sticky-header, border-collapse: separate, min-width: max-content and width/margin rules moved from .md-view to .md-table-wrap (globals.css:516-551). BuilderMarkdown renders into .md-view without the wrapper (BuilderMarkdown.tsx:27-41 — ol-only override), so its tables now fall back to the classic .md-view table collapse model: no sticky <thead> pinning to the builder pane, margin intact. The cross-surface bleed its own docstring warns about is gone.
• role="region" → role="group" on the scroll wrapper (MarkdownTable.tsx:28) — no landmark spam.
• Stale "full view toolbar" JSDoc dropped (Markdown.tsx:134).
• Added MarkdownTable.test.tsx (wrapper contract / focusable group + i18n aria-label / className forwarding).
• Test-plan checkboxes ticked.

Verified on a fresh clone at e3ba05a (web-ui/):

vitest run             → 7/7 pass (Markdown 4 + MarkdownTable 3)
tsc --noEmit           → clean
eslint (changed files) → clean
i18n-validate          → OK, 1296 keys (scrollRegionLabel present en+de)

Also ruled out a specificity trap: .lume-prose .md-view table (globals.css:106-113) outranks .md-table-wrap table, but it only sets font-family/font-size, so it doesn't override the wrapper's border/sticky model.

PR description — factcheck

Claim	Reality
"Wrapper scoped to .md-table-wrap — BuilderMarkdown and other .md-view surfaces stay at their previous default"	Substantially accurate now. Sole residual app-wide change: overflow-wrap: break-word on .md-view th/td (globals.css:502) — cosmetic, prevents long-word overflow, not a regression.
"One new i18n key"	✅ scrollRegionLabel, en+de, parity passes.
"No schema / API / env"	✅ Accurate.
Test plan (now ticked)	✅ lint / tsc / vitest verified green on e3ba05a.

Non-blocking (optional)

• MarkdownTable.tsx:27 — tabIndex={0} is still unconditional, so a table that doesn't actually overflow still takes a tab stop. Defensible under WCAG 2.1.1, and you chose this last round — noting only.
• The visual cases (sticky divider holding on scroll, no border doubling on the wrapper edge) rest on your manual test plan; I can't re-run those headlessly.

(Reviewed against head e3ba05a.)