-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Migrate Jetchat to new TextField #1543
base: main
Are you sure you want to change the base?
Conversation
This commit also integrates `contentReceiver`, dropping DragAndDrop modifier. `contentReceiver` already supports drag and drop, it additionally supports importing gifs and other rich content from keyboard or clipboard.
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.
Hello @halilozercan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request migrates the Jetchat application to use the new TextField component and integrates contentReceiver
, replacing the DragAndDrop modifier. The contentReceiver
now handles drag and drop, along with supporting GIFs and rich content from the keyboard or clipboard. The way media is attached to messages has also been updated, so that if a message contains no text, only the image is displayed. Coil-compose and coil-gif dependencies were added to simplify image display.
Highlights
- TextField Migration: Migrates Jetchat to use the new TextField component.
- Content Receiver Integration: Integrates
contentReceiver
and removes the DragAndDrop modifier, enabling support for drag and drop, GIFs, and rich content from the keyboard or clipboard. - Media Attachment Update: Changes how media is attached to messages; if there is no text content, only the image is shown.
- Dependency Addition: Adds coil-compose and coil-gif dependencies for simplified image display.
Changelog
Click here to see the changelog
- Jetchat/app/build.gradle.kts
- Added coil-compose and coil-gif dependencies.
- Jetchat/app/src/main/java/com/example/compose/jetchat/conversation/Conversation.kt
- Replaced
DragAndDropTarget
withcontentReceiver
. - Added
rememberTextFieldState
for the text field. - Implemented logic to handle images received via
contentReceiver
. - Modified message sending to handle images separately from text content.
- Added coil3.compose.AsyncImage to display images.
- Modified ChatItemBubble to conditionally render the chat bubble based on text content.
- Replaced
- Jetchat/app/src/main/java/com/example/compose/jetchat/conversation/ConversationUiState.kt
- Changed the type of
image
in theMessage
data class fromInt?
toAny?
to accommodate URIs.
- Changed the type of
- Jetchat/app/src/main/java/com/example/compose/jetchat/conversation/UserInput.kt
- Replaced
TextFieldValue
withTextFieldState
. - Added support for displaying and clearing selected images.
- Modified
UserInput
composable to accepttextFieldState
,images
, andonClearImage
parameters. - Updated
UserInputText
composable to useTextFieldState
. - Added
SelectedImages
composable to display selected images with a clear button. - Removed
keyboardActions
and usedonKeyboardAction
instead. - Added
lineLimits
toBasicTextField
.
- Replaced
- Jetchat/app/src/main/res/values/strings.xml
- Added a string resource for removing an image.
- Jetchat/gradle/libs.versions.toml
- Updated coil version to 3.1.0.
- Updated coil-kt-compose module to io.coil-kt.coil3:coil-compose.
- Added coil-kt-gif module io.coil-kt.coil3:coil-gif.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Trivia time!
What is the primary use case for Coil library in Android development?
Click here for the answer
Coil is primarily used for image loading in Android applications. It simplifies the process of fetching, decoding, and displaying images, and it's designed to be lightweight and easy to use with Kotlin coroutines.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces several enhancements to the Jetchat application, including the migration to a new TextField, integration of contentReceiver
for drag and drop and rich content support, and improved media attachment handling. The use of coil-compose and coil-gif dependencies simplifies image display. Overall, the changes improve the user experience and modernize the codebase.
Summary of Findings
- Potential NullPointerException: In the
onReceive
method of theReceiveContentListener
, theclipDataItem.text?.toString()
call could potentially lead to a NullPointerException ifclipDataItem.text
is null. Consider using the elvis operator with a more appropriate default value or handling the null case explicitly. - Image Loading Error Handling: The
AsyncImage
composable doesn't seem to have any error handling for image loading failures. It would be beneficial to add a placeholder or error message in case the image fails to load. - Efficiency of Image Sending: The current implementation sends each image as a separate message. Consider batching images into a single message to improve efficiency and reduce network overhead.
Merge Readiness
The pull request introduces valuable improvements to the Jetchat application. However, addressing the potential NullPointerException and adding error handling for image loading would enhance the robustness of the code. Consider batching images into a single message to improve efficiency and reduce network overhead. I am unable to approve this pull request, and recommend that others review and approve this code before merging. Given the high
severity issue identified, I recommend that this pull request not be merged until it is addressed.
This commit also integrates
contentReceiver
, dropping DragAndDrop modifier.contentReceiver
already supports drag and drop, it additionally supports importing gifs and other rich content from keyboard or clipboard.We also change how media is attached to each message. If there is no text content in the message, than the chat bubble is not rendered, only image bubble is shown.
Added coil-compose and coil-gif dependencies to show the images in the easiest way possible.