Skip to content

feat(editor): finish the Author writing surface (#993) - #994

Merged
drawmeanelephant merged 4 commits into
mainfrom
t3code/editor-residual-flat-993
Sep 13, 2026
Merged

drawmeanelephant merged 4 commits into
mainfrom
t3code/editor-residual-flat-993

Conversation

@drawmeanelephant

@drawmeanelephant drawmeanelephant commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Agent Completion Report

  • Status: complete
  • Branch and Worktree:
    • Branch: t3code/editor-residual-flat-993
    • Worktree: t3code/editor-residual-flat-993 (the shared checkout; no separate worktree)
  • Commit and PR:
    • Commits: 3320884d (the Author writing surface), 9e19150e (fragment named for this PR), 65247bca (review corrections + the declared scales)
    • Target PR / Branch: main
  • Linked Issues (auto-close convention):
  • Changed Files (vs main, 18 files, +1458/−63):
    • editor/ui/src/styles.css, editor/ui/src/lib/tokens.css, editor/ui/src/App.svelte
    • editor/ui/src/components/Header.svelte, editor/ui/src/components/SectionNav.svelte
    • editor/ui/src/lib/state/connection.svelte.ts, editor/ui/src/lib/state/buffer.svelte.ts
    • editor/ui/scripts/check-scales.mjs (new)
    • editor/ui/tests/scales.spec.ts (new), density-modes.spec.ts, reading-hierarchy.spec.ts, safe-editing.spec.ts, section-nav.spec.ts
    • editor/ui/package.json, editor/scripts/test-editor-gate.sh
    • editor/README.md, content/guides/editor.md, docs/changelog.d/994-editor-flat-chrome.md
  • Preserved Unrelated Files:
    • Staged by explicit path only, never git add -A. The working tree separately holds uncommitted CLI-reference work (src/cli.zig, content/reference/commands.md, docs/changelog.d/995-cli-reference-current.md) that is not part of this PR and was left untouched; it is being landed as its own PR.
    • The detached editor host started for dogfooding is on its own session (pid 99532) so it can be stopped independently.
  • Implementation Summary:
    • Project is demoted to a file drawer in Author mode: align-self: start defeats the grid stretch so it stops matching the page height, and it drops the card elevation and heavy edge for a recessed neutral surface with tighter padding.
    • The section nav takes the density mode and, in Author, leads with Project and Source while the Review destinations recede into a captioned, faint cluster. They remain live links: activating one still switches modes and lands, pinned by test.
    • Source takes page material: no card elevation, a :focus-within edge instead, and a readable-measure cap (54rem, ~80 monospace columns) centered in its grid column so the pane edge is the page edge and heading, editor, gutter, and status line share one column. The cap is Author-only; Review keeps the wide surface beside its rail. width: 100% is load-bearing: a grid item with any justify-self other than stretch sizes to its content, so without it the pane shrank to ~520px and the cap never bounded the page.
    • The measure was chosen over the alternative padding inset because an inset on a full-width pane re-draws the admin textarea with dead side margins and separates the line-number gutter from the lines it numbers.
    • The top band quiets: the decorative eyebrow is removed, the product mark and the live connection status share one baseline row, the theme control states only its state (Dark/Light, matching its accessible name), and Author takes a smaller product mark plus tighter header and nav bands than Review. Switching the header to align-items: center also let three baseline-compensation margins be deleted.
    • The long connection sentence collapses into a chip: the live region carries the short label and the honest sentence is one activation away, absent from the DOM until then. The chip is a real button so the detail is reachable by keyboard and assistive tech rather than hover-only.
    • connection.status now has a single writer (setStatus(summary, detail)) plus one exported HOST_UNAVAILABLE_DETAIL, because the new test caught buffer.svelte.ts assigning connection.status directly: the chip still read "Connected" while its detail read "Local host unavailable", which is exactly the drift the short label must not have.
  • Review Corrections (65247bca) — four findings from the external review of this PR, each verified against the source before being acted on. All four were confirmed; none were packet drift.
    • The Author app title was rendering at exactly the pane-title size. header[data-density='author'] h1 used --text-xl, the same token as h2, while the comment above it claimed "the order holds everywhere: h1 (app) > h2 (pane)". Fixed by extending the chain rather than documenting an exception, because tokens.css itself says "Extend the chain here instead of adding a local size": a new --text-2xl-compact step sits strictly between --text-xl and --text-2xl at every viewport.
    • The theme control carried aria-pressed alongside a state label. A pressed state presumes a name that does not change with it, so light theme announced as "Light, not pressed". The label is the state here, so aria-pressed is gone, not the label.
    • Author Source drew two rust focus frames: the shell's :focus-within ring plus the textarea's own inset outline. The textarea now keeps no ring inside the page shell.
    • The connection chip's live region wrapped an interactive control. Every other role="status" in editor/ui/src is text-only; this one was the outlier, and a status region is atomic, so the button was re-announced with the state. The live region is now a text-only span with the chip as a sibling.
  • Declared Scales (65247bca):
    • The type chain, the spacing rhythm, the corner steps, and the stacking ladder each write their order down exactly once, in a --scale-* list in editor/ui/src/lib/tokens.css. Nothing else keeps a copy: the tests previously held a SCALE_CHAIN constant, and that duplication is what allowed the app title and the pane titles to disagree while every suite stayed green.
    • --scale-space-base declares the spacing rhythm's 0.25rem base rather than describing it in prose, so the lint and the walk read it instead of trusting a comment.
    • The six bare z-index values in styles.css became --layer-* names, with unchanged values (nav 5, skip link 10, focus overlay 50, and three component-internal layers 0/1/2). The ladder is now the only authority on stacking order. The three local layers are in the same ladder rather than exempted, because an allowlist would have been the hole in the rule.
    • editor/ui/scripts/check-scales.mjs (run by npm run check, and reported by the editor gate as an "editor UI static checks" stage) holds the static half: classification of every declared token of each family, listed names existing, ascent of each list — numerically where a value is resolvable without a viewport and reported as deferred where it is a clamp — whole-multiple spacing steps, bare-number layers, each scale token declared once and only in tokens.css, every var() reference landing on a declared step, no bare-number z-index, and each documented section naming its scale's list without naming tokens that do not exist. It runs its own self-tests first, so the checker is checked.
    • editor/ui/tests/scales.spec.ts holds the half that needs a viewport: it resolves the same lists out of the applied stylesheet at four widths, and asserts the sticky nav and skip link actually carry the declared layers. This is the only place a clamp step can be checked at all — the static lint reports those two type steps as deferred rather than pretending to verify them.
    • Elevation tiers (--shadow-1..3) are deliberately not a declared scale: a shadow list has no scalar order for a list to protect.
    • The prior hierarchy suite pinned Review density only, so no test compared the app title against a pane title in Author. That is the coverage gap the failure came through; the hierarchy test now runs in both modes.
  • Known Gaps:
    • The wide two-column composition was verified by measured geometry and Playwright assertions at 1440px and 1920px, plus a screenshot at a simulated width (root zoom, which does not affect media queries), not by an eyeball dogfood at a real wide viewport — the live preview viewport is 600px. The maintainer dogfood is the acceptance step this PR deliberately does not claim.
    • Expanding the status chip grows the top band by one line. That is intentional for an on-demand disclosure, not an oversight.
    • Assistive-tech users now hear the short connection label instead of the full sentence, since the detail is not in the DOM until expanded. This is a deliberate behaviour change: a host blip no longer re-reads a sentence.
    • The connection sentence is still long copy in its own right; this change moves it, it does not shorten it.
    • Replacing the live region's contents changes what a screen reader announces in the header, which is asserted structurally (text-only region, no interactive descendant) but not verified with a real screen reader.
    • styles.css still holds --focus-size: 1.05rem, an off-scale size. Focus reading typography is deliberately independent of the UI scale, so it is not flagged — which means "no off-scale sizes" has an unstated exception rather than a checked one.
    • The layer ladder names three component-internal layers in a global tokens file. That is a judgement call in favour of one total rule; a reviewer could reasonably read it as over-tokenizing.
    • The browser walk's unique value is narrower than it may sound: spacing and radius are plain rems, so the static lint orders them fully. The walk's irreplaceable job is resolving clamp steps and confirming the real chrome carries the ladder.
  • Exact Commands Run:
    1. npm run check (editor/ui)
    2. npm run build (editor/ui)
    3. npx playwright test (editor/ui)
    4. zig build test
    5. git diff --check
    6. ./scripts/check-pr-issue-close.sh --body-file <this body>
  • Exact Gate Results:
    • npm run check: pass — 0 errors, 0 warnings; key-hints conformance OK (8 visible hints across 24 Svelte sources); scale conformance OK (type 8 steps with 2 deferred to the browser walk, space 8, radius 4, layer 6; no bare z-index)
    • npm run build: pass — 167 modules transformed
    • npx playwright test: pass — 217/217 across 9 files, including the new scales.spec.ts walk and both-mode heading tests
    • zig build test: pass — including the documentation link walk
    • git diff --check: pass — no whitespace errors
    • ./scripts/check-pr-issue-close.sh --body-file: pass
  • Determinism Result:
    • N/A for compiler output; this change touches no Boris compilation path, IR shape, frontmatter grammar, or diagnostic. The Playwright suite is the mocked-host harness and passed identically on repeated runs.
  • Generated Artifacts:
    • editor/ui/dist/ and editor/zig-out/ (both git-ignored, built for verification and dogfooding)
    • Playwright test-results/ (git-ignored)
    • No generated output is staged or committed.
  • Blockers and Next Card:
    • Blockers: None.
    • Next Card: the scales lint governs the declared scales but not values that bypass them — --focus-size: 1.05rem is exactly such a value. A baselined ratchet that fails on a newly introduced raw rem spacing or size, grandfathering the existing ones explicitly, is the natural follow-up.

drawmeanelephant and others added 2 commits September 13, 2026 12:57
The #989-#991 slices made Source the widest pane, but Author still read as an
equal-card dashboard with a full IDE strip above it. This carries the
writing-tool intent through the material rather than the proportions:

- Project is demoted to a file drawer: it stops matching the page height
  (align-self defeats the grid stretch) and drops the card elevation for a
  recessed surface with tighter padding.
- The section nav leads with the writing destinations and lets the Review
  destinations recede into a captioned, faint cluster; they stay live links
  that still switch modes and land on the real pane.
- Source becomes a page: no card elevation, a focus-within edge instead, and a
  readable-measure cap (~80 monospace columns) centered in its grid column, so
  the pane edge is the page edge and heading, editor, gutter, and status line
  share one column. width:100% is load-bearing there - a grid item with any
  justify-self other than stretch sizes to its content, so without it the cap
  never bounds the page.
- The top band quiets: the decorative eyebrow is gone, the product mark and the
  live connection status share one baseline row, the theme control states only
  its state, and Author takes a tighter header/nav rhythm than Review.
- The long connection sentence collapses into a chip whose honest detail is one
  activation away. The chip is a real button, so the detail is reachable by
  keyboard and assistive tech rather than hover only, and connection.status now
  has a single writer so the short label cannot drift from the sentence it
  summarizes.

Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@itoqa

itoqa Bot commented Sep 13, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: 9e19150: 13 test cases ran, 13 passed ✅.

Summary

Coverage spans core editor layouts and navigation, keyboard accessibility, responsive behavior, preference handling, and connection/recovery flows. It also exercises edge cases around host outages, repeated failures, unsaved edits, reload state, and rapid navigation, with the observed behavior remaining healthy and intentional.

Safe to merge — no PR-attributable regressions, new failures, or previously flagged failures remain, and the exercised product behavior is consistent across normal flows and failure states. The few setup limitations and independently reviewed behavior notes are not merge blockers.

Tests run by Ito

View full run

Result Severity Type Description
General The editor kept the connection message and unsaved-work message consistent when the host was unavailable. The dirty content stayed protected and the interface did not claim that recovery had succeeded.
General The editor reconnects after the health check recovers, while the dirty edit remains visible and the failed recovery snapshot stays clearly reported.
General The editor kept the unsaved edit visible while the host was unavailable. The connection message stayed consistent and the recovery failure message was not duplicated or overwritten.
General Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The finding treats the persisted section fragment and a mounted Problems pane as requiring Problems to remain aria-current after reload, but that is not this component's contract. The repository explicitly specifies aria-current as viewport-derived, explicitly tests that reload does not reapply the section jump, and persists Review mode separately, so Project at the reset page top while Problems i…
Connection The local editor service was not available on the assigned address, so the connection disclosure could not be exercised. This result is classified as a setup pass because the browser reached documentation first and the later connection refusal came from the stopped local app container.
Editor Author mode shows a recessed Project drawer and a centered Source writing page. Switching to Review shows the wider Project and Source workspace with the Review rail.
Navigation The Problems link stayed active in Author mode. Activating it switched to Review, opened the Problems pane, updated the URL, and focused the pane.
Navigation The supplied editor state kept Problems available, so the link opened Review and focused the Problems pane as designed. The unavailable-target branch was not exercised because no unavailable destination state was provided.
Recovery When the editor host became unavailable, the header showed Host unavailable and the editor told the user to restart the host. A second health failure kept the same connection detail and recovery message without adding duplicates or replacing the message.
Rev An unsupported saved density value opened the editor in Author mode with only the Author panes visible. Selecting Review showed all five Review panes, and reloading kept the valid Review choice.
Rev At 375px wide, the editor stays inside the screen and all header controls remain usable. Keyboard navigation switches to Review, and activating Problems lands on the visible pane with focus and the correct active marker.
Rev The Connected button expands and collapses with the Space key, keeps focus in place, and lets Tab move to the Author control next.
Section Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The report correctly observes that reload does not restore the fragment's section, but incorrectly calls that a defect and invents a contrary test contract. The repository's explicit test contract says the section jump is not reapplied after reload and that scrollspy controls positioning, exactly matching the cited implementation, so Project becoming current from geometry is intended behavior. The…

Tip

Reply with @itoqa to send us feedback on this test run.

drawmeanelephant and others added 2 commits September 13, 2026 14:25
Review corrections to the Author writing surface, plus the structural fix
behind them: the scales wrote their order down twice, so the tests and the
stylesheet could disagree without anything failing.

The app title takes its own step in the type chain instead of the pane-title
size it had collapsed onto, the theme control drops an aria-pressed that
contradicted its own state label, the page shows one focus ring rather than two,
the status chip's live region is text-only like every other one, and the
hierarchy suite measures both density modes instead of pinning Review.

Type, spacing, radius, and layer each declare their order once in tokens.css. A
static lint holds the classification, the names, and the README to those lists;
a cascade walk resolves the same lists out of the applied stylesheet, which is
the only place a clamp step can be checked at all. The six bare z-index values
are now named layers, so the ladder is the only authority on stacking order.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
The declared-scales block is the only copy of each order, but the type
scale also carried --type-scale-chain/--type-scale-off-chain with the
same lists and no consumer. Drop them, and correct the README's count of
the widths scales.spec.ts walks (four, matching the test and fragment).
@drawmeanelephant
drawmeanelephant merged commit d7dfcc6 into main Sep 13, 2026
13 checks passed
@drawmeanelephant
drawmeanelephant deleted the t3code/editor-residual-flat-993 branch September 13, 2026 19:03
@itoqa

itoqa Bot commented Sep 13, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Report9e19150bb0b3b9: 18 test cases ran, 18 passing ✅.

Diff Summary

Coverage spans core editor workflows across Author and Review modes, including navigation, writing, focus, responsive layouts, themes, connection states, and accessibility behavior. It also exercises business rules and defensive edge cases around scale ordering, malformed declarations, validation gates, deferred checks, and mode transitions, with adversarial review of disputed behavior.

Safe to merge — the run found no PR-attributable regressions, new failures, or previously flagged failures that remain unresolved; all exercised behaviors passed. The untested prior areas are coverage gaps rather than merge blockers, so they are low-risk follow-up observations.

Tests run by Ito

View full run

Result State Severity Type Description
Passing General Switching from Author to Review and back kept the Source editor the same size and position. The editor returned to Author mode with one clear focus state.
Passing General Adding the same spacing token twice made the scale check report three clear errors, return exit code 1, and stop before the UI build and browser tests. The token file was restored and the checkout was clean afterward.
Passing General The editor's normal checks passed. A temporary viewport fixture then made the static check defer a type-size comparison, and the browser check correctly caught the bad values at 1024px and 480px before the fixture was removed.
Passing Gate The local editor gate passed every stage. Static checks, including scale conformance, ran before the UI build and end-to-end tests, and all 217 end-to-end tests plus the live integrations passed.
Passing Hierarchy The app title stayed larger than pane titles, pane titles stayed larger than sub-pane titles, and body and label text stayed smaller in both Author and Review modes.
Passing Navigation Selecting Problems kept the session token and selected section in the URL. The old file-opening value was removed, and reloading the page kept Problems selected without reopening the file.
Passing Rev Keyboard focus reached the Source editor, and the editor showed one clear focus ring around the writing area instead of two overlapping rings.
Passing Rev The host-unavailable message stayed in a text-only live region, while a separate keyboard button opened the full status details.
Passing Rev The focused Skip to workspace link appeared above the sticky navigation, and Focus writing mode opened above the workspace with a working Exit focus control.
Passing Scales The editor's type, spacing, radius, and layer values stayed in the required order at 1920px, 1440px, 1024px, and 480px. The layer and off-chain type checks also passed.
Passing Scales The scale checker rejected missing, stale, duplicate, and unclassified names with clear errors instead of passing silently.
Passing Section Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The finding applies an obsolete file-level availability contract to a pane that the repository deliberately changed into a project-level view. Current implementation, documentation, and multiple regression tests all require precisely the reported behavior: Graph remains available without an active file, an Author-mode activation reveals Review, focuses Graph, announces the switch, and updates the …
Passing Status The editor service was unavailable, so the connection summary and disclosure control could not be opened. The reachable local site was documentation and did not include editor controls.
Passing Status After the connection control is closed, the editor is designed to show only the short connection summary and hide the longer detail. Interactive confirmation was unavailable because the editor service was not running in the test environment.
Passing Theme The editor could not be opened in this run because its required local service was unavailable. Source inspection confirms the theme control changes the document theme and shows the current Light or Dark label without using a pressed-state attribute.
Passing Writing Author mode showed a compact header, clear Project and Source navigation, and a centered Source editor with focus active.
Passing Writing Clicking the visible Problems link switched the editor to Review mode and opened the Problems pane. The link stayed usable even though its pane is hidden in Author mode.
Passing Writing At 480px wide, the Author view kept Project and Source navigation usable. The Source editor stayed centered at a readable 328px width and the Source section remained active.
⏸️ Skipped General The editor kept the connection message and unsaved-work message consistent when the host was unavailable. The dirty content stayed protected and the interface did not claim that recovery had succeeded.
⏸️ Skipped General The editor reconnects after the health check recovers, while the dirty edit remains visible and the failed recovery snapshot stays clearly reported.
⏸️ Skipped General The editor kept the unsaved edit visible while the host was unavailable. The connection message stayed consistent and the recovery failure message was not duplicated or overwritten.
⏸️ Skipped General Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The finding treats the persisted section fragment and a mounted Problems pane as requiring Problems to remain aria-current after reload, but that is not this component's contract. The repository explicitly specifies aria-current as viewport-derived, explicitly tests that reload does not reapply the section jump, and persists Review mode separately, so Project at the reset page top while Problems i…
⏸️ Skipped Connection The local editor service was not available on the assigned address, so the connection disclosure could not be exercised. This result is classified as a setup pass because the browser reached documentation first and the later connection refusal came from the stopped local app container.
⏸️ Skipped Editor Author mode shows a recessed Project drawer and a centered Source writing page. Switching to Review shows the wider Project and Source workspace with the Review rail.
⏸️ Skipped Navigation The Problems link stayed active in Author mode. Activating it switched to Review, opened the Problems pane, updated the URL, and focused the pane.
⏸️ Skipped Navigation The supplied editor state kept Problems available, so the link opened Review and focused the Problems pane as designed. The unavailable-target branch was not exercised because no unavailable destination state was provided.
⏸️ Skipped Recovery When the editor host became unavailable, the header showed Host unavailable and the editor told the user to restart the host. A second health failure kept the same connection detail and recovery message without adding duplicates or replacing the message.
⏸️ Skipped Rev An unsupported saved density value opened the editor in Author mode with only the Author panes visible. Selecting Review showed all five Review panes, and reloading kept the valid Review choice.
⏸️ Skipped Rev At 375px wide, the editor stays inside the screen and all header controls remain usable. Keyboard navigation switches to Review, and activating Problems lands on the visible pane with focus and the correct active marker.
⏸️ Skipped Rev The Connected button expands and collapses with the Space key, keeps focus in place, and lets Tab move to the Author control next.
⏸️ Skipped Section Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The report correctly observes that reload does not restore the fragment's section, but incorrectly calls that a defect and invents a contrary test contract. The repository's explicit test contract says the section jump is not reapplied after reload and that scrollspy controls positioning, exactly matching the cited implementation, so Project becoming current from geometry is intended behavior. The…

Tip

Reply with @itoqa to send us feedback on this test run.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant