fix(web): context strip labels no longer loop between compact and expanded - #13059
riccardopll wants to merge 1 commit into
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a narrowly scoped web UI bug fix that stabilizes existing context-strip measurement without adding capabilities, changing defaults, or affecting schemas and infrastructure. The new width calculation is isolated and covered by focused tests, including a regression sweep for the compact/expanded loop. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 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: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe PR adds hidden-width calculation for context strip labels, integrates leaf-span measurement into overflow handling, and adds tests for stable compact-state decisions across strip widths. ChangesContext strip measurement
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: High Suggested reviewers: Merge Risk: ⚪ Minimal · up to The composer context strip now measures split labels consistently, preventing the compact/expanded feedback loop that caused the renderer crash. Focused tests cover the calculation and settling behavior, with no current actionable merge risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…anded The strip measured hidden label text as the largest scrollWidth in the label subtree. A middle-truncated branch label is a clipped head beside an unclipped tail, so the compact pass reserved the head only while the expanded pass laid out head and tail. The two passes disagreed by about one tail, more than the 16px hysteresis, and each layout effect flipped the other until React threw error pingdotgg#185. Scrolling up hit it reliably: the collapsed composer moves its controls into the strip and parks the labels on that boundary. Sum the leaf spans instead, capped at the motion element's max-width, so both states reserve the room the label takes once expanded. Fixes pingdotgg#12891
c032f3a to
04d4cac
Compare
Fixes #12891
Problem
Scrolling up in a thread crashed the desktop app to the "Something went wrong" screen with React error # 185 (maximum update depth exceeded) on
0.0.43-nightly.20260922.2096. The minified frames decode tosetOverflows(nextOverflows)inmeasure()and the dependency-freeuseLayoutEffect(() => { measure(); })inuseLabelsOverflow(apps/web/src/components/BranchToolbar.tsx), the loop triaged in #12891.measure()reserved hidden label text as the largestscrollWidthin the label subtree. Since #12805 the branch label is aMiddleTruncate, a clipped head beside an unclipped tail, so the compact pass reserved the head only while the expanded pass laid out head and tail. The two passes disagreed by about one tail, more than the 16px hysteresis, and each layout effect flipped the other until React gave up.Scrolling up hits it reliably: the composer collapses into the resting layout, its model and mode controls move into the context strip, and the strip lands on that boundary.
Fix
Sum the leaf spans of each label instead, capped at the motion element's max-width (the one width the compact override leaves alone), so compact and expanded reserve the same room. The math lives in
resolveContextStripLabelHiddenWidthwith tests, including a sweep over strip widths asserting the compact decision settles in one pass.No visual change. Verified with the touched test file, targeted lint, and the web typecheck.
Made with Claude Fable 5.1 in Claude Code.
Summary by CodeRabbit