feat: add optimistic UI updates for message reactions#7025
feat: add optimistic UI updates for message reactions#7025divyanshu-patil wants to merge 10 commits intoRocketChat:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughImplements optimistic reaction handling in the message container with client-side Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as MessageContainer
participant Server as ServerAPI
User->>UI: Press reaction button
Note over UI: Optimistically update proxyReactions and re-render
UI->>Server: Send reaction request (await Promise<boolean>)
alt Server Success
Server-->>UI: Success
UI-->>User: Keep optimistic state, return true
else Server Failure
Server-->>UI: Error
UI->>UI: Show Alert (i18n "Error"/"Reaction_Failed")
UI->>UI: Rollback proxyReactions to server state and re-render
UI-->>User: Return false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/i18n/locales/pt-BR.json (1)
674-674: Prefer clearer failure wording for reaction errors.
"Não reagiu"sounds like a state description. For an error toast,"Não foi possível reagir"is clearer.💬 Suggested wording tweak
- "Reaction_Failed": "Não reagiu", + "Reaction_Failed": "Não foi possível reagir",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/i18n/locales/pt-BR.json` at line 674, Update the translation value for the key "Reaction_Failed" in app/i18n/locales/pt-BR.json from "Não reagiu" to the clearer error phrasing "Não foi possível reagir" so the toast reads as a failure message rather than a state.app/containers/message/index.tsx (1)
154-158: Remove the duplicate auto-translate change check.
autoTranslateRoomis evaluated twice; keep only the combined condition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/message/index.tsx` around lines 154 - 158, The component currently checks autoTranslateRoom twice; remove the first standalone check and keep the combined conditional that returns true when either nextProps.autoTranslateRoom !== autoTranslateRoom or nextProps.autoTranslateLanguage !== autoTranslateLanguage (i.e., eliminate the duplicate if that only checks autoTranslateRoom), ensuring the method (e.g., shouldComponentUpdate or equivalent using nextProps/autoTranslateRoom/autoTranslateLanguage) only contains the single combined condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/containers/message/index.tsx`:
- Around line 270-275: The code currently may call onReactionPress when it's
undefined and doesn't catch promise rejections; wrap the call to onReactionPress
in a guarded async flow: check if onReactionPress exists and only call it if
defined (treat missing handler as a successful no-op), otherwise skip calling
it; surround the await onReactionPress(emoji, item.id) with try/catch to catch
rejections and in the catch block call Alert.alert(i18n.t('Error'),
i18n.t('Reaction_Failed')) and rollback by calling this.setState({
proxyReactions: undefined }); ensure the success check uses the resolved value
from onReactionPress before skipping rollback so proxyReactions is only cleared
on actual failure or exception.
- Around line 239-254: The optimistic-update block currently mutates
updated[index].usernames but leaves updated[index].names unchanged; update both
in lockstep by zipping usernames with names, filtering out the entry where
username === username (for removes) and unzipping back to set both
updated[index].usernames and updated[index].names, and when adding push the
corresponding display name (use the available displayName variable or fall back
to username) so updated[index].usernames and updated[index].names remain
synchronized.
---
Nitpick comments:
In `@app/containers/message/index.tsx`:
- Around line 154-158: The component currently checks autoTranslateRoom twice;
remove the first standalone check and keep the combined conditional that returns
true when either nextProps.autoTranslateRoom !== autoTranslateRoom or
nextProps.autoTranslateLanguage !== autoTranslateLanguage (i.e., eliminate the
duplicate if that only checks autoTranslateRoom), ensuring the method (e.g.,
shouldComponentUpdate or equivalent using
nextProps/autoTranslateRoom/autoTranslateLanguage) only contains the single
combined condition.
In `@app/i18n/locales/pt-BR.json`:
- Line 674: Update the translation value for the key "Reaction_Failed" in
app/i18n/locales/pt-BR.json from "Não reagiu" to the clearer error phrasing "Não
foi possível reagir" so the toast reads as a failure message rather than a
state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/containers/message/index.tsxapp/i18n/locales/en.jsonapp/i18n/locales/es.jsonapp/i18n/locales/pt-BR.jsonapp/views/RoomView/index.tsx
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
app/containers/message/index.tsx (1)
app/definitions/IReaction.ts (1)
IReaction(1-6)
🔇 Additional comments (4)
app/i18n/locales/pt-BR.json (1)
321-321: Looks good for the generic error title.The new
"Error"key is consistent with the new reaction failure alert flow.app/i18n/locales/es.json (1)
156-156: Localization additions are aligned and clear.Both new keys map cleanly to the new reaction error alert behavior.
Also applies to: 325-325
app/i18n/locales/en.json (1)
330-330: English keys and copy are good.These additions are consistent with the new reaction failure UX path.
Also applies to: 686-686
app/views/RoomView/index.tsx (1)
896-912: Boolean success contract is implemented correctly.Returning
true/falsehere cleanly supports optimistic UI reconciliation in the message container.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/containers/message/index.tsx (2)
241-259:⚠️ Potential issue | 🟠 MajorKeep
namessynchronized withusernamesin optimistic updates.When modifying existing reactions, only
usernamesis updated whilenamesremains stale. PerIReactioninterface, both arrays should stay in sync.
- Line 243: Removing a user filters
usernamesbut notnames- Line 257: Adding a user appends to
usernamesbut notnames🔧 Suggested fix to keep arrays synchronized
if (alreadyReacted) { // remove - const newUsers = updated[index].usernames.filter(u => u !== username); + const currentReaction = updated[index]; + const userIdx = currentReaction.usernames.indexOf(username); + const newUsers = currentReaction.usernames.filter((_, i) => i !== userIdx); + const newNames = currentReaction.names.filter((_, i) => i !== userIdx); if (newUsers.length === 0) { updated.splice(index, 1); } else { updated[index] = { - ...updated[index], - usernames: newUsers + ...currentReaction, + usernames: newUsers, + names: newNames }; } } else { // add + const currentReaction = updated[index]; updated[index] = { - ...updated[index], - usernames: [...updated[index].usernames, username] + ...currentReaction, + usernames: [...currentReaction.usernames, username], + names: [...currentReaction.names, username] }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/message/index.tsx` around lines 241 - 259, The optimistic update mutates reaction entries in the updated array (variable updated, at index variable index) but only updates the usernames field, leaving the names field out of sync with the IReaction shape; when removing (alreadyReacted true) or adding a username (else branch) also update the corresponding names array in the same manner (filter/remove or append) so both usernames and names remain synchronized for the reaction object (updated[index]); ensure you spread the existing reaction and set both usernames and names consistently.
274-279:⚠️ Potential issue | 🟠 MajorWrap server call in try-catch to handle rejected promises.
If
onReactionPressthrows or rejects, the error propagates unhandled and rollback never executes. The guard at line 228 makes the optional chaining on line 274 redundant.🛠️ Suggested fix for robust error handling
// still call server - -const success = await onReactionPress?.(emoji, item.id); -if (!success) { +let success = false; +try { + success = await onReactionPress(emoji, item.id); +} catch { + success = false; +} +if (!success) { Alert.alert(i18n.t('Error'), i18n.t('Reaction_Failed')); // rollback on failure this.setState({ proxyReactions: undefined }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/message/index.tsx` around lines 274 - 279, The await call to onReactionPress in the reaction handler can throw/reject and currently isn't caught, so wrap the call to onReactionPress (from the reaction handler where proxyReactions is set) in a try-catch; in the try await onReactionPress(emoji, item.id) and treat any thrown error as a failed result, and in the catch call Alert.alert(i18n.t('Error'), i18n.t('Reaction_Failed')) and rollback state with this.setState({ proxyReactions: undefined }); also remove the redundant optional chaining on onReactionPress (since the earlier guard ensures it exists) or otherwise treat a missing callback as a failure so rollback always runs.
🧹 Nitpick comments (1)
app/containers/message/index.tsx (1)
260-267: Consider edge case: rapid reaction toggles may cause ID collision.Using
Date.now()for the temporary_idcould collide if the same emoji is toggled multiple times within the same millisecond. While unlikely in practice and the server will reconcile, a more robust approach would use a counter or random suffix.💡 Optional: Use crypto for unique ID
updated.push({ - _id: `${emoji}-${Date.now()}`, + _id: `${emoji}-${Date.now()}-${Math.random().toString(36).slice(2, 9)}`, emoji, usernames: [username], names: [username] });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/message/index.tsx` around lines 260 - 267, The temporary reaction _id generation using `${emoji}-${Date.now()}` can collide on rapid toggles; update the creation in the block that pushes into updated (the updated array where _id, emoji, usernames, names are set) to use a stronger unique suffix instead of only Date.now() — e.g., use crypto.randomUUID() when available or append a short random string or an incrementing counter (reactionIdCounter) to `${emoji}-${Date.now()}` so the new _id is guaranteed unique across rapid toggles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/containers/message/index.tsx`:
- Around line 241-259: The optimistic update mutates reaction entries in the
updated array (variable updated, at index variable index) but only updates the
usernames field, leaving the names field out of sync with the IReaction shape;
when removing (alreadyReacted true) or adding a username (else branch) also
update the corresponding names array in the same manner (filter/remove or
append) so both usernames and names remain synchronized for the reaction object
(updated[index]); ensure you spread the existing reaction and set both usernames
and names consistently.
- Around line 274-279: The await call to onReactionPress in the reaction handler
can throw/reject and currently isn't caught, so wrap the call to onReactionPress
(from the reaction handler where proxyReactions is set) in a try-catch; in the
try await onReactionPress(emoji, item.id) and treat any thrown error as a failed
result, and in the catch call Alert.alert(i18n.t('Error'),
i18n.t('Reaction_Failed')) and rollback state with this.setState({
proxyReactions: undefined }); also remove the redundant optional chaining on
onReactionPress (since the earlier guard ensures it exists) or otherwise treat a
missing callback as a failure so rollback always runs.
---
Nitpick comments:
In `@app/containers/message/index.tsx`:
- Around line 260-267: The temporary reaction _id generation using
`${emoji}-${Date.now()}` can collide on rapid toggles; update the creation in
the block that pushes into updated (the updated array where _id, emoji,
usernames, names are set) to use a stronger unique suffix instead of only
Date.now() — e.g., use crypto.randomUUID() when available or append a short
random string or an incrementing counter (reactionIdCounter) to
`${emoji}-${Date.now()}` so the new _id is guaranteed unique across rapid
toggles.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/containers/message/index.tsx
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
app/containers/message/index.tsx (1)
app/definitions/IReaction.ts (1)
IReaction(1-6)
🔇 Additional comments (5)
app/containers/message/index.tsx (5)
1-15: LGTM!The new imports (
Alert,IReaction,i18n) are appropriate for implementing optimistic UI updates with error feedback.
69-73: LGTM!The
proxyReactionsstate design is clean — usingundefinedto signal "use server value" is a clear convention for optimistic updates.
100-102: LGTM!Resetting
proxyReactionstoundefinedon subscription updates correctly reconciles optimistic state with server truth.
122-129: LGTM!The
shouldComponentUpdateadditions correctly trigger re-renders for both optimistic (proxyReactions) and server-side (item.reactions) reaction changes.
482-484: LGTM!The nullish coalescing pattern cleanly prioritizes optimistic state while falling back to server truth when
proxyReactionsisundefined.
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 `@app/containers/message/index.tsx`:
- Around line 100-102: The optimistic proxyReactions state can remain set when
experimentalSubscribe is unavailable (this.subscription undefined) because the
only cleanup currently happens in the subscription callback or on failure;
update the reaction success path (the code that handles the server response
after sending a reaction) to explicitly clear this.state.proxyReactions when
this.subscription is falsy. Concretely, in the function that performs the
reaction request (the same block that currently clears proxyReactions on failure
around the reaction send/response logic), add a branch after the successful
await/response that sets proxyReactions to undefined (or calls the same cleanup
used by the experimentalSubscribe callback), ensuring the UI render code that
reads proxyReactions uses fresh server state even if experimentalSubscribe is
not present.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/containers/message/index.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-05T13:55:06.688Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:06.688Z
Learning: The RocketChat/Rocket.Chat.ReactNative repository uses a fork of react-native-image-crop-picker (RocketChat/react-native-image-crop-picker) with custom Android edge-to-edge fixes, not the upstream ivpusic/react-native-image-crop-picker package. Dependencies should reference commit pins from the RocketChat fork.
Applied to files:
app/containers/message/index.tsx
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/containers/message/index.tsx
🧬 Code graph analysis (1)
app/containers/message/index.tsx (1)
app/definitions/IReaction.ts (1)
IReaction(1-6)
🔇 Additional comments (1)
app/containers/message/index.tsx (1)
228-230: Good hardening of optimistic reaction mutation path.Guarding
onReactionPressand keepingusernames+namessynchronized in add/remove flows looks correct and resolves the prior consistency pitfall.Also applies to: 243-254, 259-263
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/containers/message/index.tsx (1)
232-275: Consider extracting optimistic reaction patching into a pure helper.This block is getting dense; moving the add/remove reconciliation into a small pure function would make it easier to unit test and safer to evolve.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/message/index.tsx` around lines 232 - 275, Extract the optimistic add/remove reaction logic into a pure helper (e.g., reconcileReactions) that accepts (currentReactions, emoji, username) and returns a new reactions array without mutating inputs; update the setState call to compute updated = reconcileReactions(prev.proxyReactions ?? item.reactions ?? [], emoji, username) and return { proxyReactions: updated }. The helper should encapsulate the existing behavior: find by emoji, add username/name arrays when missing, remove username/name and drop the reaction when empty, and create a new reaction object with a generated _id (e.g., `${emoji}-${Date.now()}`) when pushing a new reaction. Make the helper pure and export it so it can be unit tested independently and ensure it preserves order and the parallel usernames/names mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/containers/message/index.tsx`:
- Around line 232-275: Extract the optimistic add/remove reaction logic into a
pure helper (e.g., reconcileReactions) that accepts (currentReactions, emoji,
username) and returns a new reactions array without mutating inputs; update the
setState call to compute updated = reconcileReactions(prev.proxyReactions ??
item.reactions ?? [], emoji, username) and return { proxyReactions: updated }.
The helper should encapsulate the existing behavior: find by emoji, add
username/name arrays when missing, remove username/name and drop the reaction
when empty, and create a new reaction object with a generated _id (e.g.,
`${emoji}-${Date.now()}`) when pushing a new reaction. Make the helper pure and
export it so it can be unit tested independently and ensure it preserves order
and the parallel usernames/names mapping.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/containers/message/index.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-05T13:55:06.688Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:06.688Z
Learning: The RocketChat/Rocket.Chat.ReactNative repository uses a fork of react-native-image-crop-picker (RocketChat/react-native-image-crop-picker) with custom Android edge-to-edge fixes, not the upstream ivpusic/react-native-image-crop-picker package. Dependencies should reference commit pins from the RocketChat fork.
Applied to files:
app/containers/message/index.tsx
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/containers/message/index.tsx
🧬 Code graph analysis (1)
app/containers/message/index.tsx (1)
app/definitions/IReaction.ts (1)
IReaction(1-6)
🔇 Additional comments (1)
app/containers/message/index.tsx (1)
224-290: Optimistic reaction flow + fallback handling looks good.The local optimistic update, rollback on failure, and explicit cleanup when no subscription is available are all solid and aligned with the PR objective.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/containers/message/index.tsx (1)
226-239:⚠️ Potential issue | 🟠 MajorKeep
namesaligned with existingnamesdata, not withusernames.Line 228 and Line 238 rebuild
namesfrom usernames, which can overwrite existing display-name values and corrupt reaction metadata during optimistic updates.🧩 Suggested fix
private removeUserFromReaction = (reaction: IReaction, username: string): IReaction | null => { - const usernames = reaction.usernames.filter(u => u !== username); - const names = usernames; + const pairs = reaction.usernames.map((u, i) => ({ username: u, name: reaction.names[i] ?? u })); + const filtered = pairs.filter(p => p.username !== username); - if (usernames.length === 0) return null; // remove entirely + if (filtered.length === 0) return null; // remove entirely - return { ...reaction, usernames, names }; + return { + ...reaction, + usernames: filtered.map(p => p.username), + names: filtered.map(p => p.name) + }; }; private addUserToReaction = (reaction: IReaction, username: string): IReaction => ({ ...reaction, usernames: [...reaction.usernames, username], - names: [...reaction.usernames, username] + names: [...reaction.names, username] });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/message/index.tsx` around lines 226 - 239, The code currently rebuilds names from usernames in removeUserFromReaction and addUserToReaction which overwrites display-name metadata; instead, operate on reaction.names in parallel with reaction.usernames: in removeUserFromReaction find the index(es) of username in reaction.usernames, remove the matching entry from both reaction.usernames and reaction.names (e.g., filter by index or splice), and return the updated reaction (or null if empty); in addUserToReaction append the username to reaction.usernames and append the corresponding display name to reaction.names (use the provided display name if available, otherwise append the username) so names remain aligned with existing names data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/containers/message/index.tsx`:
- Around line 226-239: The code currently rebuilds names from usernames in
removeUserFromReaction and addUserToReaction which overwrites display-name
metadata; instead, operate on reaction.names in parallel with
reaction.usernames: in removeUserFromReaction find the index(es) of username in
reaction.usernames, remove the matching entry from both reaction.usernames and
reaction.names (e.g., filter by index or splice), and return the updated
reaction (or null if empty); in addUserToReaction append the username to
reaction.usernames and append the corresponding display name to reaction.names
(use the provided display name if available, otherwise append the username) so
names remain aligned with existing names data.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/containers/message/index.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-05T13:55:06.688Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:06.688Z
Learning: The RocketChat/Rocket.Chat.ReactNative repository uses a fork of react-native-image-crop-picker (RocketChat/react-native-image-crop-picker) with custom Android edge-to-edge fixes, not the upstream ivpusic/react-native-image-crop-picker package. Dependencies should reference commit pins from the RocketChat fork.
Applied to files:
app/containers/message/index.tsx
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/containers/message/index.tsx
🧬 Code graph analysis (1)
app/containers/message/index.tsx (1)
app/definitions/IReaction.ts (1)
IReaction(1-6)
🔇 Additional comments (1)
app/containers/message/index.tsx (1)
293-297: Good fallback cleanup when live subscription is unavailable.Line 296 correctly prevents stale optimistic state by resetting
proxyReactionsafter a successful server update when noexperimentalSubscribehook exists.
Proposed changes
This PR introduces optimistic UI updates for message reactions, while preserving server-side consistency and adding a safe fallback mechanism.
Issue(s)
How to test or reproduce
Screenshots
before
optimistic.ui.before.mp4
after
optimistic.ui.after.mp4
Types of changes
Checklist
Further comments
Summary by CodeRabbit