fix: allow retrying/deleting stuck (TEMP) messages - #7560
Conversation
* Regression: Create private team and channel as default * minor tweak
* fix multiselect itens style * remove useless brackets * remove scroll from the top of the list items
# Conflicts: # __tests__/containers/List/__snapshots__/List.stories.storyshot # __tests__/containers/SearchBox/__snapshots__/SearchBox.stories.storyshot # __tests__/containers/TextInput/__snapshots__/TextInput.stories.storyshot # android/app/build.gradle # android/build.gradle # app/containers/TextInput/TextInput.stories.tsx # app/containers/message/Audio.tsx # app/lib/methods/audioFile.ts # app/views/CreateChannelView/index.tsx # app/views/DefaultBrowserView/index.tsx # app/views/NotificationPreferencesView/index.tsx # app/views/RoomMembersView/components/ActionsSection.tsx # app/views/RoomMembersView/index.tsx # e2e/data.ts # e2e/data/data.cloud.ts # e2e/data/data.docker.ts # e2e/helpers/data_setup.ts # e2e/tests/assorted/07-changeserver.spec.ts # e2e/tests/assorted/11-deeplinking.spec.ts # e2e/tests/onboarding/05-login.spec.ts # ios/Podfile.lock # ios/RocketChatRN.xcodeproj/project.pbxproj # ios/RocketChatRN/AppDelegate.h # ios/RocketChatRN/AppDelegate.mm # ios/RocketChatRN/Info.plist # ios/ShareRocketChatRN/Info.plist # package.json # yarn.lock
Merge 4.31.0 into master
Co-authored-by: Diego Mello <diegolmello@gmail.com>
* migrate settings view to hooks * temp - add non-null assertion * fix options * Update app/views/SettingsView/index.tsx Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com> * Update app/views/SettingsView/index.tsx Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com> * fix options Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com>
* fix layout * fix quote * update snapshot
…me (RocketChat#4576) * [FIX] Touch/Face ID toggle does not enable automatically the first time * minor tweak no competition setState
* button ignore and pased the param * load room from database and check is is ignored * move handleIgnore to lib/method/helpers * Ignore and Unignore, also reactivity * block and unblock user * pass fromRid from actionView to InfoView too * remove console.log * unsubscribe subscriptionFrom * block and unblock user from dm * test to block user and ignore user * minor tweak * tweak data * minor tweak * add test before tapBack * refactor names
Co-authored-by: Gleidson Daniel Silva <gleidson10daniel@hotmail.com>
Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com>
…at#4583) * [FIX] Screen Lock's Password screen UI is broken on tablets * minor tweak and added comment * minor tweak * minor tweak * minor tweak
* fix sys messages * update snapshot * update snapshot * add username * wip * wip * change to switch and fix types * wip * new translations * update snapshot * Update app/containers/message/utils.ts Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com> * fix types and return * remove unused translation * fix types * remove unused params * minor tweak * update story Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com> Co-authored-by: Reinaldo Neto <reinaldonetof@hotmail.com>
* [FIX] UiKit/DatePicker is broken * minor tweak
Co-authored-by: Diego Mello <diegolmello@gmail.com>
* [IMPROVE] Spotlight users order * minor tweak query * minor tweak removing query and using regex * minor tweak * minor tweak, make ts happy * fix the ts * fix lint and type TSearch
* TC-110 [FIX] Cancelling when adding a server * Removing the close button when connecting to a server
* Regression: Fix styles on FormTextInput * minor tweak padding icons * added 48 height
A message stuck in the TEMP (sending) state could not be retried or deleted from the app, forcing a full app-data wipe to clear it. MessageErrorActions (resend/delete) only surfaced for ERROR status, so TEMP messages never showed the error/retry affordance. Extend the Message.hasError getter to also treat TEMP status as an error, so the retry/delete action sheet is available for stuck-sending messages. Fixes RocketChat#6830 Signed-off-by: Deepak Bhagat <deepak988088@gmail.com>
Asserts Message.hasError is true for TEMP status so stuck-sending messages expose the resend/delete action sheet. Also exports MessageContainer to allow direct unit testing of the class getters. Verified locally: the test fails against the unfixed getter (TEMP hadError was false) and passes after the fix. Signed-off-by: Deepak Bhagat <deepak988088@gmail.com>
WalkthroughThe PR updates the app version to 4.71.2, upgrades ChangesMessage handling
Gesture-handler modal roots
Media action timing
Release metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
package.jsonParsing error: Missing semicolon. (2:7) 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 |
![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
🧹 Nitpick comments (1)
app/containers/message/index.test.tsx (1)
43-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the message fixture instead of using
any.Add an explicit return type. Use the message status type expected by
MessageContainerinstead of a genericnumber. Remove the broadas anycast where possible.As per coding guidelines, TypeScript functions must have explicit parameter and return type annotations and preserve type safety.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/containers/message/index.test.tsx` around lines 43 - 52, Update the makeMessage fixture to use explicit parameter and return types, typing status with the message status type expected by MessageContainer. Replace the broad as any assertion with the appropriate message fixture type or a narrowly scoped assertion so the test remains type-safe.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/containers/message/index.test.tsx`:
- Around line 40-41: Move the MessageContainer and messagesStatus imports to the
top of the test file, before the jest.mock statement, so all imports precede
non-import code while preserving the existing expo-av mock.
In `@app/containers/MessageComposer/components/Buttons/ActionsButton.tsx`:
- Around line 57-59: Update ActionsButton’s media-action timeout logic to use a
shared platform-specific delay constant: 550 ms on Android and the original 250
ms on non-Android platforms. Apply this constant consistently to all three media
callbacks, including takePhoto.
---
Nitpick comments:
In `@app/containers/message/index.test.tsx`:
- Around line 43-52: Update the makeMessage fixture to use explicit parameter
and return types, typing status with the message status type expected by
MessageContainer. Replace the broad as any assertion with the appropriate
message fixture type or a narrowly scoped assertion so the test remains
type-safe.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e06297e3-349c-423a-9805-87bc539e62ad
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
android/app/build.gradleandroid/app/src/main/java/chat/rocket/reactnative/notification/NotificationIntentHandler.ktapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/TwoFactor/index.tsxapp/containers/message/index.test.tsxapp/containers/message/index.tsxapp/views/ChangePasscodeView.tsxapp/views/JitsiMeetView/JitsiAuthModal.tsxapp/views/RoomView/Banner.tsxapp/views/ScreenLockedView.tsxios/RocketChatRN.xcodeproj/project.pbxprojios/RocketChatRN/Info.plistios/ShareRocketChatRN/Info.plistpackage.json
💤 Files with no reviewable changes (1)
- android/app/src/main/java/chat/rocket/reactnative/notification/NotificationIntentHandler.kt
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Check: Layne Security Scan: Layne — failure
Conclusion: failure
Found 8 issue(s): 0 critical, 8 high, 0 medium, 0 low.
Blocking findings:
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-3g43-6gmg-66jw] yarn.lock:5857
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-6chq-wfr3-2hj9] yarn.lock:5857
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-hfxv-24rg-xrqf] yarn.lock:5857
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-j5f8-grm9-p9fc] yarn.lock:5857
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-p92q-9vqr-4j8v] yarn.lock:5857
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-pf86-5x62-jrwf] yarn.lock:5857
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-pjwm-pj3p-43mv] yarn.lock:5857
- LAYNE-9c5d4aa55e64c7b9 [dep-doctor/GHSA-pmwg-cvhr-8vh7] yarn.lock:5857
To approve, post a comment:
/layne exception-approve LAYNE-9c5d4aa55e64c7b9 LAYNE-9c5d4aa55e64c7b9 LAYNE-9c5d4aa55e64c7b9 LAYNE-9c5d4aa55e64c7b9 LAYNE-9c5d4aa55e64c7b9 LAYNE-9c5d4aa55e64c7b9 LAYNE-9c5d4aa55e64c7b9 LAYNE-9c5d4aa55e64c7b9 reason: <explanation>
Commit Status: security/snyk (ggazzo): security/snyk (ggazzo)
Conclusion: failure
1 test has failed
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/views/JitsiMeetView/JitsiAuthModal.tsxapp/containers/TwoFactor/index.tsxapp/containers/message/index.test.tsxapp/views/RoomView/Banner.tsxapp/views/ScreenLockedView.tsxapp/containers/message/index.tsxapp/views/ChangePasscodeView.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/views/JitsiMeetView/JitsiAuthModal.tsxapp/containers/TwoFactor/index.tsxapp/containers/message/index.test.tsxapp/views/RoomView/Banner.tsxapp/views/ScreenLockedView.tsxapp/containers/message/index.tsxapp/views/ChangePasscodeView.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Format JavaScript and TypeScript code with Oxfmt using the repository configuration: tabs, single quotes, 130-character width, no trailing commas, omitted arrow-function parentheses where possible, and same-line brackets.
Follow Oxlint rules configured in.oxlintrc.json, including the import, React, Jest, TypeScript, and React Native plugins.
Files:
app/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/views/JitsiMeetView/JitsiAuthModal.tsxapp/containers/TwoFactor/index.tsxapp/containers/message/index.test.tsxapp/views/RoomView/Banner.tsxapp/views/ScreenLockedView.tsxapp/containers/message/index.tsxapp/views/ChangePasscodeView.tsx
🧠 Learnings (6)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/views/JitsiMeetView/JitsiAuthModal.tsxapp/containers/TwoFactor/index.tsxapp/containers/message/index.test.tsxapp/views/RoomView/Banner.tsxapp/views/ScreenLockedView.tsxapp/containers/message/index.tsxapp/views/ChangePasscodeView.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/views/JitsiMeetView/JitsiAuthModal.tsxapp/containers/TwoFactor/index.tsxapp/containers/message/index.test.tsxapp/views/RoomView/Banner.tsxapp/views/ScreenLockedView.tsxapp/containers/message/index.tsxapp/views/ChangePasscodeView.tsx
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
📚 Learning: 2026-05-07T17:47:14.516Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7303
File: package.json:5-5
Timestamp: 2026-05-07T17:47:14.516Z
Learning: When reviewing pnpm `packageManager` version pins in any `package.json` (e.g., `"packageManager": "pnpm@<version>"`), don’t rely solely on web-search results to determine whether a version exists. For very recently published versions, cross-check the target version against the official pnpm release page (https://github.com/pnpm/pnpm/releases) and the npm registry page for pnpm (https://www.npmjs.com/package/pnpm) before flagging the pinned version as non-existent.
Applied to files:
package.json
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/JitsiMeetView/JitsiAuthModal.tsxapp/views/RoomView/Banner.tsxapp/views/ScreenLockedView.tsxapp/views/ChangePasscodeView.tsx
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/containers/message/index.test.tsx
🪛 ESLint
app/containers/message/index.test.tsx
[error] 40-40: Import in body of module; reorder to top.
(import/first)
[error] 41-41: Import in body of module; reorder to top.
(import/first)
🔇 Additional comments (14)
app/containers/message/index.tsx (1)
329-329: LGTM!Also applies to: 517-517
app/containers/message/index.test.tsx (2)
1-38: LGTM!
54-68: LGTM!android/app/build.gradle (1)
93-93: LGTM!ios/RocketChatRN/Info.plist (1)
31-31: LGTM!ios/ShareRocketChatRN/Info.plist (1)
29-29: LGTM!ios/RocketChatRN.xcodeproj/project.pbxproj (1)
2938-2938: LGTM!Also applies to: 2991-2991
package.json (2)
3-3: LGTM!
54-54: 🔒 Security & PrivacyAxios lockfile is consistent.
package.jsonpinsaxiosto0.30.3, andyarn.lockresolvesaxios@0.30.3with no retained entries for0.28.1,0.30.4, orplain-crypto-js.app/containers/TwoFactor/index.tsx (1)
10-10: LGTM!Also applies to: 161-161, 202-202
app/views/ChangePasscodeView.tsx (1)
6-6: LGTM!Also applies to: 16-20, 78-85
app/views/JitsiMeetView/JitsiAuthModal.tsx (1)
5-5: LGTM!Also applies to: 52-79
app/views/RoomView/Banner.tsx (1)
2-3: LGTM!Also applies to: 45-50
app/views/ScreenLockedView.tsx (1)
6-6: LGTM!Also applies to: 23-27, 82-89
| import { MessageContainer } from './index'; | ||
| import { messagesStatus } from '../../lib/constants/messagesStatus'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)app/containers/message/index.test.tsx$|(^|/)package.json$|(^|/).oxlintrc.json$|jest|babel|metro|babel.config'
echo "== test file outline/lines =="
if [ -f app/containers/message/index.test.tsx ]; then
wc -l app/containers/message/index.test.tsx
sed -n '1,120p' app/containers/message/index.test.tsx | cat -n
fi
echo "== oxlint config =="
if [ -f .oxlintrc.json ]; then
cat -n .oxlintrc.json
fi
echo "== relevant transform/mocking files =="
for f in \
package.json \
metro.config.js \
babel.config.js \
babel.config.ts \
.babelrc \
jest.config.js \
jest.config.ts \
app.config.js \
app.config.ts; do
if [ -f "$f" ]; then
echo "-- $f --"
sed -n '1,220p' "$f" | cat -n
fi
done
echo "== expo-av mock references =="
rg -n "expo-av|Audio|Sound|messagesStatus|jest.mock\\(" . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 33238
🌐 Web query:
OxRules import/first imports must be outside CommonJS require Expressions ESLint import first
💡 Result:
The ESLint rule import/first (provided by eslint-plugin-import) enforces that all import declarations appear at the top of a file, before any other non-import statements [1][2]. Regarding your query about CommonJS require expressions and imports: 1. Interaction with require(): The import/first rule generally reports import statements that follow any non-import statement, including require() calls [2][3]. If you place a require() statement (CommonJS) before an import statement (ES Modules), ESLint will flag the subsequent import as a violation because it is no longer at the "top" of the module [2]. To resolve this, you must move all import statements above any require() calls or other executable code [2][3]. 2. Handling of import = require: For TypeScript projects, the rule includes support for import = require declarations [4][5]. This syntax is treated similarly to standard import declarations, meaning these statements must also be placed at the top of the file to comply with the rule [5]. 3. Why it exists: import declarations are hoisted in JavaScript, meaning they are evaluated before any other code in the module [1][6]. Interspersing imports with other statements can lead to unexpected behavior if those statements rely on module-level identifiers that are intended to be imported, or if they rely on the evaluation order of the imports themselves [1][6]. Keeping imports at the top ensures consistent evaluation and clearer dependency management [1][2][6]. If you have a legitimate, specific use case where code must run before an import (such as certain configuration setups), you can disable the rule for that specific block or file using // eslint-disable-next-line import/first or by disabling the rule in your .eslintrc configuration [2][3]. However, this is generally discouraged in favor of reordering the code [2].
Citations:
- 1: https://github.com/import-js/eslint-plugin-import/blob/HEAD/docs/rules/first.md
- 2: https://tutorialreference.com/javascript/examples/faq/javascript-eslint-error-import-in-body-of-module-reorder-to-top-import-first
- 3: https://bobbyhadz.com/blog/import-in-body-of-module-reorder-to-top-eslint-import-first
- 4: [Fix] first's handling of import = require import-js/eslint-plugin-import#1963
- 5: import-js/eslint-plugin-import@196d655
- 6: https://simplesenseio.github.io/eslint-config-simplesense/rules/import/first
Move the imports before jest.mock.
import/first requires all imports to come before non-import statements. Move the MessageContainer and messagesStatus imports to the top of the file; keeping the in-file expo-av mock still does not satisfy the import ordering rule.
🧰 Tools
🪛 ESLint
[error] 40-40: Import in body of module; reorder to top.
(import/first)
[error] 41-41: Import in body of module; reorder to top.
(import/first)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/containers/message/index.test.tsx` around lines 40 - 41, Move the
MessageContainer and messagesStatus imports to the top of the test file, before
the jest.mock statement, so all imports precede non-import code while preserving
the existing expo-av mock.
Sources: Coding guidelines, Linters/SAST tools
| setTimeout(() => { | ||
| takePhoto(); | ||
| }, 250); | ||
| }, 550); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 '\bActionsButton\b' --glob '*.{js,jsx,ts,tsx}' .Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 6576
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l app/containers/MessageComposer/components/Buttons/ActionsButton.tsx app/containers/MessageComposer/components/Toolbar/Default.tsx app/containers/MessageComposer/components/Unfocused/Left.tsx app/containers/MessageComposer/components/Toolbar/EmojiKeyboard.tsx
printf '\n--- ActionsButton.tsx ---\n'
cat -n app/containers/MessageComposer/components/Buttons/ActionsButton.tsx
printf '\n--- Default.tsx ---\n'
cat -n app/containers/MessageComposer/components/Toolbar/Default.tsx
printf '\n--- Left.tsx ---\n'
cat -n app/containers/MessageComposer/components/Unfocused/Left.tsx
printf '\n--- EmojiKeyboard.tsx ---\n'
cat -n app/containers/MessageComposer/components/Toolbar/EmojiKeyboard.tsx
printf '\n--- Platform imports/usages near MessageComposer components ---\n'
rg -n "Platform|Platform\.OS|\\bios\\b|android|actions|media" app/containers/MessageComposer app/containers/MessageComposer.tsx --glob '*.{js,jsx,ts,tsx}' | head -200Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 10983
Keep the media-action dismissal delay Android-specific.
ActionsButton is rendered from shared MessageComposer UI, so the unconditional 550 ms delays also run on iOS. Preserve the original 250 ms behavior on non-Android platforms and define a shared platform-specific delay constant for the three media callbacks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/containers/MessageComposer/components/Buttons/ActionsButton.tsx` around
lines 57 - 59, Update ActionsButton’s media-action timeout logic to use a shared
platform-specific delay constant: 550 ms on Android and the original 250 ms on
non-Android platforms. Apply this constant consistently to all three media
callbacks, including takePhoto.
|
Closing in favor of a clean PR from fix/6830-stuck-message-retry-v2 (rebased on current develop; the old branch was cut from a year-old master and carried 348 unrelated commits / conflicts). |
13 participants
Proposed changes
A message stuck in the
TEMP(sending) state could not be retried or deleted from the app — the only way to clear it was to wipe the app data and log back in.MessageErrorActions(resend/delete) only surfaced forERRORstatus, soTEMPmessages never showed the error/retry affordance.Message.hasErroronly returnedtrueforERRORstatus. This extends it to also treatTEMPstatus as an error, so the retry/delete action sheet is available for stuck-sending messages.Issue(s)
Fixes #6830
How to test or reproduce
TEMP(sending) state (e.g. a transient send failure).Regression test added:
app/containers/message/index.test.tsxassertsMessage.hasErroristrueforTEMP(andERROR) andfalseforSENT. It fails against the unfixed getter and passes after the fix (verified by running the suite locally).Screenshots
N/A (behavior only, no visual change).
Types of changes
Checklist
Further comments
The reported 4.66 -> 4.67 regression (messages getting stuck) could not be reproduced by maintainers and no server/client logs were provided, so the root send failure is not addressed here. This change fixes the concrete, reproducible defect that makes a stuck message unrecoverable: it now always offers Retry / Delete, which both work regardless of the original send error.
Summary by CodeRabbit