Repository navigation
fix(mobile): show complete unwrapped diff lines containing tabs - #14605
jakeleventhal wants to merge 1 commit into
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a localized iOS diff-rendering fix that makes existing horizontal scrolling account for tabs and shaped text without changing wrapping defaults or introducing a new workflow. The production changes are small and accompanied by focused layout coverage. Notes:
No code changes detected at You can add or adjust custom eligibility rules. Learn more. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe iOS diff view now supports unwrapped code layout. It measures code and hunk content widths from rendered text and applies the horizontal scroll offset when drawing native-layout text. The iOS layout tests cover unwrapped tabbed and ASCII text. ChangesiOS diff code layout
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. | 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 identifies the mobile fix for displaying complete unwrapped diff lines that contain tabs. |
| Description check | ✅ Passed | The description explains the problem, implementation, affected surfaces, visual evidence, focused tests, type-check result, limitations, and agent details. It uses “Fix” and “Evidence” instead of the … |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create a new PR
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Autopilot is currently an internal CodeRabbit preview.
Comment @coderabbitai help to get the list of available commands.
Dismissing prior approval to re-evaluate b05f3b9
4d258fe to
2d2343f
Compare
With word wrap off, tab-indented rows were counted as one column per tab and drawn into that narrower rect, so the ends could not be scrolled into view. Measure the rendered width and pan those rows with the horizontal offset.
2d2343f to
c9d3fb7
Compare
Replacement for #12234. That PR was closed under the verification rule because its screenshots predated the renderer rework and its layout tests never scrolled the view. This is the same fix on current
main, with a recording of the current view panning to the ends of the tab-indented lines.Problem
With word wrap off, the iOS review diff counts each tab as one column and draws the row into that narrower rectangle. Tab-indented lines such as a TSConfig
extendsvalue wrap inside the rectangle, and horizontal panning never reaches the real end. Word wrap is off by default in the mobile review diff, and review comment cards always passwordWrap: false, so both surfaces hit this path. #12590 fixed the wrapped path.This is a focused repair of that drawing bug. It does not change a product default, add a setting, or introduce a workflow. The iOS renderer is the only code that has the defect this patch addresses. Android keeps its own renderer.
Fix
Unwrapped rows are laid out on one line.
ReviewDiffCodeLayoutrecords the widest visual line, including tab stops and non-ASCII glyphs. The view sizes horizontal scrolling from that width, without the previouscontentWidthcap, and draws native rows atcodeStartX - horizontalOffset, so the glyphs move with the pan. Wrapped rows still wrap to the viewport and do not pan.Evidence
Word wrap off, viewport 390×250,
contentWidth2800. The rows are the tab-indented lines from the original report:The shots are the production
T3ReviewDiffView, panned through itshandleHorizontalPanhandler. Before ismainatbd89c13020. After is this branch. Host is Mac Catalyst. This machine has Command Line Tools only, soxcrun simctlis unavailable and the T3 device panel cannot boot an iPhone Simulator. The full T3 Code Dev app was not launched.On current
main, panning does not move the lines. Bothextendsvalues stay cut off (tsconandtsconfig/).On this branch the same lines start clipped at the viewport edge, then the pan reaches the closing quote and comma on both filenames.
The recording is that same pan. Both lines stay in frame, from the clipped start through the closing punctuation on
tsconfig.test.json.scroll-both-lines.mp4
apps/mobile/modules/t3-review-diff/tests/run-ios.shpassed: 18 cases, including unwrapped tab, CJK, and Arabic rows that stay on one line and whose ink falls insideusedWidth.T3ReviewDiffView.swifttypechecks for Mac Catalyst against a stubbedExpoModulesCore.Not checked: a booted iPhone Simulator, the full mobile app, and Android.
Model: grok-4.7-build-fast. Harness: Grok in T3 Code.