Repository navigation
Fix iOS terminal toolbar contrast during theme changes - #9937
Closed
azooz2003-bit wants to merge 2 commits into
Closed
azooz2003-bit wants to merge 2 commits into
azooz2003-bit wants to merge 2 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds shared terminal chrome styling, propagates theme-aware foreground colors through workspace controls, changes parity preview progression to manual fixture advancement, simplifies terminal input accessory backgrounds, and expands light/dark UI parity tests. ChangesTerminal Theme Parity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalThemeParityUITests
participant WorkspaceDetailDelayedTerminalPreviewView
participant WorkspaceDetailView
participant MobileTerminalChromeStyle
participant ToolbarControls
TerminalThemeParityUITests->>WorkspaceDetailDelayedTerminalPreviewView: select appearance and theme fixture
WorkspaceDetailDelayedTerminalPreviewView->>WorkspaceDetailView: apply fixture after delayed frame delivery
WorkspaceDetailView->>MobileTerminalChromeStyle: derive active chrome colors
MobileTerminalChromeStyle->>ToolbarControls: apply foreground, tint, background, and color scheme
TerminalThemeParityUITests->>ToolbarControls: capture controls and screenshot pixels
TerminalThemeParityUITests->>TerminalThemeParityUITests: validate contrast, geometry, and modifier states
Possibly related PRs
Suggested labels: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmuxUITests/TerminalThemeParityUITests.swift`:
- Around line 383-394: Replace the fixed Thread.sleep in the sticky gesture test
with a deadline-bounded predicate poll that waits until the rendered difference
from armedPixels confirms the one-shot state has cleared. Preserve the existing
tap sequence and add a comment explaining that tap() clears the one-shot armed
state while doubleTap() arms the sticky lock.
- Around line 245-264: Update capture(_:name:expectedBackground:baselineFrames:)
to accept a non-optional [String: CGRect], and replace optional-unwrapping logic
with an empty-dictionary check to represent “no baseline yet.” Update its
callers, including the flow around line 106, to pass the dictionary directly
rather than converting an empty dictionary to nil.
- Around line 275-300: In the controls loop, apply the qualifyingPixelFraction
assertion against control.minimumContrast only when control.isEnabled is true.
Keep the disabled-control maximumContrast >= 3 recognizability assertion active
for disabled controls, along with the existing baseline-frame validation.
- Around line 168-202: Bound the transition burst collection in the sampling
logic around burstDeadline to a fixed maximum sample count, and retain
screenshots without eagerly decoding ScreenshotPixels. Decode each screenshot
lazily as it is analyzed in the enabled-control loop, while preserving the
existing contrast checks and failure reporting; remove the redundant
samples.isEmpty assertion because the earlier XCTUnwrap already establishes
non-empty samples.
In
`@Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatSessionHeaderView.swift`:
- Around line 224-238: Update the working and idle branches in the
status-indicator view so they remain visually distinguishable when
foregroundColor is provided and pulse animation is disabled. Preserve their
state-specific shapes or styling against the terminal background, while keeping
the existing needsInput and ended treatments unchanged.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailDelayedTerminalPreviewView.swift`:
- Around line 128-138: The theme parity advance task in
advanceThemeParitySequence must be stored or driven by .task(id:) so it can be
cancelled, and must use defer to always reset themeAdvancePending on
cancellation or early return. In
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailDelayedTerminalPreviewView.swift
lines 140-154, bound deliverThemeParityPreviewFrame’s retry loop with a deadline
or attempt limit and surface exhaustion through themeStage.
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift`:
- Around line 1053-1058: Remove the unused stickyLockBorderColor assignment from
the AccessoryActionButton handling near isStickyLocked, since isStickyLocked is
always set to false and the overlay border cannot draw. Only remove the
AccessoryActionButton properties as well if a repository-wide search confirms
they have no other call sites.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3cf588d3-5cf8-4c29-993b-e7ca1fb0cf6a
📒 Files selected for processing (13)
Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatSessionHeaderView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/AltScreenNoticeButton.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalTheme+SwiftUI.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/View+MobileNavigationChrome.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceBackButton.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChangesChipLabel.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChangesToolbarButton.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailDelayedTerminalPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceToolbarTitleView.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/MobileCompactToolbarTitleStack.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swiftios/cmuxUITests/TerminalThemeParityUITests.swift
azooz2003-bit
force-pushed
the
task-ios-light-toolbar-contrast
branch
from
August 11, 2026 00:45
ac5e60b to
f6a9600
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmuxUITests/TerminalThemeParityUITests.swift`:
- Around line 376-385: Update the armed and sticky screenshot capture sequences
in the relevant UI test method to await the existing waitForScreenshot helper
after control.tap() and control.doubleTap() before capturing screenshots and
evaluating difference assertions. Add a brief comment to the tap sequence noting
that the second tap clears the one-shot armed state and doubleTap() arms the
sticky lock.
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift`:
- Around line 1053-1057: Remove the unused sticky overlay border implementation
from AccessoryActionButton, including isStickyLocked, stickyLockBorderColor, and
updateStickyLockBorder(). Also delete the unconditional isStickyLocked
assignment in TerminalInputTextView.swift while preserving the direct background
configuration behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5f1cae39-7f14-422b-9b89-0f8a2c6f6977
📒 Files selected for processing (4)
Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatSessionHeaderView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailDelayedTerminalPreviewView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swiftios/cmuxUITests/TerminalThemeParityUITests.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
azooz2003-bit
force-pushed
the
task-ios-light-toolbar-contrast
branch
from
August 11, 2026 00:58
f6a9600 to
0714596
Compare
azooz2003-bit
force-pushed
the
task-ios-light-toolbar-contrast
branch
2 times, most recently
from
August 11, 2026 01:08
9585e9b to
2626afd
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2626afd. Configure here.
azooz2003-bit
force-pushed
the
task-ios-light-toolbar-contrast
branch
from
August 11, 2026 01:20
2626afd to
d01deb4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmuxUITests/TerminalThemeParityUITests.swift`:
- Around line 417-436: Update the sticky-state wait predicate and final
assertion in the double-tap flow around control.doubleTap() to compare
stickyPixels against the previously captured resting screenshot/pixels, in
addition to distinguishing it from armedPixels. Require the sticky presentation
to differ from restingPixels by the established threshold so the wait completes
only after the sticky state has rendered, while preserving the existing
armed-state comparison.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c8f07658-683e-47e4-93eb-a3c0a965cc0e
📒 Files selected for processing (1)
ios/cmuxUITests/TerminalThemeParityUITests.swift
azooz2003-bit
force-pushed
the
task-ios-light-toolbar-contrast
branch
from
August 11, 2026 01:29
d01deb4 to
bd17609
Compare
azooz2003-bit
force-pushed
the
task-ios-light-toolbar-contrast
branch
from
August 11, 2026 02:14
bd17609 to
80472b6
Compare
This was referenced Aug 13, 2026
lawrencecchen
added
the
stale-revisit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
e9c44ad531c2e2f33e5927aa73365e09a9ca355c; all 7 dark-to-light samples fail, with Paste reaching1.00:1.80472b606427a416f143d6587a714b211998eda7; the two-appearance, six-state transition and modifier matrix passes in 186.983 seconds with 0 failures.7.0014:1, and 17/17 point-hit proxies under both system appearances.lbarbuild is installed on Aziz, signed in, paired to the tagged Mac, and has an exact-SHA usable-RPC readiness receipt.Evidence
cmux-assets/task-ios-light-toolbar-contrast/before-ada6abce/before-findings.mdcmux-assets/task-ios-light-toolbar-contrast/after-80472b60/after-findings.mdNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Medium Risk
Touches high-visibility iOS navigation and keyboard chrome across theme transitions and iOS 26 toolbar behavior; risk is mostly visual/regression rather than security or data handling.
Overview
Fixes unreadable iOS terminal chrome when the active theme changes live by painting navigation and keyboard toolbars from a single terminal theme snapshot instead of Liquid Glass and adaptive system colors.
MobileTerminalChromeStylepairs opaque background, readable foreground, and color scheme from oneTerminalTheme.mobileTerminalChromeControl(theme:)applies that snapshot to toolbar items; on iOS 26mobileTerminalSharedBackgroundHidden()hides shared glass while keeping native layout and accessibility. Workspace detail applies this to back, title, alt-screen notice, changes, trailing controls, compact chat headers, and workspace titles; back-button badge contrast now comes from the chrome style rather than static configuration.ChatStateIndicatorViewand compact title stacks accept optional monochrome foregrounds for toolbar mode.TerminalInputTextViewdrops.glass()/.prominentGlass()and the sticky-lock layer border; accessory buttons use flat terminal-owned backgrounds that repaint withrefreshThemeColors().DEBUG adds
waitForThemeParityPreviewOutputSinkand a tap-drivenTerminalThemeAdvancefixture.TerminalThemeParityUITestsruns dark/light system appearances, samples transition bursts for readable controls, and checks modifier armed vs sticky states stay visually distinct.Reviewed by Cursor Bugbot for commit 80472b6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes unreadable iOS terminal toolbar controls during live theme changes by atomically swapping a single terminal-theme snapshot into all toolbar chrome. Applies the snapshot across navigation and input accessory bars to keep contrast and layout stable on iOS 16–26.
Bug Fixes
MobileTerminalChromeStyleandmobileTerminalChromeControl(theme:); all toolbar and accessory items share one opaque background, readable foreground, and matching color scheme.mobileTerminalSharedBackgroundHidden()while keeping native layout, actions, and accessibility.MobileTerminalChromeStyle. Replaced.glass()/.prominentGlass()and removed the sticky‑lock layer border so armed/sticky states repaint with the theme.Tests
TerminalThemeParityUITeststo run in light and dark appearances, wait for the terminal output sink, drive a tap‑controlledTerminalThemeAdvance, and sample transition bursts against prior frames to assert readable control contrast.TerminalPaletteTeststo validate back‑button badge contrast mapping viaMobileTerminalChromeStyle.Written for commit 80472b6. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes