Translate summary, AI image boosts and more#3151
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated an SDK dependency and introduced several features: 3speak thumbnail handling, thumbnail-driven video sizing, summary translation UI, deeplink-based auth-transfer flow (with account switching), AI image "power" selection, and username suggestions on the transfer screen. Changes
Sequence Diagram(s)sequenceDiagram
actor Deeplink
participant LinkProcessor as "useLinkProcessor"
participant User as "User (Alert)"
participant Auth as "auth.loginWithAuthTransfer"
participant Storage as "setUserData / setSCAccount"
participant Navigation as "Navigator"
participant Toast as "Toast"
Deeplink->>LinkProcessor: receive ecency:auth-transfer?username=&accessToken=...
LinkProcessor->>User: show confirmation Alert
User-->>LinkProcessor: confirm
LinkProcessor->>Auth: loginWithAuthTransfer(username, accessToken, refreshToken, expiresIn)
Auth->>Auth: fetchAccount(username)
Auth->>Storage: setUserData(...), setSCAccount(...)
Auth-->>LinkProcessor: return account
LinkProcessor->>Navigation: navigate to PINCODE or MAIN_DRAWER
LinkProcessor->>Toast: show success toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/videoPlayer/videoPlayerView.tsx (1)
221-259:⚠️ Potential issue | 🟠 MajorEscape the URI to prevent XSS via malformed embed URLs.
The
uriparameter is directly interpolated into an HTML attribute without escaping. If a URI contains special characters like"or</iframe>, it can break out of thesrcattribute and execute arbitrary JavaScript in the WebView context instead of being confined to the iframe.Suggested fix
- const htmlIframeVideoPlayer = (_uri: string) => - `<!DOCTYPE html> + const htmlIframeVideoPlayer = (_uri: string) => { + const escapedUri = _uri + .replace(/&/g, '&') + .replace(/"/g, '"') + .replace(/</g, '<') + .replace(/>/g, '>'); + + return `<!DOCTYPE html> <html> <head> <meta name="viewport" content="width=device-width,initial-scale=1.0,maximum-scale=1.0" /> @@ <body> - <iframe src="${_uri}" + <iframe src="${escapedUri}" allow="autoplay; accelerometer; encrypted-media; gyroscope; picture-in-picture; web-share; fullscreen" allowfullscreen></iframe> @@ </body> -</html>`; +</html>`; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/videoPlayer/videoPlayerView.tsx` around lines 221 - 259, The htmlIframeVideoPlayer function currently interpolates _uri directly into the iframe src, allowing XSS if the URI contains quotes or HTML that can break out of the attribute; change htmlIframeVideoPlayer to sanitize/escape the URI before embedding (e.g., validate scheme, then percent-encode or explicitly escape characters like ", <, >, and & or pass the value through a tested sanitizer) so the resulting string cannot break out of the src attribute or inject HTML/JS; ensure the sanitized value is used in the template and add a small comment in htmlIframeVideoPlayer noting the escaping step.
🧹 Nitpick comments (3)
src/components/postView/children/postReadingMetadata.tsx (1)
121-140: LGTM on the translation effect with cancellation pattern.Good use of the cancellation flag to prevent state updates after unmount or when dependencies change. However, the empty catch block at line 133 silently swallows translation errors.
Consider logging translation errors
.catch(() => {}) + .catch((err) => console.warn('Translation failed:', err))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/postView/children/postReadingMetadata.tsx` around lines 121 - 140, The empty catch in the useEffect translation effect silently swallows errors from getTranslation; update the catch to log the error (e.g., console.error or your app logger) and optionally set an error state so UI can reflect failures. Locate the useEffect in postReadingMetadata.tsx that calls getTranslation(summary, 'auto', targetLang.code) and modify the .catch(() => {}) to .catch((err) => { /* log err and/or set translation error state, ensure not updating state if cancelled */ }) while preserving the cancelled checks and the setIsTranslating(false) in finally.src/hooks/useLinkProcessor.tsx (1)
463-463: Add radix to parseInt for safety.While
parseIntwill typically infer base 10 for numeric strings, explicitly specifying the radix is a best practice to avoid potential edge cases.Suggested fix
- const expiresIn = parseInt(params.get('expiresIn') || '86400', 10); + const expiresIn = Number.parseInt(params.get('expiresIn') || '86400', 10);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLinkProcessor.tsx` at line 463, The parseInt call that parses expiresIn in useLinkProcessor (const expiresIn = parseInt(params.get('expiresIn') || '86400', 10);) should explicitly pass a radix to avoid ambiguity; update the parseInt invocation in useLinkProcessor.tsx to include the radix 10 as the second argument (i.e., parseInt(..., 10)) while keeping the existing fallback '86400' logic intact.src/components/videoPlayer/videoPlayerView.tsx (1)
2-8: Keep this touched component onEStyleSheet.This file still imports
StyleSheet; please swap the component over to
react-native-extended-stylesheetand updateStyleSheet.create(...)below to match the repo
convention.As per coding guidelines
src/{screens,components,containers}/**/*.{ts,tsx}: Usereact-native-extended-stylesheet(imported as EStyleSheet) for styling components🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/videoPlayer/videoPlayerView.tsx` around lines 2 - 8, The file imports StyleSheet from 'react-native' but the repo convention requires using EStyleSheet from 'react-native-extended-stylesheet'; replace the StyleSheet import with import EStyleSheet (keep other imports like View, ActivityIndicator, useWindowDimensions, Image as RNImage), then update the style creation call from StyleSheet.create(...) to EStyleSheet.create(...) and ensure any style usages in the component (e.g., container, video, loader names) remain the same so only the import and create call change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/postView/children/postReadingMetadata.tsx`:
- Around line 100-119: The useEffect that loads languages (the block using
useEffect, showTranslate, languages, fetchSupportedLangs, setLanguages, appLang,
targetLang, setTargetLang) has missing dependencies and swallows errors; update
the dependency array to include languages (or languages.length), appLang, and
targetLang so the auto-selection logic sees current values and avoid stale
closures, and handle promise rejection in the .catch by logging the error (e.g.,
console.error or process logger) instead of an empty handler; ensure this change
still guards re-fetching by keeping the existing languages.length === 0 check to
prevent loops.
In `@src/components/videoPlayer/videoPlayerView.tsx`:
- Around line 56-80: The RNImage.getSize callback can be invoked after
dependencies change causing stale updates; inside the useEffect(for uri,
playerWidth, thumbnailUrl) introduce an isActive boolean flag (e.g., let
isActive = true) and in the success callback check isActive before calling
setPlayerHeight, and set isActive = false in the useEffect cleanup to prevent
stale callbacks from overwriting playerHeight; keep the existing logic using
RNImage.getSize, but guard both success and error callbacks with the isActive
check so only the latest effect can update via setPlayerHeight.
In `@src/hooks/useLinkProcessor.tsx`:
- Line 474: The intl.formatMessage call is using the missing translation id
'qr.auth_transfer_confirm'; add a new key "auth_transfer_confirm" inside the
"qr" object in en-US.json (matching the usage of intl.formatMessage({ id:
'qr.auth_transfer_confirm' }, { username })) and provide an English message that
includes the {username} interpolation (for example: "Confirm transfer to
{username}?") so the formatted string renders correctly instead of the raw key.
---
Outside diff comments:
In `@src/components/videoPlayer/videoPlayerView.tsx`:
- Around line 221-259: The htmlIframeVideoPlayer function currently interpolates
_uri directly into the iframe src, allowing XSS if the URI contains quotes or
HTML that can break out of the attribute; change htmlIframeVideoPlayer to
sanitize/escape the URI before embedding (e.g., validate scheme, then
percent-encode or explicitly escape characters like ", <, >, and & or pass the
value through a tested sanitizer) so the resulting string cannot break out of
the src attribute or inject HTML/JS; ensure the sanitized value is used in the
template and add a small comment in htmlIframeVideoPlayer noting the escaping
step.
---
Nitpick comments:
In `@src/components/postView/children/postReadingMetadata.tsx`:
- Around line 121-140: The empty catch in the useEffect translation effect
silently swallows errors from getTranslation; update the catch to log the error
(e.g., console.error or your app logger) and optionally set an error state so UI
can reflect failures. Locate the useEffect in postReadingMetadata.tsx that calls
getTranslation(summary, 'auto', targetLang.code) and modify the .catch(() => {})
to .catch((err) => { /* log err and/or set translation error state, ensure not
updating state if cancelled */ }) while preserving the cancelled checks and the
setIsTranslating(false) in finally.
In `@src/components/videoPlayer/videoPlayerView.tsx`:
- Around line 2-8: The file imports StyleSheet from 'react-native' but the repo
convention requires using EStyleSheet from 'react-native-extended-stylesheet';
replace the StyleSheet import with import EStyleSheet (keep other imports like
View, ActivityIndicator, useWindowDimensions, Image as RNImage), then update the
style creation call from StyleSheet.create(...) to EStyleSheet.create(...) and
ensure any style usages in the component (e.g., container, video, loader names)
remain the same so only the import and create call change.
In `@src/hooks/useLinkProcessor.tsx`:
- Line 463: The parseInt call that parses expiresIn in useLinkProcessor (const
expiresIn = parseInt(params.get('expiresIn') || '86400', 10);) should explicitly
pass a radix to avoid ambiguity; update the parseInt invocation in
useLinkProcessor.tsx to include the radix 10 as the second argument (i.e.,
parseInt(..., 10)) while keeping the existing fallback '86400' logic intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b259e366-991c-4c7f-957f-f2a04ef55610
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
package.jsonsrc/components/postHtmlRenderer/postHtmlRenderer.tsxsrc/components/postView/children/postReadingMetadata.tsxsrc/components/videoPlayer/videoPlayerView.tsxsrc/config/locales/en-US.jsonsrc/hooks/useLinkProcessor.tsxsrc/providers/hive/auth.tssrc/screens/aiImageGenerator/screen/aiImageGeneratorScreen.tsxsrc/screens/transfer/screen/transferScreen.tsx
| // Load languages when translate is toggled | ||
| useEffect(() => { | ||
| if (showTranslate && languages.length === 0) { | ||
| fetchSupportedLangs() | ||
| .then((res) => { | ||
| if (res?.length) { | ||
| const langs = res.map((item: any) => ({ code: item.code, name: item.name })); | ||
| setLanguages(langs); | ||
|
|
||
| // Auto-select target language based on app language | ||
| const appCode = appLang?.split('-')[0]; | ||
| const match = langs.find((l: any) => l.code === appCode); | ||
| if (!targetLang) { | ||
| setTargetLang(match || { name: 'English', code: 'en' }); | ||
| } | ||
| } | ||
| }) | ||
| .catch(() => {}); | ||
| } | ||
| }, [showTranslate]); |
There was a problem hiding this comment.
Missing dependencies in useEffect may cause stale closures.
The dependency array only includes showTranslate, but the effect references languages, appLang, and targetLang. While languages.length === 0 check prevents re-fetching, the auto-selection logic at lines 110-114 reads appLang and targetLang which could be stale.
Additionally, the empty .catch(() => {}) silently swallows errors. Consider at least logging the failure for debugging.
🛠️ Suggested fix
useEffect(() => {
if (showTranslate && languages.length === 0) {
fetchSupportedLangs()
.then((res) => {
if (res?.length) {
const langs = res.map((item: any) => ({ code: item.code, name: item.name }));
setLanguages(langs);
// Auto-select target language based on app language
const appCode = appLang?.split('-')[0];
const match = langs.find((l: any) => l.code === appCode);
if (!targetLang) {
setTargetLang(match || { name: 'English', code: 'en' });
}
}
})
- .catch(() => {});
+ .catch((err) => console.warn('Failed to fetch supported languages:', err));
}
- }, [showTranslate]);
+ }, [showTranslate, languages.length, appLang, targetLang]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/postView/children/postReadingMetadata.tsx` around lines 100 -
119, The useEffect that loads languages (the block using useEffect,
showTranslate, languages, fetchSupportedLangs, setLanguages, appLang,
targetLang, setTargetLang) has missing dependencies and swallows errors; update
the dependency array to include languages (or languages.length), appLang, and
targetLang so the auto-selection logic sees current values and avoid stale
closures, and handle promise rejection in the .catch by logging the error (e.g.,
console.error or process logger) instead of an empty handler; ensure this change
still guards re-fetching by keeping the existing languages.length === 0 check to
prevent loops.
| // Reset height when URI changes; detect portrait from thumbnail if available | ||
| useEffect(() => { | ||
| setPlayerHeight(playerWidth * (9 / 16)); | ||
| }, [uri, playerWidth]); | ||
|
|
||
| if (thumbnailUrl) { | ||
| // Load the thumbnail to detect portrait/square orientation | ||
| RNImage.getSize( | ||
| thumbnailUrl, | ||
| (w: number, h: number) => { | ||
| if (w > 0 && h > 0) { | ||
| const ratio = h / w; | ||
| if (ratio > 1.05) { | ||
| // Portrait video — use 3:4 container (matching website) | ||
| // Cap at 4/3 so the embed player controls remain accessible | ||
| const cappedRatio = Math.min(ratio, 4 / 3); | ||
| setPlayerHeight(playerWidth * cappedRatio); | ||
| } | ||
| } | ||
| }, | ||
| () => { | ||
| // Ignore thumbnail load errors | ||
| }, | ||
| ); | ||
| } | ||
| }, [uri, playerWidth, thumbnailUrl]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the videoPlayerView.tsx file
find . -name "videoPlayerView.tsx" -type fRepository: ecency/ecency-mobile
Length of output: 112
🏁 Script executed:
# Check the file exists and read the relevant section
if [ -f "src/components/videoPlayer/videoPlayerView.tsx" ]; then
wc -l src/components/videoPlayer/videoPlayerView.tsx
echo "---"
sed -n '50,85p' src/components/videoPlayer/videoPlayerView.tsx
fiRepository: ecency/ecency-mobile
Length of output: 1312
🏁 Script executed:
# Let's also check the full context of this component to understand the structure
if [ -f "src/components/videoPlayer/videoPlayerView.tsx" ]; then
head -100 src/components/videoPlayer/videoPlayerView.tsx
fiRepository: ecency/ecency-mobile
Length of output: 3313
Prevent stale RNImage.getSize() callbacks from overwriting player height.
RNImage.getSize() is async, so if the component is recycled or dependencies change before the callback fires, the stale callback will overwrite playerHeight with incorrect data from the previous thumbnail. Use an isActive flag with a cleanup function to guard against this:
Fix
useEffect(() => {
+ let isActive = true;
+
setPlayerHeight(playerWidth * (9 / 16));
if (thumbnailUrl) {
// Load the thumbnail to detect portrait/square orientation
RNImage.getSize(
thumbnailUrl,
(w: number, h: number) => {
- if (w > 0 && h > 0) {
+ if (isActive && w > 0 && h > 0) {
const ratio = h / w;
if (ratio > 1.05) {
// Portrait video — use 3:4 container (matching website)
// Cap at 4/3 so the embed player controls remain accessible
const cappedRatio = Math.min(ratio, 4 / 3);
setPlayerHeight(playerWidth * cappedRatio);
}
}
},
() => {
// Ignore thumbnail load errors
},
);
}
+
+ return () => {
+ isActive = false;
+ };
}, [uri, playerWidth, thumbnailUrl]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/videoPlayer/videoPlayerView.tsx` around lines 56 - 80, The
RNImage.getSize callback can be invoked after dependencies change causing stale
updates; inside the useEffect(for uri, playerWidth, thumbnailUrl) introduce an
isActive boolean flag (e.g., let isActive = true) and in the success callback
check isActive before calling setPlayerHeight, and set isActive = false in the
useEffect cleanup to prevent stale callbacks from overwriting playerHeight; keep
the existing logic using RNImage.getSize, but guard both success and error
callbacks with the isActive check so only the latest effect can update via
setPlayerHeight.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/videoPlayer/videoPlayerView.tsx (1)
2-8: UseEStyleSheetin this touched component.The import change keeps this file on
StyleSheet, but components undersrc/components/**are expected to usereact-native-extended-stylesheet. Please switch the touched import and theStyleSheet.create(...)call at the bottom together.As per coding guidelines, "Use
react-native-extended-stylesheet(imported as EStyleSheet) for styling components".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/videoPlayer/videoPlayerView.tsx` around lines 2 - 8, Replace the React Native StyleSheet usage with react-native-extended-stylesheet: update the import list to import EStyleSheet from 'react-native-extended-stylesheet' (instead of StyleSheet) and change the bottom call from StyleSheet.create(...) to EStyleSheet.create(...); ensure any references to StyleSheet variable in this file (e.g., styles = StyleSheet.create(...)) are renamed/updated so the component (videoPlayerView.tsx) uses EStyleSheet for styles consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/videoPlayer/videoPlayerView.tsx`:
- Around line 51-52: The fallback for player dimensions is inconsistent:
playerWidth is set to contentWidth || dim.width but playerHeight uses that value
only at init, and the rendered player sometimes reads contentWidth directly
(causing mismatched aspect ratio); update the code so both sizing and render use
the same source. Concretely, pick one approach and apply it everywhere: either
(A) require contentWidth (remove dim.width fallback) and use contentWidth for
playerWidth and playerHeight calculations (and in the uri branch at 272-275), or
(B) derive playerWidth from the measured layout width state used for rendering
(replace contentWidth with the measured layout width variable when computing
playerWidth and playerHeight and in the uri branch). Ensure you update the
initialization of playerHeight (setPlayerHeight) to compute from the same
playerWidth source and adjust any code in the uri branch (lines referenced
around player rendering) to use that shared playerWidth variable so width and
height remain consistent.
---
Nitpick comments:
In `@src/components/videoPlayer/videoPlayerView.tsx`:
- Around line 2-8: Replace the React Native StyleSheet usage with
react-native-extended-stylesheet: update the import list to import EStyleSheet
from 'react-native-extended-stylesheet' (instead of StyleSheet) and change the
bottom call from StyleSheet.create(...) to EStyleSheet.create(...); ensure any
references to StyleSheet variable in this file (e.g., styles =
StyleSheet.create(...)) are renamed/updated so the component
(videoPlayerView.tsx) uses EStyleSheet for styles consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9af18dad-3103-440f-99a9-3d23a2c971a5
📒 Files selected for processing (3)
src/components/postView/children/postReadingMetadata.tsxsrc/components/videoPlayer/videoPlayerView.tsxsrc/config/locales/en-US.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/config/locales/en-US.json
- src/components/postView/children/postReadingMetadata.tsx
Summary by CodeRabbit
New Features
Improvements
Chores