Added emoji reaction feature in chat room #4121
Conversation
WalkthroughThis pull request implements emoji reaction functionality for chat messages. Server-side updates include new methods in the WebSocket consumer to add and broadcast reactions, along with model and migration changes to support storing reactions in a JSONField. On the client side, JavaScript is enhanced with functions to handle user interactions with emoji pickers, send reaction events via WebSocket, and update the UI dynamically. Changes
Assessment against linked issues
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/consumers.py (1)
578-591: Consider including user information in reaction broadcasts.The reaction_update method effectively broadcasts reaction changes to all connected clients. However, for a richer UI, you might want to include information about which user added/removed each reaction, enabling customized displays for the current user's reactions.
async def reaction_update(self, event): """Handles broadcasting reaction updates to all users in the room.""" if not self.connected: return try: await self.send( text_data=json.dumps( - {"type": "reaction_update", "message_id": event["message_id"], "reactions": event["reactions"]} + { + "type": "reaction_update", + "message_id": event["message_id"], + "reactions": event["reactions"], + "current_user": self.scope["user"].username if self.scope["user"].is_authenticated else "Anonymous" + } ) ) except Exception as error: logger.error(f"Error sending reaction update: {str(error)}")website/templates/join_room.html (2)
60-73: Good emoji picker implementation, but consider adding more emojis.The emoji picker modal has a clean grid layout. Consider expanding beyond the current 8 emojis to give users more options, possibly with categories or a search feature for larger sets.
156-173: Effective reaction rendering with click functionality.The
updateReactionsfunction efficiently rebuilds the reaction display each time. Nice touch making each reaction clickable to toggle it on/off. Consider adding a visual indicator for reactions the current user has already added.function updateReactions(messageId, reactions) { const messageElement = document.querySelector(`[data-message-id="${messageId}"]`); if (!messageElement) return; const reactionsContainer = messageElement.querySelector('.reactions-container'); reactionsContainer.innerHTML = ''; Object.entries(reactions).forEach(([emoji, users]) => { + const username = '{{ request.user.username }}'; + const hasReacted = users.includes(username); const reactionButton = document.createElement('button'); - reactionButton.className = 'inline-flex items-center px-2 py-1 rounded-full bg-gray-100 hover:bg-gray-200 transition-colors duration-200'; + reactionButton.className = `inline-flex items-center px-2 py-1 rounded-full ${hasReacted ? 'bg-blue-100' : 'bg-gray-100'} hover:bg-gray-200 transition-colors duration-200`; reactionButton.innerHTML = ` <span class="mr-1">${emoji}</span> <span class="text-xs text-gray-600">${users.length}</span> `; reactionButton.onclick = () => addReaction(messageId, emoji); reactionsContainer.appendChild(reactionButton); }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
website/consumers.py(3 hunks)website/migrations/0237_message_reactions.py(1 hunks)website/models.py(1 hunks)website/templates/join_room.html(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run Tests
- GitHub Check: docker-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
website/migrations/0237_message_reactions.py (1)
1-18: Clean migration for adding reactions field.This is a well-structured migration that adds a JSONField to store emoji reactions. Using
default=dictensures new messages will have an empty reactions dictionary by default. The migration properly depends on the previous migration file.website/models.py (1)
2412-2412: Good implementation of reactions field.The JSONField is appropriately configured with a default empty dictionary and clear help text. This will effectively store emoji reactions and their counts for each message.
website/consumers.py (2)
396-420: Well-structured reaction handling logic.The
add_reactionmethod follows a good pattern for toggling reactions - adding a user to the emoji list if they haven't reacted or removing them if they have. The method handles error cases appropriately with null returns for non-existent messages or exceptions.
436-461: Proper validation and WebSocket message handling.Good validation of required parameters (message_id and emoji) before processing the reaction. The error responses are consistent with other WebSocket error patterns in the file. Broadcasting the reaction update to all clients follows the established pattern.
website/templates/join_room.html (5)
28-42: Clean implementation of the reaction UI elements.The reactions container is well-structured, and the initialization script for existing reactions follows best practices by waiting for DOM content to be loaded. The emoji button is appropriately styled with hover effects for better user experience.
119-155: Well-positioned emoji picker with proper event cleanup.The positioning of the emoji picker relative to the clicked button works well. Good implementation of click-outside listener cleanup to prevent memory leaks. The
addReactionfunction correctly sends the message to the WebSocket and handles socket state.
217-230: Good integration of reaction buttons in newly appended messages.The reaction button event listeners are properly set up for dynamically added messages, maintaining consistent behavior with existing messages.
246-248: Appropriate handling of reaction updates in the WebSocket message handler.The WebSocket message handler properly processes reaction updates and passes them to the updateReactions function.
360-379: Good setup of event handlers for emoji options and reaction buttons.Event delegation is used appropriately to attach handlers to emoji options and reaction buttons. The code stops event propagation to prevent unwanted clicks from bubbling up.
|
@coderabbitai review this full code and tell me if there is any error |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 94904 🏁 Script executed: Length of output: 7423 @igennova After reviewing the emoji reaction feature implementation, I've identified a few potential issues: Backend Issues:
Frontend Issues:
Security Considerations:
Recommendations:
The feature works functionally, but these improvements would make it more robust and user-friendly. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
website/templates/join_room.html (7)
28-42: Inline Reaction Script – Consider Centralizing Update Calls
The markup embeds a<script>tag inside each message block to callupdateReactionson DOMContentLoaded when reactions exist. This can lead to duplicated code and potential performance overhead when many messages are rendered. Consider centralizing the update logic (e.g. by iterating over all message elements after the DOM loads) and moving the script out of the loop.
60-81: Emoji Picker Modal – Accessibility & Maintainability
The emoji picker modal is well integrated with appropriate classes and transition classes. However, consider adding ARIA attributes (likerole="dialog"andaria-label) to improve accessibility. Additionally, if future designs require re-theming, moving these styles to an external CSS file might improve maintainability.
179-191: Smooth Dismissal incloseEmojiPickerFunction
Using a CSS class change combined with a timeout creates a smooth exit animation for the emoji picker. As an optional improvement, you might use the CSStransitionendevent instead of a fixed 200 ms timeout to better synchronize with the actual end of the transition.
212-229: Dynamic Reaction Rendering inupdateReactionsFunction – Accessibility Suggestions
The function rebuilds the reaction buttons dynamically and reflects the updated count. For improved accessibility and semantics, consider:
- Adding appropriate
aria-labelattributes to the reaction buttons (e.g. "React with [emoji], [count] reactions").- Setting the button type explicitly (e.g.
type="button") to avoid accidental form submissions in certain contexts.
245-251: New Message Rendering inappendMessage– Timestamp Handling
Within dynamically appended messages, the timestamp is hard-coded to "just now". If the backend provides a more precise timestamp, consider using it to help users better contextualize new messages. Otherwise, document that this is an intentional design decision for new messages.
416-424: Emoji Option Click Handler – User Feedback Opportunity
The event handler for emoji options uses the globally storedactiveEmojiButton. While it functions correctly when set, consider adding a fallback (or visual indication) if no active reaction button is detected. This would improve the user experience in edge cases.
453-467: Inline Style Injection for Emoji Picker Transitions – Consider External Stylesheet
The code dynamically injects a<style>block for emoji picker transitions. While this approach works in a single-page context, moving such styles to an external stylesheet could enhance maintainability and leverage browser caching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/templates/join_room.html(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Tests
- GitHub Check: docker-test
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
website/templates/join_room.html (3)
127-177: Dynamic Emoji Picker Positioning inshowEmojiPickerFunction
The logic for dynamically positioning the emoji picker based on available screen space is clear and thorough (including handling mobile centering). One minor consideration is that fetching the element (document.getElementById('emoji-picker')) on every call might be cached if used frequently, though it isn’t a critical issue. The addition of an outside click listener is appropriately placed.
193-197: Robust Outside Click Handling incloseEmojiPickerOnClickOutside
The function correctly checks whether the clicked target lies outside both the emoji picker and the reaction button, ensuring a proper close action. This implementation is concise and effective.
426-435: Existing Reaction Buttons Initialization
The block that attaches click listeners to existing.add-reaction-btnelements ensures that the emoji picker is shown for messages present at page load. This redundancy with the dynamic event attachment inappendMessageis acceptable to cover both initial and new messages.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
website/templates/join_room.html (9)
28-42: Review of Inline Reaction Update in Message Loop
The markup for displaying reactions is clear and correctly integrated into each message. However, embedding a script tag that adds a DOMContentLoaded listener for every message with reactions may lead to redundant event attachments if there are many messages. Consider initializing these updates once after the entire DOM is loaded or refactoring to update the reactions on demand.
60-72: Emoji Picker Modal Markup Accessibility
The emoji picker modal markup is well organized using a grid layout for emoji options. To improve accessibility, consider adding ARIA attributes (e.g.aria-label,role="dialog") so that assistive technologies can better interpret the modal content.
127-177: Dynamic Emoji Picker Positioning
TheshowEmojiPickerfunction effectively calculates the position of the emoji picker relative to the trigger button and adjusts for mobile screens. A couple of suggestions:
- Instead of a hard-coded picker height (
const pickerHeight = 200;), consider usingpicker.offsetHeightfor dynamic measurement.- Verify that attaching the
clicklistener for closing the picker (viacloseEmojiPickerOnClickOutside) on every invocation does not lead to unintended multiple registrations over time.
200-223: User Notification System
TheshowNotificationfunction effectively provides feedback with animated notifications. To further enhance accessibility, consider adding an ARIA role (e.g.role="alert") to the notification elements.
225-268: Asynchronous Reaction Submission and Reconnection Logic
TheaddReactionfunction robustly checks for a valid WebSocket connection and attempts reconnection if needed before sending the reaction payload. While this is a solid approach, be mindful of potential race conditions if users trigger multiple reactions in quick succession. Ensuring that concurrent reconnection attempts are handled gracefully would improve robustness.
398-440: WebSocket Connection Management and Error Handling
TheconnectWebSocketfunction correctly establishes a connection, manages reconnection attempts, and updates the UI based on connection status. It might be beneficial to log more detailed error information in theonerrorhandler to facilitate debugging if connection issues persist.
449-459: Deletion Request Functionality
ThedeleteMessagefunction sends a deletion request via WebSocket. Although the current implementation is simple and functional, adding error handling (in case the WebSocket fails to send the message) could further improve resilience.
461-467: HTML Escaping Utility
TheescapeHtmlfunction properly sanitizes input to prevent XSS vulnerabilities. For improved readability, consider reformatting the chained.replacecalls to reduce extraneous whitespace.
538-552: Inline Styling for Emoji Picker Transitions
The script that dynamically appends style rules for the emoji picker transition is a pragmatic solution for ensuring smooth animations. For long-term maintainability, consider moving these styles to an external CSS file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/templates/join_room.html(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run Tests
- GitHub Check: docker-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
website/templates/join_room.html (9)
102-103: WebSocket Reconnection Delay & Active Emoji Button State
The declaration ofconst reconnectDelay = 2000;andlet activeEmojiButton = null;is clear and appropriate. The reconnection delay appears reasonable, and the active emoji button state is properly tracked.
111-111: Emoji Picker Element Selection
The emoji picker is correctly selected viadocument.getElementById('emoji-picker'). Ensure that this element exists when the script executes (or defer the code accordingly).
179-191: Smooth Emoji Picker Closure
ThecloseEmojiPickerfunction provides a graceful fade-out and correctly cleans up the active emoji state and event listeners. The implementation is clear; just keep an eye out for any potential timing overlaps if the picker is triggered rapidly.
193-197: Click-Outside Handler Functionality
ThecloseEmojiPickerOnClickOutsidefunction correctly checks that clicks outside both the emoji picker and the trigger button result in closing the picker. This is a clean and efficient implementation.
270-308: Dynamic Reaction Update Handling
TheupdateReactionsfunction safely updates the reactions container by clearing the previous content and iterating over the provided reactions. It includes appropriate checks (such as ensuringusersis an array) and logs warnings for invalid data. This implementation meets the expected functionality.
311-345: Appending New Messages with Reaction Elements
TheappendMessagefunction integrates reaction-related UI elements seamlessly into new messages. While the static text "just now" is acceptable for immediate feedback, consider whether a dynamic timestamp (similar tonaturaltime) would be more consistent with existing messages.
368-396: WebSocket Message Dispatcher
ThehandleWebSocketMessagefunction uses a clear switch-case structure to manage different message types (includingreaction_update). This modular approach aids maintainability and aligns well with the overall chat functionality.
441-447: Message Deletion Handler
ThehandleDeleteMessagefunction efficiently removes a message element from the DOM when a deletion event is received. This straightforward approach is effective for the intended functionality.
469-529: Event Listener Initialization on DOMContentLoaded
The setup code within theDOMContentLoadedevent effectively initializes WebSocket connections, form handlers, emoji option handlers, reaction button listeners, and deletion handlers. This comprehensive initialization supports all interactive features. Ensure that dynamic element additions later (e.g., new messages) remain in sync with these event listeners.
|
@coderabbitai please refactor this to use a library either a js library or python library that has all of the emojis - we can show a list of the top 10 and have the option to select any of the others |
|
Oops, something went wrong! Please try again later. 🐰 💔 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
website/templates/join_room.html (7)
6-9: External CSS for EmojiMart
The new extra CSS block imports the EmojiMart stylesheet to apply the proper styles for the emoji picker. Consider verifying that the external CDN is reliably available or providing a fallback option (or self-hosting the CSS) in case of network issues.
32-46: Reactions Container Integration
This section injects a reactions container into each message along with an inline script that callsupdateReactionson DOM content loaded. Instead of attaching a separate DOMContentLoaded listener for each message, consider invokingupdateReactionsimmediately if the element is already in the DOM to improve efficiency and avoid potential timing issues.
64-77: Emoji Picker Modal Implementation
The newly added emoji picker modal is well structured with a dedicated quick emojis section and a full picker container. For enhanced accessibility, consider adding appropriate ARIA attributes (such asrole="dialog"andaria-modal="true") to improve usability for assistive technologies.
92-92: EmojiMart Script Inclusion
The external script inclusion for EmojiMart is correctly added. Ensure that you have fallbacks or error handling in place in case the CDN resource fails to load.
285-328: Handling Emoji Reactions via WebSocket
TheaddReactionfunction robustly handles sending emoji reactions through the WebSocket. It includes a reconnection mechanism and error notifications—which is excellent. Just be cautious that repeated reconnection attempts do not trigger overlapping connection attempts when multiple reactions are sent concurrently.
330-369: Dynamic Reaction Updates
TheupdateReactionsfunction efficiently clears and re-populates the reactions container for a message. For performance improvements, especially with many reactions or frequent updates, consider updating only the changed parts of the DOM rather than completely clearing the container each time.
597-612: Inline Styling for Emoji Picker Transitions
Appending a<style>element for emoji picker transitions directly into the document head ensures that the animation effects work immediately. For long-term maintainability, consider moving these styles into an external stylesheet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/templates/join_room.html(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (7)
website/templates/join_room.html (7)
99-100: WebSocket and Reaction Button Initialization
The initialization ofreconnectDelayandactiveEmojiButtonis straightforward and appropriate. No issues noted here.
240-251: Closing the Emoji Picker
ThecloseEmojiPickerfunction cleanly applies a fading transition to hide the emoji picker. The logic to remove the outside click listener and clear the active emoji state is correct.
260-283: User Notification System
TheshowNotificationfunction provides direct user feedback by dynamically creating and animating a notification element. This is a practical approach; consider further customizing the notifications for varying message types or durations if needed later.
371-426: Appending New Chat Messages
The updatedappendMessagefunction includes the reaction UI components seamlessly within each new message. The integration with delete and reaction button event handlers is consistent. Keep an eye on duplicated markup for maintenance, and consider abstracting common UI fragments if more features are added later.
561-570: Emoji Option Handlers
The event listener setup for.emoji-optionelements is a proactive step toward handling additional emoji selections. Verify that these elements exist in the rendered DOM (or are dynamically added) to ensure the events attach as expected.
571-580: Reaction Button Event Binding
The delegated event binding for.add-reaction-btnon existing messages is correctly implemented, ensuring that users can trigger the emoji picker as intended.
183-237:❓ Verification inconclusive
Positioning the Emoji Picker
TheshowEmojiPickerfunction implements dynamic positioning based on the available screen space – a solid approach for responsive design. However, note that the call toinitEmojiPicker()(line 189) is not awaited, which might lead to race conditions if the picker has not finished initializing before its styles are applied. Additionally, relying onpicker.offsetWidthfor layout calculations may be unreliable when the picker is hidden.
Attention: Emoji Picker Positioning and Initialization
- The call to
initEmojiPicker()is not awaited, which could lead to unpredictable behavior if its initialization is asynchronous. Please verify whetherinitEmojiPicker()returns a Promise. If so, await its completion before proceeding.- Relying on
picker.offsetWidthfor layout calculations can be unreliable if the emoji picker is hidden. Consider ensuring the picker is visible during measurements or use an alternative method to determine its width.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
website/templates/join_room.html (6)
32-42: Inline Reaction Update Script in Message Loop
The inline<script>that callsupdateReactionsfor messages with reactions is functional; however, embedding a DOMContentLoaded listener inside each message may lead to redundant event registrations when many messages are rendered. Consider refactoring by moving the update call to a centralized initialization (for example, after the entire messages container is built) to improve efficiency and maintainability.
64-77: Emoji Picker Modal Markup
The markup for the emoji picker modal is well structured and uses clear utility classes. To further enhance the feature, please consider adding ARIA attributes (e.g.,role="dialog",aria-modal="true") for accessibility and ensuring keyboard navigability.
310-334: User Notification System (showNotification)
The notification system is a clean, self-contained implementation to provide user feedback. The use of CSS transitions for a fade-out effect is effective. For further flexibility, you might consider making the display duration configurable or extracting the style definitions to a separate stylesheet for easier theming.
336-379: Robust Reaction Submission withaddReaction
TheaddReactionfunction incorporates a retry mechanism when the WebSocket isn’t ready, which is a thoughtful design. A few points for review:
- Consider abstracting the reconnection/retry logic into a dedicated helper to keep this function focused on sending the reaction.
- Be mindful of potential race conditions when multiple reactions are triggered in quick succession.
Overall, the error handling viashowNotificationis appropriate.
612-621: Event Listener Setup for Emoji Options
This block attaches click handlers to elements with the.emoji-optionclass. Verify that these elements are distinct from the quick emoji buttons created dynamically ininitEmojiPicker, and ensure that no duplicate event listeners are attached if elements overlap in functionality. Consolidating similar event handlers could improve clarity.
649-662: Inline Styles for Emoji Picker Transitions
Appending a<style>element dynamically to govern the picker’s opacity transition is a pragmatic solution. For maintainability, consider moving these styles into a dedicated CSS file or within a<style>block in the base template so that theme updates or overrides can be managed more centrally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/templates/join_room.html(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Tests
- GitHub Check: docker-test
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
website/templates/join_room.html (4)
6-9: Load External EmojiMart CSS
The new CSS block cleanly loads the EmojiMart stylesheet from a CDN. This is a straightforward inclusion and appears correct.
141-225: Asynchronous Emoji Picker Initialization (initEmojiPicker)
The async function efficiently handles both quick emoji button creation and the full picker initialization with a graceful loading state. The error handling that replaces the content and adds a Retry button is a nice touch. One suggestion is to review the hardcoded loading text for translation/localization if needed.
227-288: Dynamic Positioning inshowEmojiPicker
The function correctly awaits picker initialization and computes positioning based on available screen space. A couple of points for consideration:
- Verify that using
picker.offsetWidthyields accurate dimensions immediately after dynamic style adjustments (especially if the picker’s content may change).- Consider additional testing on diverse devices to ensure that the mobile and desktop positioning logic delivers consistent results.
381-420: Updating Reaction UI inupdateReactions
The function handles the update of reaction counts and creates clickable buttons for toggling reactions. The defensive checks (e.g., ensuring thatusersis an array) are commendable. Continue to monitor that the UI updates synchronize correctly with the underlying state, especially in high-concurrency scenarios.
|
/giphy Done |
|
|
|
@coderabbitai it looks like some code was removed. Was it just moved or was it removed? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@DonnieBLT Let me analyze what code changes were made to determine if code was removed or just moved: Based on my review of the current codebase, I haven't found evidence of code being removed without replacement. The PR appears to be additive, introducing emoji reaction features with:
Looking at the code architecture, it seems that emoji reactions were newly implemented rather than refactored from existing code. The changes follow an additive pattern where new functionality is layered on top of the existing chat system. Would you like me to check any specific sections of code where you think functionality might have been removed? 🏁 Script executed: Length of output: 452 🏁 Script executed: Length of output: 116380 🏁 Script executed: Length of output: 24281 🏁 Script executed: Length of output: 7463 🏁 Script executed: Length of output: 10573 🏁 Script executed: Length of output: 6545 @DonnieBLT Based on my analysis of the PR (4121), I can confirm that the code changes primarily involve the addition of the new emoji reaction feature:
The deletions (57 lines) appear to be from restructuring the existing chat UI to accommodate the new emoji reactions feature, rather than removing functionality. Based on the commit history, the emoji feature implementation required reorganizing some of the message display and handling code to properly integrate reactions. No standalone functions or key features appear to have been removed; instead, the code was restructured to enhance the chat with emoji reactions. |
|
@igennova the reactions are not working for anonymous users |
Working fine @DonnieBLT |
|
When I refresh they are gone |
|
Yours are gone too |
|
Also got this: SyntaxError: JSON Parse error: Single quotes (') are not allowed in JSON |
|
It's not gone, @DonnieBLT. The reactions only show when someone clicks on 'React'. If you click on 'React' again, they appear. Do you want them to be visible all the time instead of only when 'React' is clicked? |
|
Yes please |
2 participants
Fixes #3681
Summary by CodeRabbit