Skip to content

fix(web): keep line numbers clear when code wraps - #1260

Merged
tiann merged 3 commits into
tiann:mainfrom
junmo-kim:fix/terminal-wrap-fidelity
Aug 1, 2026
Merged

fix(web): keep line numbers clear when code wraps#1260
tiann merged 3 commits into
tiann:mainfrom
junmo-kim:fix/terminal-wrap-fidelity

Conversation

@junmo-kim

@junmo-kim junmo-kim commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Problem

The global code-wrap support I added in #985 introduced this regression. I sized the gray line-number gutter from the number of digits alone and missed the cell's horizontal padding, so narrow wrapped output can leave line numbers crowded against or overlapping the code column.

Diff views already consume the global code-wrap preference but do not expose the wrap control themselves, so changing the setting requires finding a separate code block.

Solution

  • Include the gutter's horizontal padding in the line-number grid track, keeping its gray background width constant and leaving a clear gap between one-, two-, and three-digit line numbers and the code column.
  • Keep the existing native wrapping and indentation behavior unchanged.
  • Expose the existing global wrap toggle in diff preview, inline, and dialog headers.
  • Keep diff controls out of nested interactive regions, restore focus to the actual dialog invoker, and exclude the controls from share exports.

Before / After

Before After
Line-number gutter before the fix Line-number gutter after the fix

Before, the gutter track reserves only the digit width. After, it reserves the digit width plus both padded sides, so the number and code columns remain visually separate at every line-count boundary.

Tests

  • bun run typecheck
  • bun run test
  • bun run build:web
  • CodeBlock unit coverage for one-, two-, and three-digit gutters
  • Focused Web tests for DiffView, CodexDiff ToolCard interaction, and dialog focus restoration
  • Mobile Chromium geometry regression asserting a constant gutter width and numberRight < codeTextLeft
  • Isolated Hub/Vite/fake-CLI full-stack E2E with tagged terminal output and CodexDiff messages

@junmo-kim
junmo-kim force-pushed the fix/terminal-wrap-fidelity branch from 6a16798 to d7a05c1 Compare July 30, 2026 23:16
@junmo-kim junmo-kim changed the title fix(web): preserve wrapped output fidelity fix(web): keep line numbers clear when code wraps Jul 30, 2026
@junmo-kim
junmo-kim marked this pull request as ready for review July 30, 2026 23:56

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Browser geometry regression is not gated by CI — the added Playwright test verifies the actual gutter separation and mobile wrapping, but the PR workflow only runs typecheck and unit tests; it never invokes test:e2e. Evidence: e2e/terminal-wrap-fidelity.spec.ts:8, .github/workflows/test.yml:16, .github/workflows/test.yml:26
    Suggested fix:
    - run: bunx playwright install --with-deps chromium
    - run: bun run test:e2e -- terminal-wrap-fidelity.spec.ts

Questions

  • None.

Summary

  • Review mode: initial
  • One Minor testing gap found. No Blocker/Major correctness, security, data-loss, or performance issue found in the changed lines. Residual risk: the mobile browser geometry asserted by the new spec is not enforced by current PR CI.

Testing

  • Not run (automation). The live GitHub test check passed, but that workflow does not run Playwright.

HAPI Bot

Comment thread e2e/terminal-wrap-fidelity.spec.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • None.

Questions

  • None.

Summary

  • Review mode: follow-up after new commits
  • No Blocker, Major, Minor, or Nit findings in the latest full diff. The prior browser-CI coverage gap is resolved by .github/workflows/test.yml:17-18. Residual risk: the browser geometry assertion is environment-sensitive and depends on the new Playwright CI step remaining stable.

Testing

  • Not run (automation). Reviewed the current workflow and test coverage statically; live GitHub checks were consulted before submission.

HAPI Bot

@tiann
tiann merged commit 084d346 into tiann:main Aug 1, 2026
2 checks passed
@junmo-kim
junmo-kim deleted the fix/terminal-wrap-fidelity branch August 1, 2026 09:21
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.

2 participants