regression(iOS): image picker fails to open from message action button

[Rohit3523]

Proposed changes

Right now if we go to any channel or DM and press the + button and select "Take a Photo", "Take a Video" or "Choose from library", It fails to open the device image picker on iOS

Issue(s)

https://rocketchat.atlassian.net/browse/SUP-1012
Introduced on #6970

How to test or reproduce

1. Go to any room
2. Click on +
3. Select "Take a Photo", "Take a Video" or "Choose from library"
4. It should open the image picker and let us send image or video

Screenshots

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• Improvement (non-breaking change which improves a current function)
• New feature (non-breaking change which adds functionality)
• Documentation update (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• I have added necessary documentation (if applicable)
• Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

• Bug Fixes
  • Adjusted timing for media-related actions (photo capture, video recording, and library selection) on Android devices.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2f9b1ac5-da40-4d1a-a6ff-f917385ea7fa

📥 Commits

Reviewing files that changed from the base of the PR and between 8856b12 and 43207e2.

📒 Files selected for processing (1)

• app/containers/MessageComposer/components/Buttons/ActionsButton.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• app/containers/MessageComposer/components/Buttons/ActionsButton.tsx

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: ESLint and Test / run-eslint-and-test
• GitHub Check: format

---

Walkthrough

Adjusted the Android action-sheet workaround timing in the ActionsButton component, increasing the delay from 250ms to 550ms for media-related actions (Take a photo, Take a video, Choose from library).

Changes

Cohort / File(s)	Summary
Android Action-Sheet Timing Adjustment app/containers/MessageComposer/components/Buttons/ActionsButton.tsx	Increased setTimeout delay for media actions from 250ms to 550ms following manual action-sheet dismissal (3 occurrences).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(iOS): image picker fails to open from message action button' directly addresses the main change: a timing adjustment to fix the iOS image picker bug.
Linked Issues check	✅ Passed	The PR's timeout adjustment (250ms to 550ms) on media-related actions directly addresses SUP-1012's requirement to restore file-upload functionality on iOS.
Out of Scope Changes check	✅ Passed	All changes are scoped to the ActionsButton component's media action timeouts, directly related to fixing the iOS image picker issue outlined in SUP-1012.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

app/containers/ActionSheet/ActionSheet.tsx (1)

74-83: Consider wrapping callback invocations in try-catch.

Both snapshotOnClose and snapshotItemOnClose are user-provided callbacks that could throw. An unhandled exception would prevent subsequent callbacks from executing and could leave the component in an inconsistent state.

🛡️ Optional: Add defensive error handling

 const onDidDismiss = () => {
   setIsVisible(false);
   // Keep contentHeight to avoid flickering on next show
   const snapshotOnClose = onCloseSnapshotRef.current;
   const snapshotItemOnClose = itemOnCloseSnapshotRef.current;
   onCloseSnapshotRef.current = undefined;
   itemOnCloseSnapshotRef.current = undefined;
-  snapshotOnClose?.();
-  snapshotItemOnClose?.();
+  try {
+    snapshotOnClose?.();
+  } catch (e) {
+    console.warn('ActionSheet onClose callback error:', e);
+  }
+  try {
+    snapshotItemOnClose?.();
+  } catch (e) {
+    console.warn('ActionSheet itemOnClose callback error:', e);
+  }
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/containers/ActionSheet/ActionSheet.tsx` around lines 74 - 83, The
onDidDismiss callback currently invokes user-provided callbacks snapshotOnClose
and snapshotItemOnClose directly, which can throw and prevent the other from
running; wrap each invocation in its own try-catch (inside onDidDismiss) so
errors are caught and logged (e.g., console.error or a logger) while ensuring
onCloseSnapshotRef and itemOnCloseSnapshotRef are still cleared; reference the
onDidDismiss function and the snapshotOnClose / snapshotItemOnClose variables
(and onCloseSnapshotRef / itemOnCloseSnapshotRef) when making this change.

🤖 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/ActionSheet/ActionSheet.tsx`:
- Around line 74-83: The onDidDismiss callback currently invokes user-provided
callbacks snapshotOnClose and snapshotItemOnClose directly, which can throw and
prevent the other from running; wrap each invocation in its own try-catch
(inside onDidDismiss) so errors are caught and logged (e.g., console.error or a
logger) while ensuring onCloseSnapshotRef and itemOnCloseSnapshotRef are still
cleared; reference the onDidDismiss function and the snapshotOnClose /
snapshotItemOnClose variables (and onCloseSnapshotRef / itemOnCloseSnapshotRef)
when making this change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 007b307d-0d0f-4973-b82d-43097e2a6a7d

📥 Commits

Reviewing files that changed from the base of the PR and between 859b766 and 85dbec7.

📒 Files selected for processing (5)

• app/containers/ActionSheet/ActionSheet.tsx
• app/containers/ActionSheet/BottomSheetContent.tsx
• app/containers/ActionSheet/Item.tsx
• app/containers/ActionSheet/Provider.tsx
• app/containers/MessageComposer/components/Buttons/ActionsButton.tsx

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: E2E Build iOS / ios-build
• GitHub Check: E2E Build Android / android-build
• GitHub Check: ESLint and Test / run-eslint-and-test

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2026-03-17T19:15:30.463Z

Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.

Applied to files:

• app/containers/MessageComposer/components/Buttons/ActionsButton.tsx

🔇 Additional comments (5)

app/containers/ActionSheet/Provider.tsx (1)

15-16: Type changes look correct for the new callback pattern.

Making onPress optional and adding onClose enables items to either execute immediately on press or defer execution until after the sheet closes. This is the correct approach for fixing iOS file picker issues where the action sheet must fully dismiss before opening system pickers.

app/containers/ActionSheet/BottomSheetContent.tsx (1)

17-17: Type signature and Cancel handler are correctly updated.

The hide prop type properly accepts the optional callback parameter, and the Cancel button appropriately calls hide() without arguments since no item-specific action is needed on close.

Also applies to: 48-48

app/containers/MessageComposer/components/Buttons/ActionsButton.tsx (1)

43-78: Clean migration from setTimeout workaround to lifecycle-based callbacks.

Using onClose for media picker actions (takePhoto, takeVideo, chooseFromLibrary, chooseFile) ensures the action sheet fully dismisses before invoking iOS system pickers, which is the correct fix for the reported issue.

Note: Canned_Responses (line 46) retains onPress for immediate navigation, while createDiscussion (line 77) uses onClose. Both navigate to other screens—verify if createDiscussion specifically requires waiting for sheet dismissal or if onPress would work consistently.

app/containers/ActionSheet/ActionSheet.tsx (1)

39-44: Callback capture pattern is correct for async dismiss flow.

Storing itemOnClose in a ref before calling dismiss() ensures the callback reference survives the async animation. This correctly replaces the previous setTimeout approach with proper lifecycle handling.

app/containers/ActionSheet/Item.tsx (1)

24-34: Verify intended behavior when both onPress and onClose are defined.

The current logic executes onPress synchronously after initiating hide(), while onClose is deferred until the sheet animation completes. If an item defines both callbacks, both will execute—which may be intentional but could lead to unexpected behavior.

Consider documenting this behavior or adding mutual exclusivity if only one callback pattern should be used per item.

#!/bin/bash
# Check if any action sheet items define both onPress and onClose
rg -n 'onPress.*onClose|onClose.*onPress' --type ts --type tsx -g '!*.test.*' -g '!*.spec.*' || echo "No items with both callbacks found"

# Find all action sheet option definitions to verify usage patterns
ast-grep --pattern $'options.push({
  $$$
  onPress: $_,
  $$$
})'

