fix: image preview is not showing in share view#7271
Conversation
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated sizing strategy in ImageViewer: the outer container uses Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/containers/ImageViewer/ImageViewer.tsx (1)
125-131: Harden async playback toggle with explicit error handling.Line 130 dispatches
handleGifPlaybackas fire-and-forget; ifstartAnimating/stopAnimatingrejects, the error is currently unhandled.Proposed refactor
- const handleGifPlayback = async () => { - if (isPlaying) { - setIsPlaying(false); - await expoImageRef.current?.stopAnimating(); - return; - } - setIsPlaying(true); - await expoImageRef.current?.startAnimating(); - }; + const handleGifPlayback = async (): Promise<void> => { + try { + if (isPlaying) { + await expoImageRef.current?.stopAnimating(); + setIsPlaying(false); + return; + } + await expoImageRef.current?.startAnimating(); + setIsPlaying(true); + } catch { + // Keep UI stable if native animation call fails. + } + };As per coding guidelines: "Use explicit error handling with try/catch blocks for async operations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/ImageViewer/ImageViewer.tsx` around lines 125 - 131, The onEnd handler currently calls scheduleOnRN(handleGifPlayback) as fire-and-forget, so any rejection from handleGifPlayback (and its calls to startAnimating/stopAnimating) is unhandled; change the onEnd to pass an async wrapper to scheduleOnRN that awaits handleGifPlayback inside a try/catch and logs or reports the error (e.g., use an available logger or console.error) to avoid swallowing promise rejections — update the singleTapGesture.onEnd callback that references scheduleOnRN and handleGifPlayback accordingly so failures from startAnimating/stopAnimating are explicitly caught and handled.
🤖 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/ImageViewer/ImageViewer.tsx`:
- Line 6: Tests fail because react-native-worklets (imported as scheduleOnRN in
ImageViewer.tsx) has no built-in Jest mock; add a manual mock in your Jest setup
by updating jest.setup.js to mock the module 'react-native-worklets' so
scheduleOnRN is a jest.fn that synchronously invokes the passed function and
returns its result, ensuring all uses of scheduleOnRN in ImageViewer and
elsewhere work in tests.
---
Nitpick comments:
In `@app/containers/ImageViewer/ImageViewer.tsx`:
- Around line 125-131: The onEnd handler currently calls
scheduleOnRN(handleGifPlayback) as fire-and-forget, so any rejection from
handleGifPlayback (and its calls to startAnimating/stopAnimating) is unhandled;
change the onEnd to pass an async wrapper to scheduleOnRN that awaits
handleGifPlayback inside a try/catch and logs or reports the error (e.g., use an
available logger or console.error) to avoid swallowing promise rejections —
update the singleTapGesture.onEnd callback that references scheduleOnRN and
handleGifPlayback accordingly so failures from startAnimating/stopAnimating are
explicitly caught and handled.
🪄 Autofix (Beta)
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
Run ID: c6fce1d8-f2ba-4a0d-bb67-de322aa734f0
📒 Files selected for processing (1)
app/containers/ImageViewer/ImageViewer.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). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/ImageViewer/ImageViewer.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
**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms
Files:
app/containers/ImageViewer/ImageViewer.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/containers/ImageViewer/ImageViewer.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Create reusable UI components in app/containers/ directory
Files:
app/containers/ImageViewer/ImageViewer.tsx
🧠 Learnings (2)
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.
Applied to files:
app/containers/ImageViewer/ImageViewer.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Use React 19 with React Native 0.79 and Expo 53
Applied to files:
app/containers/ImageViewer/ImageViewer.tsx
🔇 Additional comments (1)
app/containers/ImageViewer/ImageViewer.tsx (1)
133-134: Good gesture arbitration withGesture.Exclusive.Line 133 cleanly prevents single-tap playback toggles from firing when a double-tap zoom gesture is recognized, while Line 134 preserves pinch/pan simultaneity.
| import { Gesture, GestureDetector } from 'react-native-gesture-handler'; | ||
| import Animated, { withTiming, useSharedValue, useAnimatedStyle, withSpring } from 'react-native-reanimated'; | ||
| import { Image, type ImageStyle } from 'expo-image'; | ||
| import { scheduleOnRN } from 'react-native-worklets'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching Jest setup files for react-native-worklets mock..."
fd -i 'jest\.setup\.(js|ts)$' | while read -r file; do
echo "== $file =="
rg -n "react-native-worklets|scheduleOnRN|jest\\.mock\\(" "$file" || true
done
echo
echo "Searching repository for any manual mock references..."
rg -n "jest\\.mock\\('react-native-worklets'|scheduleOnRN" -SRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 3999
Add Jest mock for react-native-worklets in jest.setup.js
Line 6 imports scheduleOnRN from react-native-worklets, which is used throughout the codebase. This library version does not provide a built-in Jest mock, so tests will fail without a manual mock setup. Add the following to jest.setup.js:
jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/containers/ImageViewer/ImageViewer.tsx` at line 6, Tests fail because
react-native-worklets (imported as scheduleOnRN in ImageViewer.tsx) has no
built-in Jest mock; add a manual mock in your Jest setup by updating
jest.setup.js to mock the module 'react-native-worklets' so scheduleOnRN is a
jest.fn that synchronously invokes the passed function and returns its result,
ensuring all uses of scheduleOnRN in ImageViewer and elsewhere work in tests.
OtavioStasiak
left a comment
OtavioStasiak
left a comment
There was a problem hiding this comment.
This isn’t really the core issue... it’s more of a nitpick.
In expo-image v3 on Fabric, flex: 1 gets resolved to flex-basis: 0%. That causes the native image to collapse to zero size before its intrinsic dimensions are calculated, which is why it renders blank.
As a quick workaround, try replacing flex: 1 on the image with:
{ width: '100%', height: '100%' }
Also, update the Touch styles like this:
This should at least give you a solid starting point to track down the actual fix.
Rohit3523
force-pushed
the
image-share-preview-fix
branch
from
8acf9a2 to
7948d70
Compare
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/containers/ImageViewer/ImageViewer.tsx (1)
24-27: Renamestyles.flexto reflect actual behavior.
flexis now a full-size style, not a flex style. Renaming improves readability and reduces confusion in future edits.♻️ Suggested refactor
const styles = StyleSheet.create({ - flex: { + fullSize: { width: '100%', height: '100%' }, image: { flex: 1 } }); @@ - <View style={[styles.flex, { width, height, backgroundColor: colors.surfaceNeutral }]}> + <View style={[styles.fullSize, { width, height, backgroundColor: colors.surfaceNeutral }]}> <GestureDetector gesture={gesture}> - <Animated.View onLayout={onLayout} style={[styles.flex, style]}> - <Touch onPress={handleGifPlayback} style={styles.flex} rectButtonStyle={styles.flex}> + <Animated.View onLayout={onLayout} style={[styles.fullSize, style]}> + <Touch onPress={handleGifPlayback} style={styles.fullSize} rectButtonStyle={styles.fullSize}>As per coding guidelines, "
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/ImageViewer/ImageViewer.tsx` around lines 24 - 27, Rename the misleading styles.flex in ImageViewer.tsx to a descriptive name like styles.fullSize (or styles.fullSizeContainer) and update every usage within the file (e.g., JSX elements referencing styles.flex) to the new identifier; adjust the const styles declaration (the object containing flex: { width: '100%', height: '100%' }) to use the new key and run a quick search in ImageViewer.tsx for "styles.flex" to replace all references so behavior is unchanged but naming is clear.
🤖 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/ImageViewer/ImageViewer.tsx`:
- Around line 24-27: Rename the misleading styles.flex in ImageViewer.tsx to a
descriptive name like styles.fullSize (or styles.fullSizeContainer) and update
every usage within the file (e.g., JSX elements referencing styles.flex) to the
new identifier; adjust the const styles declaration (the object containing flex:
{ width: '100%', height: '100%' }) to use the new key and run a quick search in
ImageViewer.tsx for "styles.flex" to replace all references so behavior is
unchanged but naming is clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66d21535-456a-499f-bfbc-f74a1bee2a07
📒 Files selected for processing (1)
app/containers/ImageViewer/ImageViewer.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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/ImageViewer/ImageViewer.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
**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms
Files:
app/containers/ImageViewer/ImageViewer.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/containers/ImageViewer/ImageViewer.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Create reusable UI components in app/containers/ directory
Files:
app/containers/ImageViewer/ImageViewer.tsx
🧠 Learnings (3)
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.
Applied to files:
app/containers/ImageViewer/ImageViewer.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use ESLint with rocket.chat/eslint-config base including React, React Native, TypeScript, and Jest plugins
Applied to files:
app/containers/ImageViewer/ImageViewer.tsx
📚 Learning: 2026-04-04T21:34:30.268Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6808
File: app/containers/MessageComposer/components/ComposerInput.tsx:337-341
Timestamp: 2026-04-04T21:34:30.268Z
Learning: In Rocket.Chat React Native, the markdown composer's autocomplete insertion (ComposerInput.tsx onAutocompleteItemSelected) does NOT need to add a space between an underscore italic delimiter `_` and a `@` or `#` mention sigil. The web platform (using the same rocket.chat/message-parser) does not add such a space either, so parity with web is the correct behavior. The previous learning about "space between `_` and mention sigil" applies only to test/story file content strings, not to the composer's runtime autocomplete behavior.
Applied to files:
app/containers/ImageViewer/ImageViewer.tsx
🔇 Additional comments (1)
app/containers/ImageViewer/ImageViewer.tsx (1)
25-26: Good layout fix for preview rendering.Using explicit
width: '100%'andheight: '100%'here makes the nested wrapper fill behavior deterministic and matches the reported share-view preview issue.
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
official_android_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
2 participants
Proposed changes
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-2149
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit