Repository navigation
Use appearance-specific onboarding screenshots - #8803
Merged
Merged
Conversation
📝 WalkthroughWalkthroughOnboarding screenshots now account for locale and system appearance, including resource names, cache sizing, and loading tests. A workspace selection test now uses the view’s current state and event-handling APIs. ChangesTheme-aware onboarding screenshots
Workspace selection test alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SwiftUI
participant OnboardingScreenshot
participant ImageResources
SwiftUI->>OnboardingScreenshot: resolve current ColorScheme
OnboardingScreenshot->>OnboardingScreenshot: build language and appearance resource name
OnboardingScreenshot->>ImageResources: load localized light or dark image
ImageResources-->>OnboardingScreenshot: return screenshot image
Possibly related PRs
🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches📝 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 |
Contributor
Greptile SummaryThis PR adds dark-mode onboarding screenshot assets and updates
Confidence Score: 5/5Safe to merge — the change is self-contained, well-tested, and touches only onboarding asset selection logic. The appearance dimension is correctly threaded through the task ID, cache key, and cache size limit. The @Concurrent loadImage path keeps disk I/O off the main actor, and the @mainactor screenshotCache access is properly isolated. Dark-mode PNG assets are deliberate product resources with verified dimensions. No production rule violations found. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["OnboardingScreenshot.body"] -->|"task(id: resourceName)"| B["resourceName computed"]
B --> C["content + language + appearance"]
C --> D{"appearance"}
D -->|"light"| E["Onboarding-{content}-{lang}"]
D -->|"dark"| F["Onboarding-{content}-{lang}-dark"]
E --> G["image(content:language:appearance:)"]
F --> G
G --> H{"NSCache hit?"}
H -->|"yes"| I["return cached UIImage"]
H -->|"no"| J["loadImage @concurrent"]
J --> K["Bundle.module URL lookup"]
K --> L["Data(contentsOf:) + byPreparingForDisplay()"]
L --> M["store in screenshotCache"]
M --> I
I --> N["screenshot @State renders Image"]
Reviews (3): Last reviewed commit: "Require explicit onboarding screenshot a..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingScreenshot.swift`:
- Line 79: Make the appearance parameter of image(...) in OnboardingScreenshot
required by removing its .light default, then update every direct image(...)
call site to pass the intended OnboardingScreenshotAppearance explicitly.
🪄 Autofix (Beta)
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: 446af1ee-a2f4-463e-9836-894351292b42
⛔ Files ignored due to path filters (4)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/OnboardingScreenshots/Onboarding-notifications-en-dark.pngis excluded by!**/*.pngPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/OnboardingScreenshots/Onboarding-notifications-ja-dark.pngis excluded by!**/*.pngPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/OnboardingScreenshots/Onboarding-workspaces-en-dark.pngis excluded by!**/*.pngPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/OnboardingScreenshots/Onboarding-workspaces-ja-dark.pngis excluded by!**/*.png
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingScreenshot.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/OnboardingSceneChromeTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMacSelectionTests.swift
| content: Content, | ||
| language: OnboardingScreenshotLanguage | ||
| language: OnboardingScreenshotLanguage, | ||
| appearance: OnboardingScreenshotAppearance = .light |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n 'OnboardingScreenshot\.image\s*\(' Packages/iOS/CmuxMobileShellUIRepository: manaflow-ai/cmux
Length of output: 316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'OnboardingScreenshot.swift|OnboardingSceneChromeTests.swift' . | sed 's#^\./##'
echo "== file outline =="
ast-grep outline Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingScreenshot.swift --view expanded || true
echo "== relevant source =="
sed -n '1,140p' Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingScreenshot.swift
echo "== tests around call =="
sed -n '60,125p' Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/OnboardingSceneChromeTests.swift
echo "== search all appearance param usages without full repo explosion =="
rg -n 'OnboardingScreenshot\.image|OnboardingScreenshotAppearance|image\s*\(' Packages/iOS/CmuxMobileShellUI -g '*.swift'Repository: manaflow-ai/cmux
Length of output: 10502
Make the screenshot appearance parameter required.
image(...) selects an appearance-specific resource, but the public default of .light allows future omitted calls to render light screenshots when a dark screenshot should be used. Remove the default and update any direct call sites.
Proposed fix
- appearance: OnboardingScreenshotAppearance = .light
+ appearance: OnboardingScreenshotAppearance📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| appearance: OnboardingScreenshotAppearance = .light | |
| appearance: OnboardingScreenshotAppearance |
🤖 Prompt for 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.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingScreenshot.swift`
at line 79, Make the appearance parameter of image(...) in OnboardingScreenshot
required by removing its .light default, then update every direct image(...)
call site to pass the intended OnboardingScreenshotAppearance explicitly.
Source: Path instructions
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Collaborator
Author
|
Updated the screenshots so each light/dark pair is captured from the same preview entrypoint with the same fixture, language, status bar, and crop. The only switched input is the simulator appearance. Verification:
Dogfood tag: http://127.0.0.1:17320/ondark |
5 of 6 tasks
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
1 participant
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
colorScheme, keeping existing light asset names and using-darkassets in dark mode.Verification
git diff --check origin/main...HEADjq empty Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstringssips -g pixelWidth -g pixelHeight Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/OnboardingScreenshots/Onboarding-*-dark.pngfile Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/OnboardingScreenshots/Onboarding-*.pngxcodebuild test -workspace ios/cmux.xcworkspace -scheme cmux-ios -destination 'platform=iOS Simulator,name=cmux-ondark-root-0724' -derivedDataPath /Users/abdulazizalbahar/Library/Developer/Xcode/DerivedData/cmux-ios-ondark PRODUCT_BUNDLE_IDENTIFIER=dev.cmux.ios.ondark 'PRODUCT_DISPLAY_NAME=cmux DEV ondark' CMUX_GIT_SHA=11ec9fab9b CMUX_DEV_TAG=ondark CMUX_PRESENCE_BASE_URL= CMUX_IOS_AUTH_ENV= 'CMUX_API_BASE_URL=http://localhost:9170' 'CMUX_IROH_BROKER_BASE_URL=https://cmux-staging.vercel.app' EXCLUDED_SOURCE_FILE_NAMES=Info.plist CODE_SIGNING_ALLOWED=NO SWIFT_OPTIMIZATION_LEVEL=-O SWIFT_COMPILATION_MODE=wholemodule GCC_OPTIMIZATION_LEVEL=s -only-testing:CmuxMobileShellUITests/OnboardingSceneChromeTestsxcodebuild test -workspace ios/cmux.xcworkspace -scheme cmux-ios -destination 'platform=iOS Simulator,name=cmux-ondark-root-0724' -derivedDataPath /Users/abdulazizalbahar/Library/Developer/Xcode/DerivedData/cmux-ios-ondark PRODUCT_BUNDLE_IDENTIFIER=dev.cmux.ios.ondark 'PRODUCT_DISPLAY_NAME=cmux DEV ondark' CMUX_GIT_SHA=11ec9fab9b CMUX_DEV_TAG=ondark CMUX_PRESENCE_BASE_URL= CMUX_IOS_AUTH_ENV= 'CMUX_API_BASE_URL=http://localhost:9170' 'CMUX_IROH_BROKER_BASE_URL=https://cmux-staging.vercel.app' EXCLUDED_SOURCE_FILE_NAMES=Info.plist CODE_SIGNING_ALLOWED=NO SWIFT_OPTIMIZATION_LEVEL=-O SWIFT_COMPILATION_MODE=wholemodule GCC_OPTIMIZATION_LEVEL=s -only-testing:cmuxUITests/cmuxUITests/testOnboardingScenesNotificationFeedResumeAndScannerFallback./scripts/reload-cloud.sh --tag ondark, GitHub Actions run https://github.com/manaflow-ai/cmux/actions/runs/30063089274./ios/scripts/reload.sh --tag ondark --simulator cmux-ondark-root-0724 --no-setup --no-launch/,/handler/sign-in, and/handler/after-sign-inwith redirect following.Dogfood
Tag: http://127.0.0.1:17320/ondark
Simulator:
cmux-ondark-root-0724, bundledev.cmux.ios.ondark, launched at first-run onboarding in dark mode.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Show onboarding screenshots that match the system light/dark appearance. Adds dark-mode assets, regenerates light images, and requires an explicit appearance when loading to ensure the right image and cache entry.
New Features
Onboarding-*-dark.pngassets (EN/JA) and regenerated light variants.appearance(auto-resolved from SwiftUIcolorScheme) and caches by content/language/appearance.Bug Fixes
Written for commit 9af9da5. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests