Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughReorganized Appearance.ColorPalette into semantic groups (Accent, Background/Surface, Text, Border, Avatar, Badge, Button, Chat, Control, Input, Presence, Reaction, Navigation/Utilities) and added many new public lazy color tokens; behavior and UIKit wiring remain unchanged. (45 words) Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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 |
| @@ -8,124 +8,276 @@ import UIKit | |||
|
|
|||
| public extension Appearance { | |||
| @MainActor final class ColorPalette { | |||
There was a problem hiding this comment.
No color changes, just docs from Figma (or autogenerated). Regrouped as well.
Generated by 🚫 Danger |
![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 the current code and only fix it if needed.
Inline comments:
In `@Sources/StreamChatCommonUI/Appearance`+ColorPalette.swift:
- Around line 27-28: The documentation for backgroundCoreElevation0 is
incorrect—it's not "Always white" because the property returns chrome0 which
follows light/dark appearances; update the comment for the
backgroundCoreElevation0 property (referencing backgroundCoreElevation0 and
chrome0) to state that it's the base/background layer that uses chrome0 and may
vary with appearance (e.g., light/dark) rather than claiming it's always white.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9fcb306e-b0a4-4c81-b00a-3346bd9a7d83
📒 Files selected for processing (1)
Sources/StreamChatCommonUI/Appearance+ColorPalette.swift
martinmitrevski
left a comment
martinmitrevski
left a comment
There was a problem hiding this comment.
nice, we should also put something similar in the docs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/StreamChatCommonUI/Appearance+ColorPalette.swift (1)
278-298:⚠️ Potential issue | 🟡 MinorThe navigation section isn't strictly SwiftUI-only.
This header reads too narrowly:
navigationBarTitlein the same block updatesUINavigationBar.appearance()directly, and the properties are still public onAppearance.ColorPalette. I'd either remove the platform qualifier here or scope it to the specific tokens that are truly SwiftUI-only.✏️ Suggested doc fix
- // MARK: - Navigation (SwiftUI SDK only) + // MARK: - Navigation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/StreamChatCommonUI/Appearance`+ColorPalette.swift around lines 278 - 298, The "Navigation (SwiftUI SDK only)" header is misleading because properties like navigationBarTitle, navigationBarSubtitle, navigationBarBackground, navigationBarTintColor, and navigationBarGlyph live on Appearance.ColorPalette and directly update UINavigationBar.appearance(); update the doc comment to remove or broaden the "SwiftUI SDK only" qualifier (or alternatively mark only truly SwiftUI-specific tokens) so the header accurately reflects that these tokens apply to UIKit as well; ensure the comment above this block clearly states which tokens are UIKit/SwiftUI shared versus any that are SwiftUI-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/StreamChatCommonUI/Appearance`+ColorPalette.swift:
- Around line 39-40: The doc for backgroundCoreSurfaceCard is misleading because
it states it "matches the surface in light mode" while backgroundCoreSurfaceCard
actually maps to chrome50 (lifted) and backgroundCoreSurfaceDefault maps to
chrome100; update the documentation comment on the backgroundCoreSurfaceCard
property to state that it is a slightly lifted card surface in both light and
dark modes (e.g., "Background for contained, card-style elements; a slightly
lifted surface (chrome50 in light, chrome100 in dark)"), and optionally mention
backgroundCoreSurfaceDefault for comparison to make the distinction clear.
---
Outside diff comments:
In `@Sources/StreamChatCommonUI/Appearance`+ColorPalette.swift:
- Around line 278-298: The "Navigation (SwiftUI SDK only)" header is misleading
because properties like navigationBarTitle, navigationBarSubtitle,
navigationBarBackground, navigationBarTintColor, and navigationBarGlyph live on
Appearance.ColorPalette and directly update UINavigationBar.appearance(); update
the doc comment to remove or broaden the "SwiftUI SDK only" qualifier (or
alternatively mark only truly SwiftUI-specific tokens) so the header accurately
reflects that these tokens apply to UIKit as well; ensure the comment above this
block clearly states which tokens are UIKit/SwiftUI shared versus any that are
SwiftUI-only.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a234e760-0af3-4e2b-af79-77ec8dbea685
📒 Files selected for processing (1)
Sources/StreamChatCommonUI/Appearance+ColorPalette.swift
| /// Background for contained, card-style elements. Matches the surface in light mode but lifts slightly in dark mode. | ||
| public lazy var backgroundCoreSurfaceCard: UIColor = UIColor(light: chrome50, dark: chrome100) |
There was a problem hiding this comment.
Fix the backgroundCoreSurfaceCard doc wording.
The description says this token matches the default surface in light mode, but backgroundCoreSurfaceCard resolves to chrome50 while backgroundCoreSurfaceDefault resolves to chrome100. That means the card surface is lifted in light mode too, so the current doc is misleading for consumers comparing tokens.
✏️ Suggested doc fix
- /// Background for contained, card-style elements. Matches the surface in light mode but lifts slightly in dark mode.
+ /// Background for contained, card-style elements. Slightly lifted from the default surface, with additional separation in dark mode.
public lazy var backgroundCoreSurfaceCard: UIColor = UIColor(light: chrome50, dark: chrome100)📝 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.
| /// Background for contained, card-style elements. Matches the surface in light mode but lifts slightly in dark mode. | |
| public lazy var backgroundCoreSurfaceCard: UIColor = UIColor(light: chrome50, dark: chrome100) | |
| /// Background for contained, card-style elements. Slightly lifted from the default surface, with additional separation in dark mode. | |
| public lazy var backgroundCoreSurfaceCard: UIColor = UIColor(light: chrome50, dark: chrome100) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/StreamChatCommonUI/Appearance`+ColorPalette.swift around lines 39 -
40, The doc for backgroundCoreSurfaceCard is misleading because it states it
"matches the surface in light mode" while backgroundCoreSurfaceCard actually
maps to chrome50 (lifted) and backgroundCoreSurfaceDefault maps to chrome100;
update the documentation comment on the backgroundCoreSurfaceCard property to
state that it is a slightly lifted card surface in both light and dark modes
(e.g., "Background for contained, card-style elements; a slightly lifted surface
(chrome50 in light, chrome100 in dark)"), and optionally mention
backgroundCoreSurfaceDefault for comparison to make the distinction clear.
|
Public Interface🚀 No changes affecting the public interface. |
SDK Size
|
|
3 participants
🔗 Issue Links
IOS-1560
🎯 Goal
Add documentation to the
ColorPalettetokens based on the Figma Design System, making it easier for SDK consumers tounderstand and customize the color system.
📝 Summary
///doc comments to all 120 publicColorPaletteproperties with descriptions sourced from the Figma DesignSystem
// MARK:section headers to group tokens by semantic category (Accent, Background, Text, Border, Avatar,Badge, Button, Chat, Control, Input, Presence, Reaction, Navigation, Utilities)
🛠 Implementation
reference
patterns
documentation/organizational change with no behavioral impact
🎨 Showcase
Add relevant screenshots and/or videos/gifs to easily see what this PR changes, if applicable.
🧪 Manual Testing Notes
Explain how this change can be tested manually, if applicable.
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
New Features
Refactor
Documentation