fix(icon): GitHub dark bg + correct monochrome wiring (#534)#575
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughUpdates launcher background color to ChangesAndroid Launcher Icon Styling & AppBar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile Summary
Confidence Score: 3/5Not safe to merge as-is — the core color change described in the PR is not present in the committed code. The monochrome wiring fix is clean and correct, but the primary stated purpose of the PR — replacing #101010 with #5E35B1 to fix contrast — is unfulfilled. The actual committed value #0D1117 is still a near-black, leaving the original contrast complaint from #534 unresolved. This is a P1 discrepancy between the PR description and the committed artifact. Score is pulled below the P1 ceiling of 4 because the mismatch directly undermines the only visual-correctness goal of the PR. composeApp/src/androidMain/res/values/colors.xml — committed color value does not match PR description. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Android Launcher Request] --> B{Icon Type}
B -->|Standard / Round| C[adaptive-icon v26]
C --> D["background: @color/ic_launcher_background #0D1117"]
C --> E["foreground: @mipmap/ic_launcher_foreground multi-color"]
C --> F{Android 13+ Themed Icons?}
F -->|Yes| G["monochrome: @drawable/ic_launcher_monochrome silhouette vector (fixed)"]
F -->|No| H[Monochrome layer ignored]
G --> I[System tints with wallpaper color - correct silhouette]
J[HomeRoot TopAppBar] --> K["Image: Res.drawable.app_icon Modifier.size(48.dp) contentScale = ContentScale.Crop"]
K --> L["No clip / no forced dark bg (removed in this PR)"]
Reviews (5): Last reviewed commit: "Simplify app icon styling in HomeRoot" | Re-trigger Greptile |
e477103 to
3f5c8d0
Compare
3f5c8d0 to
e06596b
Compare
e06596b to
767f616
Compare
OP (#534, Pixel 8 light mode) reported low contrast between GHS app icon background
#101010and the foreground logo. Two changes:Background
#101010→#5E35B1(Material 3 deep purple, matches the existing "Purple" Tweaks theme preset). White/light foreground now lands on saturated brand color. Splash screen auto-updates since it references the same@color/ic_launcher_background.Monochrome layer was wired to the colored
@mipmap/ic_launcher_foreground, so Android 13+ "Themed icons" mode tinted a multi-color asset and looked off. Now points at the existing@drawable/ic_launcher_monochromevector that was already in the tree but unused. Themed-icons mode now produces a properly wallpaper-tinted silhouette.Desktop platforms (
.icns/.ico/.pngincomposeApp/src/jvmMain/resources/logo/) are deliberately untouched in this PR — they need a full design pass with a 1024×1024 master to regenerate cleanly. Followup PR once Android version is validated visually.Closes #534.
Summary by CodeRabbit