[CHA-848] feat: edit and delete example #108
Conversation
WalkthroughThis pull request enhances the chat application by adding functionality for editing and deleting messages. A new state variable tracks the message being edited, while functions are introduced to manage sending new messages and updating existing ones. The message subscription logic is updated to handle Created, Updated, and Deleted events. Additionally, UI components are modified to accept callbacks for editing and deleting messages, and the chat input field changes its button label based on the editing state. Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Code Coverage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
example/src/main/java/com/ably/chat/example/MainActivity.kt (1)
290-363: 🛠️ Refactor suggestionMessageBubble exceeds length limit and needs style fixes
The MessageBubble composable is well-implemented but has a few issues:
- The function is too long (72 lines) - the maximum allowed is 60 lines
- Missing trailing commas before ")" on lines 350 and 359
Consider extracting the dropdown menu and confirmation dialog into separate composable functions to reduce the length:
@Composable fun MessageBubble(message: Message, onEdit: () -> Unit = {}, onDelete: () -> Unit = {}) { var expanded by remember { mutableStateOf(false) } var confirmationDialogShown by remember { mutableStateOf(false) } Row( modifier = Modifier .fillMaxWidth() .padding(8.dp), horizontalArrangement = if (message.clientId == randomClientId) Arrangement.End else Arrangement.Start, ) { Box( modifier = Modifier .background( color = if (message.clientId != randomClientId) Color.Blue else Color.Gray, shape = RoundedCornerShape(8.dp), ) .padding(12.dp), ) { Text( text = message.text, color = Color.White, ) } if (message.clientId == randomClientId) { - Box { - IconButton(onClick = { expanded = true }) { - Icon(Icons.Default.MoreVert, contentDescription = "More Options") - } - - DropdownMenu( - expanded = expanded, - onDismissRequest = { expanded = false }, - ) { - DropdownMenuItem( - text = { Text("Edit") }, - onClick = { - expanded = false - onEdit() - }, - ) - DropdownMenuItem( - text = { Text("Delete") }, - onClick = { - expanded = false - confirmationDialogShown = true - }, - ) - } - } + MessageOptions( + expanded = expanded, + onExpandChange = { expanded = it }, + onEdit = onEdit, + onDeleteRequest = { confirmationDialogShown = true } + ) } if (confirmationDialogShown) { - AlertDialog( - onDismissRequest = { confirmationDialogShown = false }, - title = { Text("Delete Message") }, - text = { Text("Are you sure you want to delete this message?") }, - confirmButton = { - Button( - onClick = { - onDelete() - confirmationDialogShown = false - } - ) { - Text("Delete") - } - }, - dismissButton = { - Button(onClick = { confirmationDialogShown = false }) { - Text("Cancel") - } - } - ) + DeleteConfirmationDialog( + onConfirm = { + onDelete() + confirmationDialogShown = false + }, + onDismiss = { confirmationDialogShown = false } + ) } } } @Composable private fun MessageOptions( expanded: Boolean, onExpandChange: (Boolean) -> Unit, onEdit: () -> Unit, onDeleteRequest: () -> Unit ) { Box { IconButton(onClick = { onExpandChange(true) }) { Icon(Icons.Default.MoreVert, contentDescription = "More Options") } DropdownMenu( expanded = expanded, onDismissRequest = { onExpandChange(false) }, ) { DropdownMenuItem( text = { Text("Edit") }, onClick = { onExpandChange(false) onEdit() }, ) DropdownMenuItem( text = { Text("Delete") }, onClick = { onExpandChange(false) onDeleteRequest() }, ) } } } @Composable private fun DeleteConfirmationDialog( onConfirm: () -> Unit, onDismiss: () -> Unit ) { AlertDialog( onDismissRequest = onDismiss, title = { Text("Delete Message") }, text = { Text("Are you sure you want to delete this message?") }, confirmButton = { Button( onClick = onConfirm, ) { Text("Delete") } }, dismissButton = { Button(onClick = onDismiss) { Text("Cancel") } }, ) }🧰 Tools
🪛 GitHub Actions: Check
[warning] 290-290: The function MessageBubble is too long (72). The maximum length is 60. [LongMethod]
[warning] 359-359: Missing trailing comma before ")" [TrailingCommaOnCallSite]
[warning] 350-350: Missing trailing comma before ")" [TrailingCommaOnCallSite]
🧹 Nitpick comments (1)
example/src/main/java/com/ably/chat/example/MainActivity.kt (1)
156-165: Consider refactoring the Chat composableThe pipeline failures indicate that the Chat function is too complex (complexity: 18, threshold: 15). With the addition of edit functionality, consider refactoring this function to reduce cognitive complexity.
You could extract some of the logic into smaller helper functions or composables, such as:
- Extract message handling logic into a separate function
- Extract the message subscription logic into a custom hook
- Consider using a ViewModel to manage the chat state and logic
🧰 Tools
🪛 GitHub Actions: Check
[warning] 156-156: The function Chat appears to be too complex based on Cognitive Complexity (complexity: 18). Defined complexity threshold for methods is set to '15' [CognitiveComplexMethod]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
example/src/main/java/com/ably/chat/example/MainActivity.kt(8 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check
example/src/main/java/com/ably/chat/example/MainActivity.kt
[warning] 290-290: The function MessageBubble is too long (72). The maximum length is 60. [LongMethod]
[warning] 156-156: The function Chat appears to be too complex based on Cognitive Complexity (complexity: 18). Defined complexity threshold for methods is set to '15' [CognitiveComplexMethod]
[warning] 359-359: Missing trailing comma before ")" [TrailingCommaOnCallSite]
[warning] 350-350: Missing trailing comma before ")" [TrailingCommaOnCallSite]
[warning] 210-210: Unexpected blank line(s) before "}" [NoBlankLineBeforeRbrace]
[warning] 198-198: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
[warning] 204-204: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
[warning] 392-392: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
[warning] 398-398: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
[warning] 404-404: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: coverage
🔇 Additional comments (5)
example/src/main/java/com/ably/chat/example/MainActivity.kt (5)
163-164: Clean implementation of edit mode state!Good use of a nullable state variable for tracking the message being edited, with a derived boolean to simplify checks.
166-186: Well-structured message handling functionsGood separation of concerns between sending new messages and updating existing ones. The early return pattern with labeled function reference in
handleEditis a nice approach for handling the null case.
245-257: Appropriate callback implementationThe implementation of the edit and delete callbacks is clean and effective. When editing, you correctly update both the edited state and the input field text.
260-282: Clean ChatInputField usage with updating stateGood use of the new
updatingparameter and conditional logic for sending vs updating messages.
20-22: Good addition of necessary importsAppropriate imports were added for the new UI components (MoreVert icon, AlertDialog, DropdownMenu, DropdownMenuItem) and for handling message events.
Also applies to: 24-25, 50-50
| when { | ||
| updating -> { | ||
| Button(enabled = !sending, onClick = onSendClick) { | ||
| Text("Update") | ||
| } | ||
| } | ||
| } else { | ||
| Button(onClick = onReactionClick) { | ||
| Text("\uD83D\uDC4D") | ||
|
|
||
| messageInput.text.isEmpty() -> { | ||
| Button(onClick = onReactionClick) { | ||
| Text("\uD83D\uDC4D") | ||
| } | ||
| } | ||
|
|
||
| else -> { | ||
| Button(enabled = !sending, onClick = onSendClick) { | ||
| Text("Send") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix style issues in ChatInputField
Remove the extra braces on when statements to comply with the style guidelines.
- when {
- updating -> {
+ when {
+ updating ->
Button(enabled = !sending, onClick = onSendClick) {
Text("Update")
}
- }
-
- messageInput.text.isEmpty() -> {
+ messageInput.text.isEmpty() ->
Button(onClick = onReactionClick) {
Text("\uD83D\uDC4D")
}
- }
-
- else -> {
+ else ->
Button(enabled = !sending, onClick = onSendClick) {
Text("Send")
}
- }
}📝 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.
| when { | |
| updating -> { | |
| Button(enabled = !sending, onClick = onSendClick) { | |
| Text("Update") | |
| } | |
| } | |
| } else { | |
| Button(onClick = onReactionClick) { | |
| Text("\uD83D\uDC4D") | |
| messageInput.text.isEmpty() -> { | |
| Button(onClick = onReactionClick) { | |
| Text("\uD83D\uDC4D") | |
| } | |
| } | |
| else -> { | |
| Button(enabled = !sending, onClick = onSendClick) { | |
| Text("Send") | |
| } | |
| } | |
| } | |
| when { | |
| updating -> | |
| Button(enabled = !sending, onClick = onSendClick) { | |
| Text("Update") | |
| } | |
| messageInput.text.isEmpty() -> | |
| Button(onClick = onReactionClick) { | |
| Text("\uD83D\uDC4D") | |
| } | |
| else -> | |
| Button(enabled = !sending, onClick = onSendClick) { | |
| Text("Send") | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Check
[warning] 392-392: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
[warning] 398-398: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
[warning] 404-404: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
| val subscription = room.messages.subscribe { event -> | ||
| when (event.type) { | ||
| MessageEventType.Created -> { | ||
| messages += event.message | ||
| coroutineScope.launch { | ||
| listState.animateScrollToItem(messages.size - 1) | ||
| } | ||
| } | ||
|
|
||
| MessageEventType.Updated -> { | ||
| messages = messages.map { | ||
| if (it.serial != event.message.serial) it else event.message | ||
| } | ||
| } | ||
|
|
||
| MessageEventType.Deleted -> { | ||
| messages = messages.filter { | ||
| it.serial != event.message.serial | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Message event handling needs style fixes
While the logic for handling different message event types is solid, there are style issues to address:
- Remove extra braces on when statements (lines 198 and 204)
- Remove the unexpected blank line before closing brace (line 210)
val subscription = room.messages.subscribe { event ->
when (event.type) {
MessageEventType.Created -> {
messages += event.message
coroutineScope.launch {
listState.animateScrollToItem(messages.size - 1)
}
}
- MessageEventType.Updated -> {
+ MessageEventType.Updated ->
messages = messages.map {
if (it.serial != event.message.serial) it else event.message
}
- }
+
- MessageEventType.Deleted -> {
+ MessageEventType.Deleted ->
messages = messages.filter {
it.serial != event.message.serial
}
- }
+
}
-
}📝 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.
| val subscription = room.messages.subscribe { event -> | |
| when (event.type) { | |
| MessageEventType.Created -> { | |
| messages += event.message | |
| coroutineScope.launch { | |
| listState.animateScrollToItem(messages.size - 1) | |
| } | |
| } | |
| MessageEventType.Updated -> { | |
| messages = messages.map { | |
| if (it.serial != event.message.serial) it else event.message | |
| } | |
| } | |
| MessageEventType.Deleted -> { | |
| messages = messages.filter { | |
| it.serial != event.message.serial | |
| } | |
| } | |
| } | |
| val subscription = room.messages.subscribe { event -> | |
| when (event.type) { | |
| MessageEventType.Created -> { | |
| messages += event.message | |
| coroutineScope.launch { | |
| listState.animateScrollToItem(messages.size - 1) | |
| } | |
| } | |
| MessageEventType.Updated -> | |
| messages = messages.map { | |
| if (it.serial != event.message.serial) it else event.message | |
| } | |
| MessageEventType.Deleted -> | |
| messages = messages.filter { | |
| it.serial != event.message.serial | |
| } | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Check
[warning] 210-210: Unexpected blank line(s) before "}" [NoBlankLineBeforeRbrace]
[warning] 198-198: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
[warning] 204-204: Extra braces exist on this branch, remove them. [BracesOnWhenStatements]
ttypic
force-pushed
the
CHA-848/feat-edit-delete-example
branch
from
e502e33 to
d86c94b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
example/src/main/java/com/ably/chat/example/MainActivity.kt (2)
189-205: 🛠️ Refactor suggestionFix style issues in message event handling
While the logic for handling different message event types is correct, there are style issues with the when statement structure.
Apply this diff to fix the style issues:
val subscription = room.messages.subscribe { event -> when (event.type) { MessageEventType.Created -> { messages += event.message coroutineScope.launch { listState.animateScrollToItem(messages.size - 1) } } - MessageEventType.Updated -> messages = messages.map { - if (it.serial != event.message.serial) it else event.message - } + MessageEventType.Updated -> + messages = messages.map { + if (it.serial != event.message.serial) it else event.message + } - MessageEventType.Deleted -> messages = messages.filter { - it.serial != event.message.serial - } + MessageEventType.Deleted -> + messages = messages.filter { + it.serial != event.message.serial + } } }
365-365: 🛠️ Refactor suggestionFix style issues in ChatInputField
The when statement in ChatInputField doesn't follow the recommended style guidelines.
Apply this diff to fix the style issues:
when { - updating -> Button(enabled = !sending, onClick = onSendClick) { - Text("Update") - } + updating -> + Button(enabled = !sending, onClick = onSendClick) { + Text("Update") + } - messageInput.text.isEmpty() -> Button(onClick = onReactionClick) { + messageInput.text.isEmpty() -> + Button(onClick = onReactionClick) { Text("\uD83D\uDC4D") } - else -> Button(enabled = !sending, onClick = onSendClick) { - Text("Send") - } + else -> + Button(enabled = !sending, onClick = onSendClick) { + Text("Send") + } }Also applies to: 387-399
🧹 Nitpick comments (5)
example/src/main/java/com/ably/chat/example/MainActivity.kt (5)
154-154: Consider refactoring to address complexityThe addition of warning suppressions for "LongMethod" and "CognitiveComplexMethod" indicates the Chat composable is becoming complex. While this works for now, consider breaking it into smaller, more focused components in the future.
284-358: Well-implemented message bubble with edit/delete optionsThe MessageBubble composable is well implemented with dropdown menu for edit and delete options, and a confirmation dialog for delete action. Good use of state management for expanded menu and confirmation dialog.
Consider adding error handling for the delete operation to provide feedback to the user if the deletion fails.
422-438: Update MessageBubblePreview to include new parametersThe MessageBubblePreview doesn't include the new onEdit and onDelete parameters. Update it to fully test the updated composable.
@Preview @Composable fun MessageBubblePreview() { AblyChatExampleTheme { MessageBubble( message = Message( text = "Hello World!", serial = "fake", roomId = "roomId", clientId = "clientId", createdAt = System.currentTimeMillis(), metadata = MessageMetadata(), headers = mapOf(), action = MessageAction.MESSAGE_CREATE, version = "fake", timestamp = System.currentTimeMillis(), ), + onEdit = {}, + onDelete = {}, ) } }
441-453: Update ChatInputPreview to include the new updating parameterThe ChatInputPreview doesn't include the new updating parameter. Update it to fully test both states of the composable.
@Preview @Composable fun ChatInputPreview() { AblyChatExampleTheme { ChatInputField( sending = false, + updating = false, messageInput = TextFieldValue(""), onMessageChange = {}, onSendClick = {}, onReactionClick = {}, ) } } +@Preview +@Composable +fun ChatInputUpdatingPreview() { + AblyChatExampleTheme { + ChatInputField( + sending = false, + updating = true, + messageInput = TextFieldValue("Test message"), + onMessageChange = {}, + onSendClick = {}, + onReactionClick = {}, + ) + } +}
166-186: Add error handling for message operationsThe current implementation doesn't handle errors that may occur during message operations (send, update). Consider adding try-catch blocks to provide feedback to the user if these operations fail.
val handleSend = { coroutineScope.launch { + try { room.messages.send( text = messageText.text, ) messageText = TextFieldValue("") sending = false + } catch (e: Exception) { + // Handle error, e.g., show a toast or error message + sending = false + } } } val handleEdit = handleEdit@{ val editedMessage = edited ?: return@handleEdit coroutineScope.launch { + try { room.messages.update( editedMessage.copy(text = messageText.text), ) messageText = TextFieldValue("") edited = null sending = false + } catch (e: Exception) { + // Handle error, e.g., show a toast or error message + sending = false + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
example/src/main/java/com/ably/chat/example/MainActivity.kt(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: coverage
🔇 Additional comments (7)
example/src/main/java/com/ably/chat/example/MainActivity.kt (7)
19-25: New imports for edit/delete functionalityThe added imports for MoreVert icon, AlertDialog, DropdownMenu, and DropdownMenuItem are appropriate for implementing the message editing and deletion functionality.
50-50: MessageEventType import for handling different message statesGood addition of the MessageEventType import which is essential for handling the Created, Updated, and Deleted message events.
163-164: Well-structured state management for edit modeGood implementation of the edited state variable and the derived updating state. This clearly separates the message editing state from other concerns.
166-174: Well-implemented message sending handlerThe handleSend function properly encapsulates the message sending logic, including resetting the input and sending state after completion.
176-186: Edit handler with proper null safetyThe handleEdit function correctly handles the null case with an early return and properly manages state after updating a message.
240-251: Good implementation of message optionsThe MessageBubble now properly accepts callbacks for edit and delete actions. This is a clean way to handle these operations while keeping the component reusable.
267-270: Well-structured message sending logicThe when block correctly directs message actions based on whether you're updating an existing message or sending a new one.
There was a problem hiding this comment.
You might like to update screenshot at https://github.com/ably/ably-chat-kotlin/blob/main/example/README.md if there are releted UI changes.
Otherwise LGTM
ttypic
force-pushed
the
CHA-848/feat-edit-delete-example
branch
from
d86c94b to
8c27173
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
example/src/main/java/com/ably/chat/example/MainActivity.kt (4)
154-154: Consider renaming the suppressions for better readability.The current annotation suppresses warnings for "LongMethod" and "CognitiveComplexMethod". Instead of suppressing these warnings, consider refactoring the Chat composable into smaller, more focused components to reduce complexity.
284-359: Comprehensive MessageBubble implementation with edit/delete features.The implementation includes:
- Edit and delete options via dropdown menu
- Confirmation dialog for message deletion
- Proper handling of different user messages (only showing options for own messages)
Consider adding error handling for the delete operation to gracefully handle potential failures.
- onDelete() + try { + onDelete() + } catch (e: Exception) { + // Handle error (e.g. show a snackbar) + Log.e("MessageBubble", "Failed to delete message", e) + }
420-439: Update MessageBubblePreview to include new parameters.The preview doesn't include the new onEdit and onDelete parameters, which would be useful for testing the UI in different states.
@Composable fun MessageBubblePreview() { AblyChatExampleTheme { MessageBubble( message = Message( text = "Hello World!", serial = "fake", roomId = "roomId", clientId = "clientId", createdAt = System.currentTimeMillis(), metadata = MessageMetadata(), headers = mapOf(), action = MessageAction.MESSAGE_CREATE, version = "fake", timestamp = System.currentTimeMillis(), ), + onEdit = {}, + onDelete = {}, ) } }
441-453: Update ChatInputPreview to include the updating parameter.The preview doesn't include the new updating parameter, which would be useful for testing both states of the input field.
@Composable fun ChatInputPreview() { AblyChatExampleTheme { ChatInputField( sending = false, + updating = false, messageInput = TextFieldValue(""), onMessageChange = {}, onSendClick = {}, onReactionClick = {}, ) } } +@Preview +@Composable +fun ChatInputUpdatingPreview() { + AblyChatExampleTheme { + ChatInputField( + sending = false, + updating = true, + messageInput = TextFieldValue("Edited message"), + onMessageChange = {}, + onSendClick = {}, + onReactionClick = {}, + ) + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
images/example-app-screenshot.pngis excluded by!**/*.png
📒 Files selected for processing (1)
example/src/main/java/com/ably/chat/example/MainActivity.kt(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: coverage
- GitHub Check: check
🔇 Additional comments (6)
example/src/main/java/com/ably/chat/example/MainActivity.kt (6)
163-164: LGTM! Clean implementation of edit state management.The state variable
editedand the derived stateupdatingprovide a clear way to track when a message is being edited.
166-186: Well-structured message handling functions.The separation of
handleSendandhandleEditfunctions makes the code more maintainable and follows the single responsibility principle. The null check with early return inhandleEditis a good defensive programming practice.
189-205: Message event handling implementation is solid.The code properly handles all message event types (Created, Updated, Deleted) and maintains the message list state appropriately. The style fixes from previous reviews have been implemented correctly.
240-251: Good implementation of message edit and delete functionality.The MessageBubble component now receives edit and delete callbacks, which are correctly implemented in the parent component.
267-269: Clean conditional handling for send vs. update.Using a when expression to choose between different actions based on the update state is clean and readable.
387-398: LGTM! UI state handling follows style guidelines.The ChatInputField now correctly shows "Update" or "Send" based on the state, and the code follows the style guidelines (no extra braces on when statements).
Added example of edits and deletes to the example app
ttypic
force-pushed
the
CHA-848/feat-edit-delete-example
branch
from
8c27173 to
a747c9d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
example/src/main/java/com/ably/chat/example/MainActivity.kt (3)
154-154: Consider refactoring this large methodThe
@SuppressWarnings("LongMethod", "CognitiveComplexMethod")annotation indicates that this method is complex. Long methods are harder to understand, test, and maintain.Consider breaking down the
Chatcomposable into smaller, more focused composables to improve readability and maintainability. For example, you could extract the message handling logic into a separate function or create a dedicated composable for the messages list.
422-439: Update the preview to include new parametersThe
MessageBubblePreviewdoesn't include the newonEditandonDeleteparameters. While they have default values and will compile, it's good practice to update previews to reflect the current implementation.MessageBubble( message = Message( text = "Hello World!", serial = "fake", roomId = "roomId", clientId = "clientId", createdAt = System.currentTimeMillis(), metadata = MessageMetadata(), headers = mapOf(), action = MessageAction.MESSAGE_CREATE, version = "fake", timestamp = System.currentTimeMillis(), ), + onEdit = {}, + onDelete = {}, )
441-453: Update ChatInputPreview with updating parameterThe
ChatInputPreviewshould be updated to include the newupdatingparameter to fully represent the current implementation.AblyChatExampleTheme { ChatInputField( sending = false, + updating = false, messageInput = TextFieldValue(""), onMessageChange = {}, onSendClick = {}, onReactionClick = {}, ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
images/example-app-screenshot.pngis excluded by!**/*.png
📒 Files selected for processing (1)
example/src/main/java/com/ably/chat/example/MainActivity.kt(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: coverage
🔇 Additional comments (8)
example/src/main/java/com/ably/chat/example/MainActivity.kt (8)
20-25: Good use of UI components for edit/delete functionalityThe added imports for MoreVert icon, AlertDialog, DropdownMenu, and DropdownMenuItem are appropriate for implementing the edit and delete functionality as they provide the necessary UI components for these actions.
163-164: LGTM: Good state management for edit modeThe introduction of an
editedstate to track the message being edited and a derivedupdatingboolean is a clean approach for managing the edit state.
166-186: Well-structured message handling functionsThe separation of
handleSendandhandleEditfunctions improves code organization and makes the logic clear. The early return inhandleEditwith a labeled return is a good defensive practice.
189-205: Message event handling is well-implementedThe implementation correctly handles all three message event types (Created, Updated, Deleted) with appropriate state updates and UI reactions. The style has been fixed according to previous review comments.
240-251: Good implementation of edit and delete actionsThe added callbacks to
MessageBubbleenable the edit and delete functionality with appropriate state management. The message deletion is properly handled through the Room API.
267-270: Clean conditional handling of send vs editThe implementation correctly routes the action based on whether the user is updating an existing message or sending a new one.
286-358: Well-implemented message bubble with options menuThe enhanced
MessageBubblecomposable now provides a user-friendly way to edit and delete messages:
- The dropdown menu appears only for the current user's messages
- The confirmation dialog for deletion prevents accidental deletions
- The callbacks properly integrate with the parent composable's logic
The UI implementation follows best practices for user interaction.
387-399: Button state handling improved with clean syntaxThe button display logic now correctly switches between "Update", "Send", and reaction buttons based on the state. The style complies with previous review comments by removing extra braces.
Edit and delete example for the Chat example app
Summary by CodeRabbit