Skip to content

refactor(ui): prose-layer follow-ups from the #546 Phase B three-way review #618

Closed

Description

@Astro-Han

TL;DR

Three independent reviews (Fable subagent, codex xhigh, pi glm-5.2 xhigh) of the Phase B prose pass (refactor/546-prose-polish) confirmed two in-scope bugs — both fixed on that branch — and a set of verified but out-of-scope findings. This issue tracks those so they don't get lost. Natural batching point: the PR6 boundary (tool-result bodies start reusing .maka-prose).

1. External links paint two underlines (user-visible, probe-verified)

markdown-link.css is imported unlayered (styles.css) while chat-message.css sits in layer(components). Unlayered author styles beat all layers, so .maka-markdown-link-external { text-decoration: underline … } silently defeats the layered .maka-prose a { text-decoration: none } — while the layered border-bottom hairline still applies. Every external prose link paints a real underline and the border underline a few px apart, with duplicated hover transitions.

Verified by computed-style probe: textDecorationLine: underline + borderBottomWidth: 1px simultaneously on .maka-markdown-link-external.

Fix shape: bring markdown-link.css into the layer system (it was marked "structurally clean" in the #546 audit, but its cascade position fights the prose layer it decorates).

2. Re-home load-bearing typography before .maka-prose is reused (PR6 prerequisite)

These live on .maka-bubble-assistant, not .maka-prose:

  • line-height: var(--leading-normal) — the WCAG 1.4.12 floor (1.5)
  • word-wrap: break-word — no global overflow-wrap fallback exists
  • color, the 72ch measure cap
  • > :first-child / > :last-child margin trims

A bare .maka-prose tool-result body would get horizontally overflowing URLs/tokens, line-height inherited from surrounding UI (risk of dipping below the 1.5 floor at 13px), untrimmed edge margins, and a 20px top margin when it opens with a heading. Moving the typography onto .maka-prose is behavior-neutral today (in chat, one div carries both classes).

3. Split the prose layer out of chat-message.css + hygiene

chat-message.css now holds bubble geometry, prose typography, code-block chrome, copy button, streaming caret, user-turn meta, and hljs colors in one file. At the PR6 boundary:

  • Extract the .maka-prose rules into their own file (e.g. prose.css), per the one-surface-one-file rule above.
  • Dedupe the two separate .maka-prose code blocks (font-variant-numeric sits mid-heading-ladder, far from the pill block).
  • Prune stale archaeology comments — e.g. the PR-MESSAGE-BODY-READING-RHYTHM-0 comment still claims h1/h2 letter-spacing is tightened to -0.012em; the actual rule is var(--tracking-normal) (= 0).

4. Pre-existing P3s (verified in review, need measurement or a decision)

  • hljs dark-theme contrast: derived colors subtract lightness in both themes (calc(l - 0.15) strings, - 0.1 built-ins); in dark themes this lowers contrast — plausibly under 4.5:1 at 12–13px on the dark code background. Needs actual contrast measurement.
  • img has no prose rule: Tailwind preflight (display: block; max-width: 100%) breaks inline images mid-paragraph. Rare in chat, likelier in tool results.
  • Focus indicators: prose links use outline: none + 20%-alpha box-shadow ring; task-list checkboxes are appearance: none — weak against WCAG non-text contrast and invisible in forced-colors. App-wide pattern, decide once.
  • Contract-test parser: cssBlocks (markdown-prose-contract.test.ts) emits a spurious 50% block from @keyframes nesting. Benign today; fragile if a keyframe selector ever collides with an assertion regex.

5. display: block tables strip a11y table semantics (introduced by the Phase B PR — accepted trade-off, tracked here)

Correction from the merge-gate review: this is not pre-existing. The Phase B PR moved prose tables from default display: table to GitHub's shrink-wrap shape (display: block; width: max-content; max-width: 100%; overflow-x: auto), which drops the implicit table/row/cell ARIA roles in Chromium — screen readers lose table navigation. GitHub's markdown CSS carries the identical trade-off, and it was accepted for the merge.

Proper fix (when someone wants it): wrap <table> in a scroll <div> at the markdown component layer (markdown-body.tsx table override) and restore display: table — semantics come back and the shrink-wrap/scroll behavior is preserved. Fits naturally with item 3's PR6-boundary work.

Provenance

Three-way review of refactor/546-prose-polish; in-scope fixes landed as 6068f57, 7db0e46, 26d8c66, a27e6d5 (tbody-scoped table reset, caret suppression under the truncation badge, contract-gap closures).

Activity

  1. Astro-Han commented on Jul 7, 2026

    @Astro-Han
    ContributorAuthor

    Status after PR #626

    Items 1–5 all landed in #626 (45f59fe): layered markdown-link.css, self-contained .maka-prose, prose.css split, per-theme pinned hljs lightness (measured min 4.59:1), table ARIA scroll wrapper + img inline fix. Body line-height decided: stays 1.5.

    Scope change: the PR6 boundary this issue anchored on (tool-result bodies reusing .maka-prose) was explored and then deliberately reverted — redact-then-render-markdown turns the renderer into a decode oracle against redactSecrets, and a survey of opencode/pi/codex/gemini-cli confirmed raw tool output should render literally. Tool results stay <pre>; the assistant/system bubble remains the only .maka-prose consumer.

    New follow-up: markdown list rendering

    Owner-observed after #626: list display in the chat markdown body is off (details TBD — to be reproduced and scoped tomorrow). Recording here so it doesn't get lost.

    Queued alongside it (separate workstream, likely its own issue once scoped): tool-activity display simplification (default to a one-line "tool + intent + status" row, per the peer-tool survey) and a broader streaming-output rework.

    Still open from item 4: focus-indicator decision (app-wide), contract-test @keyframes parser nit, optional hljs contrast-ratio lock test.

  2. Astro-Han commented on Jul 9, 2026

    @Astro-Han
    ContributorAuthor

    Closing as done.

    Landed in #626 (merged 2026-07-07), covering the five follow-ups from the Phase B prose review:

    1. External-link double underline — markdown-link.css imported into layer(components)
    2. Load-bearing typography moved onto .maka-prose (self-contained for reuse)
    3. Prose rules split into prose.css + hygiene
    4. hljs contrast (measured ≥4.5:1) + inline prose images
    5. Table ARIA semantics restored (scroll wrapper + display: table)

    Optional leftover from that PR (not blocking): pin hljs contrast ratios in a contract test rather than live probe only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions