Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe TabBarView's background modifier is refactored to use conditional view-building instead of always instantiating a GeometryReader. The preference key for the selected tab frame is now only set when a tab is actively selected, optimizing the view hierarchy. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR narrows the Key points:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant P as pane.selectedTabId
participant FB as ForEach (tabs)
participant BG as .background view
participant GR as GeometryReader
participant PK as SelectedTabFramePreferenceKey
participant SB as tabBarBackground separator
Note over FB,PK: Old approach — GeometryReader present for ALL tabs
FB->>BG: render N tab backgrounds
BG->>GR: create N GeometryReaders
GR->>PK: publish frame (selected tab) or nil (others)
PK->>SB: reduce → non-nil frame wins
Note over FB,PK: New approach (this PR) — GeometryReader only for selected tab
P->>FB: selectedTabId changes
FB->>BG: render N tab backgrounds
BG->>GR: create 1 GeometryReader (selected tab only)
GR->>PK: publish frame for selected tab
PK->>SB: reduce → selected tab frame (defaultValue=nil when none selected)
Last reviewed commit: 085411e |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .background { | ||
| if pane.selectedTabId == tab.id { | ||
| GeometryReader { geometry in | ||
| Color.clear.preference( | ||
| key: SelectedTabFramePreferenceKey.self, | ||
| value: geometry.frame(in: .named("tabBar")) | ||
| ) | ||
| } | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Potential one-frame separator flicker on tab switch
With the old approach every tab always had a live GeometryReader in its background, so when selection moved from tab A to tab B the new GeometryReader (for B) was already laid out and could publish frame_B in the same render pass that tab A's GeometryReader went silent. With the new approach the GeometryReader for tab B is inserted for the first time on that render pass. SwiftUI needs to lay it out before it can publish a frame, which may introduce a single-frame window where no view publishes a preference value and SelectedTabFramePreferenceKey temporarily resolves to its defaultValue of nil. During that frame, tabBarBackground would render the separator as full-width (no gap) before the correct gap is restored on the next layout pass.
In practice this flicker (≤16 ms at 60 fps) is likely imperceptible during normal tab switching, but it is a subtle behavioral difference worth being aware of. If it ever becomes noticeable, one mitigation is to keep the previous frame cached in tabBarBackground until the preference produces a non-nil value:
.onPreferenceChange(SelectedTabFramePreferenceKey.self) { frame in
if let frame {
selectedTabFrameInBar = frame
}
// Intentionally skip nil to avoid a one-frame gap during tab transitions.
}This keeps selectedTabFrameInBar at the last-known good frame during the brief transition window.
1 participant
Summary
Only publish the selected tab frame preference in .
Why
The cmux workspace typing-lag stress test showed the tab bar spending too much main-thread time recomputing frame preferences for every tab during dense workspace, split, and Bonsplit-tab churn.
Testing
Summary by cubic
Measure and publish the tab frame only for the selected tab in
TabBarViewto cut main-thread work and reduce typing lag during heavy tab churn. Removes unnecessaryGeometryReaderwork and preference updates for non-selected tabs, improving responsiveness in the cmux workspace stress test.Written for commit 085411e. Summary will update on new commits.
Summary by CodeRabbit