Skip to content

[Spec 1380] artifact-canvas: horizontal multi-column reading mode - #1398

Merged
amrmelsayed merged 60 commits into
mainfrom
builder/spir-1380
Aug 11, 2026
Merged

[Spec 1380] artifact-canvas: horizontal multi-column reading mode#1398
amrmelsayed merged 60 commits into
mainfrom
builder/spir-1380

Conversation

@amrmelsayed

@amrmelsayed amrmelsayed commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Opt-in horizontal multi-column reading mode for the artifact canvas: content flows
top-to-bottom in fixed-height, readable-measure columns continuing rightward (newspaper
flow) in a horizontally scrolling container — an entire spec section visible at once on a
wide display. Vertical stays the default and is behaviorally untouched except for one
deliberate fix (#1396, below). Implemented natively (CSS multicol, zero new runtime
dependencies) across six plan phases, each consulted and committed separately.

Closes #1380
Fixes #1396

Changes

  • Mode core: initialReadingMode/onReadingModeChange props (untrusted values coerced
    to vertical), a token-styled toggle in canvas chrome, the .codev-canvas-mode-horizontal
    CSS layer with new --codev-canvas-column-width/-gap tokens, and mode-switch position
    preservation (viewport-start block anchor, axis-aware restore).
  • Fragmentation protection: break-inside: avoid for pre/table/img/cards/composer
    (descendant-scoped — nested blocks too), with height caps + inner scroll for anything
    taller than a column (Chromium fragments-and-overflows over-tall protected blocks instead
    of honoring break-inside). Unbounded embeds self-bound to the viewport height.
  • Input semantics: native non-passive wheel remap (vertical-dominant unmodified deltas
    only; yields to inner scrollers; trackpad gestures and pinch untouched), PageUp/PageDown
    column paging on measured geometry with the same yield rule, axis-aware scrollIntoView
    for every jump path.
  • Fragment-aware "+": pointer-side anchoring via getClientRects() flow-coordinate
    math — hovering a prose block's continuation fragment lights the affordance in that
    column; keyboard anchors the first fragment; clamping uses flow height, not the union box.
  • Progress + a11y: "Column k of n" readout (live chip + debounced aria-live),
    minimap suppressed in horizontal, the body as a focusable labeled region
    (aria-roledescription), resize re-anchoring.
  • Hosts: VS Code webview — persisted mode bootstrapped via data-reading-mode in the
    initial HTML (the canvas mounts before the first host message), readingModeChange added
    to WebviewToHostMessage, a Memento plumbed through the provider, sanitizeReadingMode
    gating both untrusted directions; vite dev host — localStorage persistence + full
    review-pass wiring. Height contexts in both.
  • artifact-canvas: fenced code blocks miss the #1343 row model — data-line lands on the inner code element, so pre selectors match nothing #1396 root fix: fences now render <pre data-line tabindex><code tabindex> (matching
    code_block), restoring the artifact-canvas: full-row '+' affordance (GitHub-diff pattern) — structurally eliminate the hover travel-gap bug class #1343 row model for code blocks.

Deliberate behavior changes (worth reviewer attention)

Testing

  • 146 package jsdom tests across 16 files, including wheel/paging decision tables, fragment
    math over fabricated rects, mode/coercion/persistence contracts.
  • 31 real-browser (Playwright/Chromium) regression tests over a 1109-line fixture — new
    canvas-browser CI job. Covers fragmentation invariants, reachability, tall-block caps,
    wheel/paging efficacy (incl. no residual vertical scroll), fragment-anchored affordance,
    watch-reload recovery, resize re-anchoring, dark-theme smoke, and the complete
    add → edit → delete review pass in horizontal mode.
  • 751 VS Code unit tests (10 new: sanitize gate, bootstrap embedding incl. injection
    attempt, persistence round-trip through resolveCustomTextEditor).
  • Repo-wide suite green. Interactive dual-host demos happen at dev-approval per the testing
    guide.

Spec

codev/specs/1380-artifact-canvas-horizontal-mul.md

Plan

codev/plans/1380-artifact-canvas-horizontal-mul.md

Review

codev/reviews/1380-artifact-canvas-horizontal-mul.md

…e, horizontal column CSS layer, mode-switch anchoring
…rst tab stop, CSS scoping guard, style conformance
…ction + tall-block caps, Playwright regression fixture, CI wiring
…scendant protection for nested pre/table, 1100-line fixture, reachability + composer-clamp + policy + dark tests
…h inner-scroller yield, measured column paging, axis-aware jumps, focusable card scrollers
…ement in flow coordinates; fences carry data-line on the pre (fixes #1396)
…balState persistence, vite dev host localStorage + height contexts, README
…pair, provider persistence round-trip tests, layout-glue rationale, thread log
…tes the full review pass (markerLine + verified edit/delete + browser test)
…ode column step, typed mode callback, approval frontmatter
@amrmelsayed

Copy link
Copy Markdown
Collaborator Author

Architect Review

APPROVE on the code — the pr gate itself waits on Amr's hands-on demo pass (this is the month's largest UI change; the human checkpoint should be a human reading a real spec in columns, not my summary of one).

Verified across the review passes:

  • The spec's decision records survived implementation intact: measured column geometry (not the raw token), inner-scroll caps for every protected type with the Chromium fragments-AND-overflows finding driving height caps off the JS-published --codev-canvas-column-height, pointer-side fragment anchoring via getClientRects flow-coordinates, minimap suppression, template-embedded mode bootstrap (no vertical flash), prose-max-width inert in horizontal.
  • The artifact-canvas: fenced code blocks miss the #1343 row model — data-line lands on the inner code element, so pre selectors match nothing #1396 root fix is the right one: data-line stamped on the pre (renderer rule), tabindex retained on the inner code as the scroll container — fences are two tab stops by design, which is the accessible answer, and pre[data-line] selectors are meaningful again. The vertical-mode behavior change is called out rather than buried.
  • The staleness merge was handled correctly: 52-commits-behind resolved by merging origin/main; the one conflict (test.yml — my flagged shared-file risk) kept both the streamdeck steps and the new canvas-browser job, verified in the tree; all 7 CI checks running on the merge result.
  • Phase deviations documented honestly (phase-5 growth from a real flake; phase-6 stub-parser gains flagged as shared-fixture changes), governance routed cold with a genuinely comprehensive arch entry, 15 consult rounds across phases with final APPROVEs, and the real-browser suite (31 Playwright tests) already caught one latent production bug (artifact-canvas: fenced code blocks miss the #1343 row model — data-line lands on the inner code element, so pre selectors match nothing #1396) before the feature it guards shipped.
  • On the codex Gemini-lane rebuttal: the builder's handling was correct — porch emitted 2-way per the environment (the agy lane skips non-blockingly when unauthenticated, by design since consult: pre-flight auth-state cache for agy to prevent N-spawn browser-tab burst on unauthenticated state #1077), and strict mode rightly forbids hand-run extras. No config change warranted; the gate proceeds on 2-way + this review.

For Amr's demo pass (the gate decision): vite host — pnpm dev:example in packages/artifact-canvas, toggle top-right; VS Code — Reopen With → Codev Markdown Preview on any spec. Suggested checks: read a long spec in columns; hover/comment across columns incl. a fragmented paragraph; a tall code block's inner scroll; the vertical-fence pass (the one deliberate change to today's behavior); toggle round-trip landing in the same section.


Architect review

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

Labels

None yet

Projects

None yet

1 participant