-
Notifications
You must be signed in to change notification settings - Fork 309
Introduce contextual menu content & item #6091
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: redesign/avatar
Are you sure you want to change the base?
Conversation
SDK Size Comparison 📏
|
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR introduces a new contextual menu UI component for Android Compose alongside supporting theme enhancements. A new ContextualMenu.kt file adds three composables: ContextualMenuContent for styling, ContextualMenuItem for individual menu items with state-based styling, and ContextualMenuDivider for dividers. The theme system is extended with two new colors and one new design token. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamColors.kt`:
- Around line 175-179: Add KDoc comments for the new public constructor
properties textSecondary and backgroundElevationElevation2 in the StreamColors
class: document textSecondary as "Used for secondary text color (e.g., captions,
metadata)" and backgroundElevationElevation2 as "Used for elevated surface
backgrounds at elevation level 2"; place the KDoc immediately above the
corresponding public val declarations so they appear in generated docs. Also add
a brief note in the StreamColors class KDoc or PR description that adding these
required constructor parameters is a source/binary-breaking change for consumers
who instantiate StreamColors(...) directly and confirm this is intentional.
🧹 Nitpick comments (1)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/common/ContextualMenu.kt (1)
129-145: Use StreamPreview helper for Compose previews.This preview should use the repo’s StreamPreview helper instead of the plain
@Previewannotation. As per coding guidelines, Compose previews should use StreamPreview helpers.♻️ Proposed change
-@Preview(showBackground = true) +@StreamPreview(showBackground = true)
| public val textPrimary: Color, | ||
| public val textSecondary: Color, | ||
| public val stateBgDisabled: Color, | ||
| public val stateTextDisabled: Color, | ||
|
|
||
| public val backgroundElevationElevation2: Color, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and read the StreamColors.kt file to understand the full structure
fd -n "StreamColors.kt" --type fRepository: GetStream/stream-chat-android
Length of output: 303
🏁 Script executed:
#!/bin/bash
# Find the StreamColors.kt file
fd "StreamColors.kt" --type fRepository: GetStream/stream-chat-android
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Read the full StreamColors.kt file to understand the structure and KDoc
cat -n stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamColors.ktRepository: GetStream/stream-chat-android
Length of output: 28262
🏁 Script executed:
#!/bin/bash
# Find direct StreamColors constructor usage that may require updates
rg -n --type=kt 'StreamColors\s*\(' | head -50Repository: GetStream/stream-chat-android
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Find direct StreamColors constructor usage
rg -n 'StreamColors\s*\(' --type=kotlin | head -50Repository: GetStream/stream-chat-android
Length of output: 553
🏁 Script executed:
#!/bin/bash
# More thorough search for any StreamColors instantiation patterns
rg -n 'StreamColors' --type=kotlin -C 2 | grep -E '(StreamColors\(|\.copy\(|= StreamColors)' | head -100Repository: GetStream/stream-chat-android
Length of output: 712
🏁 Script executed:
#!/bin/bash
# Check if there are any test files that might instantiate StreamColors directly
fd -e ".kt" | xargs rg -l "StreamColors" | xargs rg 'StreamColors\s*\([^)]*='Repository: GetStream/stream-chat-android
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Search for StreamColors instantiation in test files and other locations
find . -name "*.kt" -type f | xargs grep -l "StreamColors(" | xargs grep -A 5 "StreamColors("Repository: GetStream/stream-chat-android
Length of output: 1298
Add KDoc for new public parameters.
The textSecondary and backgroundElevationElevation2 parameters are missing KDoc. Per coding guidelines, all public APIs must be documented with KDoc. Add documentation explaining the purpose of each parameter:
textSecondary: Brief description of its use case (e.g., "Used for secondary text color")backgroundElevationElevation2: Brief description of its use case (e.g., "Used for elevated surface backgrounds")
Additionally, note that adding required constructor parameters is a source/binary-breaking change for external consumers who instantiate StreamColors(...) directly. Confirm this is intentional.
🤖 Prompt for AI Agents
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamColors.kt`
around lines 175 - 179, Add KDoc comments for the new public constructor
properties textSecondary and backgroundElevationElevation2 in the StreamColors
class: document textSecondary as "Used for secondary text color (e.g., captions,
metadata)" and backgroundElevationElevation2 as "Used for elevated surface
backgrounds at elevation level 2"; place the KDoc immediately above the
corresponding public val declarations so they appear in generated docs. Also add
a brief note in the StreamColors class KDoc or PR description that adding these
required constructor parameters is a source/binary-breaking change for consumers
who instantiate StreamColors(...) directly and confirm this is intentional.


🎯 Goal
Introduce the contextual menu content & item components.
🛠 Implementation details
For the time being, this is just the menu content itself and there is no behavior associated to it, e.g. it's not acting as a popup out of the box, but can be used in one.
🎨 UI Changes
Figma
🧪 Testing
It's not used yet, but you can check the
@Preview☑️Contributor Checklist
General
Code & documentation
☑️Reviewer Checklist
🎉 GIF
Please provide a suitable gif that describes your work on this pull request
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.