[github-actions]

Android Build Available

Rocket.Chat 4.72.0.108450

Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNS_DOReG0_XT7Q3cdkUat7mExm-_5XMJTcIRL5u5thA-PsNRwFuIJ7wC6zZ49KXHjMYswTiUbaxNZH7nY-V

[github-actions]

iOS Build Available

Rocket.Chat 4.72.0.108451

[coderabbitai]

🧹 Nitpick comments (1)

app/containers/ActionSheet/Provider.tsx (1)

15-16: Tighten item typing to avoid silent no-op actions

Line 15 making onPress optional plus Line 16 adding runOnSheetClose allows enabled options with no executable handler. That can create tappable rows that do nothing. Prefer a discriminated union so actionable items always require onPress, and missing handlers are only valid for explicitly disabled items.

Proposed type-shape refactor

-export type TActionSheetOptionsItem = {
+type TActionSheetActionableItem = {
+	onPress: () => void;
+	runOnSheetClose?: boolean;
+};
+
+type TActionSheetDisabledItem = {
+	onPress?: never;
+	runOnSheetClose?: never;
+	enabled: false;
+	disabledReason?: string;
+};
+
+export type TActionSheetOptionsItem = {
 	title: string;
 	subtitle?: string;
 	accessibilityLabel?: string;
 	icon?: TIconsName;
 	danger?: boolean;
 	testID?: string;
-	onPress?: () => void;
-	runOnSheetClose?: boolean;
 	right?: () => React.ReactElement;
 	enabled?: boolean;
 	accessibilityRole?: AccessibilityRole;
 	disabledReason?: string;
-};
+} & (TActionSheetActionableItem | TActionSheetDisabledItem);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/containers/ActionSheet/Provider.tsx` around lines 15 - 16, The current
ActionSheet item shape allows enabled rows with no handler because onPress is
optional while runOnSheetClose exists; change the item type to a discriminated
union so actionable items require an onPress and disabled items cannot provide
onPress: e.g. define Item = { disabled: true; label: string; onPress?: never;
runOnSheetClose?: never } | { disabled?: false; label: string; onPress: () =>
void; runOnSheetClose?: boolean } (use the actual interface/type name in
Provider.tsx), then update any code that constructs items or calls item.onPress
(renderers, handlers) to satisfy the new types and handle the disabled branch
safely.

🤖 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/ActionSheet/Provider.tsx`:
- Around line 15-16: The current ActionSheet item shape allows enabled rows with
no handler because onPress is optional while runOnSheetClose exists; change the
item type to a discriminated union so actionable items require an onPress and
disabled items cannot provide onPress: e.g. define Item = { disabled: true;
label: string; onPress?: never; runOnSheetClose?: never } | { disabled?: false;
label: string; onPress: () => void; runOnSheetClose?: boolean } (use the actual
interface/type name in Provider.tsx), then update any code that constructs items
or calls item.onPress (renderers, handlers) to satisfy the new types and handle
the disabled branch safely.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cd4a1065-b04c-454f-8afb-b99d2213d6dc

📥 Commits

Reviewing files that changed from the base of the PR and between 5cf11fb and 8856b12.

📒 Files selected for processing (5)

• app/containers/ActionSheet/ActionSheet.tsx
• app/containers/ActionSheet/BottomSheetContent.tsx
• app/containers/ActionSheet/Item.tsx
• app/containers/ActionSheet/Provider.tsx
• app/containers/MessageComposer/components/Buttons/ActionsButton.tsx

🚧 Files skipped from review as they are similar to previous changes (4)

• app/containers/ActionSheet/BottomSheetContent.tsx
• app/containers/ActionSheet/Item.tsx
• app/containers/MessageComposer/components/Buttons/ActionsButton.tsx
• app/containers/ActionSheet/ActionSheet.tsx

📜 Review details

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

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.

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.

[unnilennium]

verified and confirmed as fixed