Skip to content

fix(web): keep the wrapped file editor anchored on redo and digit boundaries - #10508

Open
Mnigos wants to merge 2 commits into
pingdotgg:mainfrom
Mnigos:file-preview-caret
Open

Mnigos wants to merge 2 commits into
pingdotgg:mainfrom
Mnigos:file-preview-caret

Conversation

@Mnigos

@Mnigos Mnigos commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Editing a large wrapped file in the Files surface still jumps in two cases that stayed open on #7907 after #10018: pressing Enter when the line count crosses a digit boundary (999 to 1000), and the first Redo after a run of Undos. Both leave the caret off screen.

Both fixes live in the patched @pierre/diffs editor.

  • Digit boundary. The editor asked the virtualizer to drop every measured row height so the wider gutter could be remeasured. The file shrank to single-row estimates under the viewport, and the browser clamped the scroll position before any row could be remeasured. Measured heights now stay in place as estimates across the edit and across width changes. Rendered rows are remeasured by the existing resize path, and off screen rows are remeasured when they render. The width tracking from fix(web): retain wrapped row heights during edits #10018 is removed for the same reason: an under-estimate moves the viewport, a stale estimate does not.
  • First Redo. Undo removed the trailing row elements from the DOM, but the editor's line element cache kept them. The Redo re-created the row, looked up the detached element, read an offsetTop of 0, and scrolled the caret to the top of the rendered block. Update after merging main (2026-10-07): @pierre/diffs 1.5.2 (chore(deps): upgrade @pierre/diffs to 1.5.2 and @pierre/trees to beta.6 #16644) clears the cached row elements on every line-count change, so this half is fixed upstream. The redo test in this PR passes on main's unmodified 1.5.2 patch and stays as a regression test. This PR now carries only the digit-boundary and width changes, ported to the 1.5.2 patch.

Not changed: resizing the panel with the caret mid file can still jump. The browser's scroll anchoring and the virtualizer's own anchor both move the viewport after a native rewrap, which needs a change to the virtualizer's render order. That part of #7907 stays open.

Verification

  • vp test run apps/web/src/components/files/fileEditorVirtualization.test.ts: 28 tests pass. The width and digit-boundary tests now encode the estimate policy. The redo test failed on the 1.3.0 base with translateY(0px); on the 1.5.2 base it passes before and after. The 12 width and digit-boundary tests fail on main's 1.5.2 patch and pass here.
  • Web typecheck and lint on the touched files.
  • Chromium web client, 1280×800, word wrap on, explorer hidden, 419 px editor, same sample files and steps as the retest on [Bug]: Files surface jumps scroll focus and throws "Line doesn't exist" with word wrapping activated #7907. scrollTop/L<caret line> comes from the native shadow-DOM selection.
Scenario Before (main) After
Enter at line 999 of 999 49188/L999 to 29308/L801, first visible line 994 to 828, caret off screen 49188/L999 to 49200/L1000, first visible line stays 994
First Redo after Enter ×15, end, Undo ×8 52268/L1198 to 40008/L1001, view jumps to line 1001 52268/L1198 to 52288/L1199, caret visible

Before

Recorded on main today for the #7907 retest (comment).

Before: the digit-boundary Enter and the first Redo jump

Before recording, MP4

After

After: Enter at line 999 keeps line 1000 visible, the first Redo keeps line 1199 visible

After recording, MP4

Screenshots after the boundary Enter and after the first Redo

After the Enter at line 999: caret on line 1000, lines 995 to 1000 in view

After the first Redo: caret on line 1199 at the end of the file

Related to #7907. Implemented with Claude Code (Claude Fable 5).

Note

