feat: render responsive Markdown tables#93
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Style tables rendered by marked with GFM inside .markdown scope: - Scrollable wrapper via display:block + overflow-x:auto on <table> - Full width, collapsed borders, 1px border using --line token - Header rows with --muted-2 background and bold text - Cells with consistent 8px 12px padding - Alternating row backgrounds via color-mix on --muted-2 - Hover highlight on rows - Uses existing ClickClack design tokens, no hardcoded colors
6ad85f9 to
14dc0d5
Compare
|
Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 2:24 AM ET / 06:24 UTC. Summary Reproducibility: not applicable. as a bug-report gate; the PR nevertheless provides a high-confidence authenticated app flow demonstrating wide-table overflow and the improved result. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Merge the exact green head so Markdown tables retain native layout and alignment while wide content remains contained, keyboard reachable, and covered by the authenticated app-flow test. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug-report gate; the PR nevertheless provides a high-confidence authenticated app flow demonstrating wide-table overflow and the improved result. Is this the best way to solve the issue? Yes. A dedicated overflow wrapper preserves native table layout and GFM alignment more safely than changing the table itself into a block scroller, and the patch validates that behavior end to end. AGENTS.md: found, but no applicable review policy affected this item. Codex review notes: model internal, reasoning high; reviewed against 064a46fc73e1. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (5 earlier review cycles)
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: feat: render responsive Markdown tables This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
Summary
Why
Wide Markdown tables could overflow the message layout. Applying scrolling directly to the table also changes it from native table layout, which can break grid sizing. The wrapper provides overflow containment while the table remains
display: table.Real behavior proof
Verified inside the actual authenticated ClickClack app at a 480 x 720 viewport. The proof flow opened
/app, loaded the bootstrapped#generalchannel, sent a wide aligned Markdown table through the real message composer, located the resulting non-pending message row, and horizontally scrolled that row's table.ClickClack, heading#general, routed channel URLMessage bodycomposer andSendbutton.message-row:not(.is-pending)inside the ClickClack timelineclientWidth = 404pxscrollWidth = 671pxoverflow-x: autorole="region",aria-label="Scrollable Markdown table",tabindex="0"display: table, width671.453pxscrollLeft: 0 -> 260ArrowRight, observescrollLeft > 0Before horizontal scroll, inside ClickClack:
After horizontal scroll, inside ClickClack:
Validation
pnpm checkpnpm buildpnpm exec playwright test- 99 passedCLICKCLACK_E2E_PORT=18194 pnpm exec playwright test tests/e2e/markdown-table.spec.ts- 1 passedgit diff --exit-code -- apps/api/internal/webassets/distpatch is correct(0.97)patch is correct(0.94)