Keep wrapped file editor anchored on redo and digit boundaries

  • Fixes layout invalidation so partial resets only remove measurements at or after the changed line, preserving earlier row measurements instead of clearing everything
  • Removes cached code-width state and width-change cache invalidation from VirtualizedFile; Editor now removes deleted gutter rows from the line-element cache and always passes the edit start line to document-change invalidation instead of selecting the file start based on line-number digit width
  • Expands the editor test DOM shim to model sibling insertion, detached rows, parent relationships, offset-top calculation, and CSSStyleDeclaration reads/writes
  • Adds test coverage for undo/redo row recreation and caret positioning, line-number digit boundary preservation through undo/redo, and wrapped measurement width reconciliation on resize
  • Risk: generated dist.components.VirtualizedFile and dist.editor.Editor artifacts remove cached code-width state and change layout invalidation to use edit start line — callers relying on full-width cache invalidation on resize or digit-width-based file-start selection will see different measurement retention behavior

Macroscope summarized 5660af3.

Summary by CodeRabbit

  • Bug Fixes
    • Improved virtualized file rendering when documents are edited, resized, hidden, or contain varying line counts.
    • Preserved accurate offscreen content measurements during width changes and updates.
    • Improved editor caret placement after undo and redo operations.
    • Corrected selection and gutter behavior when controlled-selection and line-selection options are enabled.
    • Improved language detection consistency during file rendering.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 7, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7650243

Macroscope's review found this PR approvable — This is a focused production bug fix that preserves wrapped-editor measurements across edits and resizes to prevent viewport jumps, with targeted regression coverage and no schema, default, deployment, security, or static-analysis changes. The runtime impact is localized to the existing file-editor virtualization path.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: bdbcc0d1-0c07-4d36-a69c-ff8ce3b9992b

📥 Commits

Reviewing files that changed from the base of the PR and between 8b2838e and 5660af3959485f6c194600c0baf986bed0c90f96.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • apps/web/src/components/files/fileEditorVirtualization.test.ts
  • patches/@pierre%2Fdiffs@1.3.0-beta.10.patch

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Virtualized file layout updates now support partial invalidation, resize reconciliation, and zero-width safeguards. Editor updates synchronize changed documents and stale rows. Renderer updates clean caches, compute languages consistently, and expose additional package entry points.

Changes

Virtualized editor behavior

Layer / File(s) Summary
Partial layout invalidation and resize handling
apps/web/src/components/files/fileEditorVirtualization.test.ts, patches/@pierre%2Fdiffs@1.3.0-beta.10.patch
Virtualized layout updates preserve unaffected measurements, skip zero-width measurement, and expose resize notifications. Tests cover width changes, resize delivery, and hidden-code measurement.
Editor synchronization and selection behavior
apps/web/src/components/files/fileEditorVirtualization.test.ts, patches/@pierre%2Fdiffs@1.3.0-beta.10.patch
Editor updates remove stale rows, synchronize document changes, pass the change start line to layout invalidation, and respect controlled-selection options.
Line-count and undo/redo regression coverage
apps/web/src/components/files/fileEditorVirtualization.test.ts
Test fixtures support rendered rows, offsets, selection, and canvas measurement. Tests cover digit-boundary changes and caret placement after undo and redo.
Renderer cache cleanup and package exports
patches/@pierre%2Fdiffs@1.3.0-beta.10.patch
FileRenderer cleans worker and file-cache state during render-cache updates, computes language before highlighting, and adds public type and utility exports.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 5660a

The editor now retains virtualization measurements across relevant edits and width changes and removes stale line elements so redo can locate the recreated caret. The targeted regressions are covered, with no remaining merge-readiness risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the wrapped file editor fixes for redo and line-number digit boundaries.
Description check ✅ Passed The description explains the problem, changes, and focused verification, and includes before-and-after evidence. It links issue #7907 but does not state explicit maintainer approval or explain why the…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@Mnigos
Mnigos force-pushed the file-preview-caret branch from 5660af3 to 19d507e Compare September 7, 2026 17:19
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@Mnigos
Mnigos force-pushed the file-preview-caret branch from 379ebd9 to cb77aa8 Compare October 1, 2026 17:54
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 1, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 7, 2026 01:26

Dismissing prior approval to re-evaluate 7650243

This branch has not been deployed

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants