feat(Android): image pasting in message composer #6848
Conversation
WalkthroughAdds Android clipboard image-paste support by rendering a rich-text input on Android and implementing paste→validate→navigate-to-share flow; updates composer input ref typing and accessibility focus resolution to more robustly obtain the native host element. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ComposerInput
participant TypeRichTextInput
participant PasteHandler
participant CanUploadService
participant ShareView
participant Alert
User->>ComposerInput: Paste image
ComposerInput->>TypeRichTextInput: onPasteImageData(payload)
TypeRichTextInput->>PasteHandler: deliver payload
PasteHandler->>CanUploadService: canUploadFile(file, allowList, maxSize, perm)
alt Upload Allowed
PasteHandler->>ShareView: navigate(with file payload)
ShareView-->>User: preview + confirm
else Upload Denied
PasteHandler->>Alert: show translated error message
Alert-->>User: display error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 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 |
divyanshu-patil
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
app/containers/MessageComposer/MessageComposer.tsx (1)
36-36: Loss of type safety withanytype.Changing from a specific type to
anyeliminates type checking and IntelliSense support. Consider using a union type or a more specific type that accommodates both TextInput and TypeRichTextInput refs.For example:
-const composerInputRef = useRef<any>(null); +const composerInputRef = useRef<TextInput | { getNativeRef?: () => any; focus: () => void; setValue?: (text: string) => void } | null>(null);Or create a shared interface for both input components.
app/containers/MessageComposer/components/ComposerInput.tsx (2)
420-425: Avoid type assertion withas any.Using
as anybypasses type checking. Consider defining a proper interface for the file object or using theIShareAttachmenttype thatcanUploadFileexpects.+import type { IShareAttachment } from '../../../lib/methods/helpers'; + const file = { filename: e.fileName, size: e.fileSize, mime: e.type, path: e.uri -} as any; +} as IShareAttachment;
406-446: Consider adding error handling for navigation.The
Navigation.navigatecall on line 434 doesn't handle potential navigation failures. Consider wrapping it in a try-catch block to handle edge cases gracefully.if (canUploadResult.success) { + try { Navigation.navigate('ShareView', { room, thread: thread || tmid, attachments: [file], action, finishShareView, startShareView }); + } catch (error) { + log(error); + handleError('error-navigation-failed'); + } } else {package.json (1)
124-124: Consider the stability implications of pre-1.0 versions.The
react-native-typerichpackage at version 0.1.9 is the latest published version on npm with no known security vulnerabilities. However, as a pre-1.0 package, it may not have reached stable release status. Evaluate whether this early-stage version meets your project's stability requirements or if a more mature alternative should be considered.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
app/containers/MessageComposer/MessageComposer.tsx(2 hunks)app/containers/MessageComposer/components/ComposerInput.tsx(9 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/containers/MessageComposer/MessageComposer.tsx (1)
app/containers/UIKit/index.tsx (1)
input(169-183)
app/containers/MessageComposer/components/ComposerInput.tsx (4)
app/containers/MessageComposer/hooks/useCanUploadFile.ts (1)
useCanUploadFile(6-18)app/lib/database/services/Subscription.ts (1)
getSubscriptionByRoomId(8-17)app/lib/database/services/Thread.ts (1)
getThreadById(7-16)app/lib/methods/helpers/media.ts (1)
canUploadFile(3-37)
🔇 Additional comments (2)
app/containers/MessageComposer/components/ComposerInput.tsx (2)
190-194: Verify platform-specific input update methods.The Android path uses
setValue()while the non-Android path usessetNativeProps({ text }). Ensure that both methods correctly update the input value and trigger the expected behavior in their respective components.Based on learnings, verify that
TypeRichTextInput.setValue()is the correct method for updating text programmatically on Android, and that it handles cursor positioning and selection as expected.
491-491: Selection change handler names are intentional for different components.Line 491 uses
onSelectionChange(React Native's TextInput) while line 466 usesonChangeSelection(TypeRichTextInput from react-native-typerich). The different prop names correspond to different event signatures: TextInput providese.nativeEvent.selection, while TypeRichTextInput providesewith directstartandendproperties. This is correct usage for the respective libraries and requires no changes.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
app/containers/MessageComposer/components/ComposerInput.tsx (6)
81-81: Remove commented code.The commented line
// const isAndroid = false;should be removed as it appears to be debug code.const isAndroid = Platform.OS === 'android'; -// const isAndroid = false;
217-217: Remove debug console.log statement.Debug logging should be removed before merging to production.
const onChangeText: TextInputProps['onChangeText'] = text => { textRef.current = text; debouncedOnChangeText(text); - console.log(text); setInput(text, undefined, true); };
225-231: Remove debug console.log statements.Multiple debug console.log statements should be removed before merging.
const onChangeSelection = (e: OnChangeSelectionEvent) => { const { start, end } = e; const selection = { start, end }; - console.log('selection========', e); selectionRef.current = selection; - console.log('sel', selection); };
411-411: Remove debug console.log statement.Debug logging should be removed before merging.
const handleOnPaste = async (e: onPasteImageEventData) => { if (e.error) { handleError(e.error.message); return; } - console.log(e); if (!rid) return;
444-444: Remove debug console.log statement.Debug logging should be removed before merging.
} else { handleError(canUploadResult.error); - console.log('error block'); }
454-479: Remove debug text from placeholder.Line 458 contains
'custom textinput ${placeholder}'which appears to be debug text. The placeholder should match the non-Android path.<TypeRichTextInput style={[styles.textInput]} color={colors.fontDefault} - placeholder={`custom textinput ${placeholder}`} + placeholder={placeholder} placeholderTextColor={colors.fontAnnotation}
🧹 Nitpick comments (3)
app/containers/MessageComposer/components/ComposerInput.tsx (3)
233-253: Consider standardizing focus/blur handler usage.The focus/blur logic is correctly extracted into
handleFocusandhandleBlur. However, the Android path (lines 467-468) directly uses these handlers, while the non-Android path (lines 492-493) uses theonFocus/onBlurwrappers. For consistency, consider using the same approach in both paths.Apply this diff to use wrappers consistently:
- onFocus={handleFocus} - onBlur={handleBlur} + onFocus={onFocus} + onBlur={onBlur}
425-425: Replace type assertion with proper interface.The
as anytype assertion bypasses type safety. Consider defining a proper type or using theIShareAttachmentinterface directly.- } as any; + } as IShareAttachment;Alternatively, import and use the correct type from the share attachment definitions.
463-463: Fix typo in comment.Line 469 contains a typo: "behaiviour" should be "behaviour".
- // underlineColorAndroid='transparent' // by default behaiviour + // underlineColorAndroid='transparent' // by default behaviourAlso applies to: 469-469
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/containers/MessageComposer/components/ComposerInput.tsx(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/containers/MessageComposer/components/ComposerInput.tsx (4)
app/containers/MessageComposer/hooks/useCanUploadFile.ts (1)
useCanUploadFile(6-18)app/lib/database/services/Subscription.ts (1)
getSubscriptionByRoomId(8-17)app/lib/database/services/Thread.ts (1)
getThreadById(7-16)app/lib/methods/helpers/media.ts (1)
canUploadFile(3-37)
🔇 Additional comments (6)
app/containers/MessageComposer/components/ComposerInput.tsx (6)
2-2: LGTM! Imports support the new Android paste functionality.The new imports are all used appropriately in the implementation: Alert for error dialogs, TypeRichTextInput for Android-specific input handling, canUploadFile for permission validation, and database services for retrieving room/thread context during paste operations.
Also applies to: 6-6, 8-8, 20-20, 47-48
75-80: LGTM! Permission checks properly initialized.The permission flags and settings are correctly retrieved and will be used in the paste handler to validate file uploads.
190-194: LGTM! Platform-specific input update methods handled correctly.The conditional logic appropriately uses
setValuefor Android's TypeRichTextInput andsetNativePropsfor the standard TextInput, accounting for the different APIs of these components.
448-450: LGTM! Error handling with translated messages.The error handler correctly displays translated error messages via Alert, with a fallback for untranslated errors.
480-502: LGTM! TextInput configuration for non-Android platforms.The standard TextInput is correctly configured with appropriate handlers and props for iOS and other platforms. The different prop names (
onSelectionChangevsonChangeSelection) are correct as they match each component's API.
517-518: LGTM! Style property ordering.Minor reordering of fontSize placement in the style definition. No functional impact.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/containers/MessageComposer/components/ComposerInput.tsx (1)
460-509: Consider extracting shared props to reduce duplication.The Android and non-Android render paths duplicate many props (placeholder, colors, multiline, etc.). While the differences in event handlers are intentional, you could extract the common props into a shared object to improve maintainability.
Example refactor:
const commonProps = { placeholderTextColor: colors.fontAnnotation, defaultValue: '', multiline: true, ...(autocompleteType ? { autoComplete: 'off', autoCorrect: false, autoCapitalize: 'none' } : {}), keyboardAppearance: theme === 'light' ? 'light' : 'dark', testID: `message-composer-input${tmid ? '-thread' : sharing ? '-share' : ''}`, onChangeText, onTouchStart };Then spread
commonPropsinto both branches and add platform-specific props separately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/containers/MessageComposer/components/ComposerInput.tsx(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/containers/MessageComposer/components/ComposerInput.tsx (4)
app/containers/MessageComposer/hooks/useCanUploadFile.ts (1)
useCanUploadFile(6-18)app/lib/database/services/Subscription.ts (1)
getSubscriptionByRoomId(8-17)app/lib/database/services/Thread.ts (1)
getThreadById(7-16)app/lib/methods/helpers/media.ts (1)
canUploadFile(3-37)
🔇 Additional comments (3)
app/containers/MessageComposer/components/ComposerInput.tsx (3)
75-79: LGTM!The upload permission and file constraints are correctly retrieved using the custom hook and Redux selectors.
233-253: LGTM!The focus/blur handler wrappers allow for platform-specific event handling. The Android
TypeRichTextInputuseshandleFocus/handleBlurdirectly, while the standardTextInputuses the wrappedonFocus/onBlurhandlers.
190-194: VerifysetValuemethod signature in react-native-typerich package.The Android path uses
inputRef.current?.setValue(text)which is a TypeRichTextInput-specific API (version 0.1.9). The package exists on npm, and the platform-specific branching is correct—TypeRichTextInput is only rendered on Android, while the non-Android path uses standard React Native TextInput withsetNativeProps. However, confirm the exact method signature and parameters forsetValuein the react-native-typerich documentation or package source.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
app/containers/MessageComposer/components/ComposerInput.tsx (6)
81-82: Remove commented debug code.The commented line at line 82 should be removed as it's leftover debug code.
Apply this diff:
const isAndroid = Platform.OS === 'android'; -// const isAndroid = false;
218-218: Remove debug console.log statement.Debug logging should be removed before merging to production.
Apply this diff:
const onChangeText: TextInputProps['onChangeText'] = text => { textRef.current = text; debouncedOnChangeText(text); - console.log(text); setInput(text, undefined, true); };
226-232: Remove debug console.log statements.Multiple debug console.log statements should be removed before merging.
Apply this diff:
const onChangeSelection = (e: OnChangeSelectionEvent) => { const { start, end } = e; const selection = { start, end }; - console.log('selection========', e); selectionRef.current = selection; - console.log('sel', selection); };
412-412: Remove debug console.log statement.Debug logging should be removed before merging to production.
Apply this diff:
const handleOnImagePaste = async (e: onPasteImageEventData) => { if (e.error) { handleError(e.error.message); return; } - console.log(e); if (!rid) return;
451-451: Remove debug console.log statement.Debug logging should be removed before merging to production.
Apply this diff:
} else { handleError(canUploadResult.error); - console.log('error block'); }
465-465: Remove debug text from placeholder.The placeholder contains debug text
'custom textinput ${placeholder}'which should be replaced with just{placeholder}to match the non-Android path.Apply this diff:
<TypeRichTextInput style={[styles.textInput]} color={colors.fontDefault} - placeholder={`custom textinput ${placeholder}`} + placeholder={placeholder} placeholderTextColor={colors.fontAnnotation}
🧹 Nitpick comments (1)
app/containers/MessageComposer/components/ComposerInput.tsx (1)
455-457: Improve error message fallback handling.When
erroris undefined, the Alert will display "undefined" as the message. Consider providing a default error message.Apply this diff to improve the fallback:
const handleError = (error?: string) => { - Alert.alert(I18n.t('Error_uploading'), error && I18n.isTranslated(error) ? I18n.t(error) : error); + Alert.alert( + I18n.t('Error_uploading'), + error && I18n.isTranslated(error) ? I18n.t(error) : (error || I18n.t('An_error_occurred')) + ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
app/containers/MessageComposer/components/ComposerInput.tsx(8 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
app/containers/MessageComposer/components/ComposerInput.tsx (5)
app/containers/MessageComposer/hooks/useCanUploadFile.ts (1)
useCanUploadFile(6-18)app/lib/database/services/Subscription.ts (1)
getSubscriptionByRoomId(8-17)app/lib/database/services/Thread.ts (1)
getThreadById(7-16)app/definitions/IAttachment.ts (1)
IShareAttachment(78-92)app/lib/methods/helpers/media.ts (1)
canUploadFile(3-37)
🔇 Additional comments (3)
app/containers/MessageComposer/components/ComposerInput.tsx (3)
191-195: LGTM - Platform-specific input methods handled correctly.The code appropriately uses
setValuefor the Android TypeRichTextInput andsetNativePropsfor the standard TextInput, with proper optional chaining for safety.
234-254: LGTM - Event handlers properly structured.The separation between core handlers (
handleFocus,handleBlur) and typed wrappers (onFocus,onBlur) is appropriate. The Android path calls core handlers directly while the non-Android TextInput path uses the typed wrappers, accommodating different component APIs.
470-470: Verify blurOnSubmit behavior for TypeRichTextInput.The Android path has
blurOnSubmitcommented out (line 470), while the non-Android TextInput explicitly setsblurOnSubmit={false}(line 493). Inspect the TypeRichTextInput component implementation to confirm whether it supports the blurOnSubmit prop, what its default behavior is, and why it differs from the standard TextInput approach.
| if (canUploadResult.success) { | ||
| Navigation.navigate('ShareView', { | ||
| room, | ||
| thread: thread || tmid, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find ShareView component definition
fd -e tsx -e ts | xargs rg -l "ShareView" | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 539
🏁 Script executed:
# Look for ShareView component and its type definition
rg "export.*ShareView|interface.*ShareView|type.*ShareView" -A 10Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1423
🏁 Script executed:
# Check the ComposerInput file to understand the types of thread and tmid variables
cat -n app/containers/MessageComposer/components/ComposerInput.tsx | sed -n '430,460p'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 960
🏁 Script executed:
# Get the function signature and variable declarations in ComposerInput.tsx
cat -n app/containers/MessageComposer/components/ComposerInput.tsx | sed -n '1,100p'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 5708
🏁 Script executed:
# Search for thread and tmid variable declarations and their types
rg "thread|tmid" app/containers/MessageComposer/components/ComposerInput.tsx -B 3 -A 1 | head -80Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 2336
🏁 Script executed:
# Check the exact function containing line 443
ast-grep --pattern $'const $_ = ($_) => {
$$$
thread: thread || tmid
$$$
}'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 60
🏁 Script executed:
# Find getThreadById definition and its return type
rg "export.*getThreadById|function getThreadById" -A 5Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 498
🏁 Script executed:
# Check useRoomContext return type
rg "useRoomContext" app/views/RoomView/context.tsx -A 30 | head -80Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 148
🏁 Script executed:
# Look for the complete function context around line 443 to see the full picture
cat -n app/containers/MessageComposer/components/ComposerInput.tsx | sed -n '410,450p'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1140
🏁 Script executed:
# Find RoomContext definition
fd -type f -name "*.tsx" -o -name "*.ts" | xargs rg -l "useRoomContext" | grep -i contextRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 295
🏁 Script executed:
# Check the collection find() return type and thread model type
rg "threadCollection.find|TThreadModel|IThread" app/lib/database/services/Thread.ts -A 3 -B 3Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 261
🏁 Script executed:
# Look for the context definition
fd -type f \( -name "*context*" \) | grep -i roomRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 248
🏁 Script executed:
# Read RoomContext type definition
cat -n app/views/RoomView/context.ts | head -150Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1099
🏁 Script executed:
# Look for the context provider and useRoomContext hook definition
rg "useRoomContext|export.*RoomContext" app/views/RoomView/context.ts -A 15 | head -100Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 797
🏁 Script executed:
// Instead of:
thread: thread || tmid
// Use:
thread: thread || tmid || ''
// or guard against undefined earlierRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 410
Remove the type union issue by ensuring a non-undefined value is always passed to ShareView.
The code passes thread: thread || tmid to ShareView, but tmid is typed as string | undefined in the RoomContext, and getThreadById() returns TThreadModel | null. This means the expression can evaluate to undefined when both thread and tmid are falsy. However, ShareView's type definition expects thread: TThreadModel | string (excluding undefined).
To fix this, ensure a valid value is always passed—either by providing a fallback value (e.g., thread: thread || tmid || '') or by adding a guard condition to prevent navigation when neither value is available.
🤖 Prompt for AI Agents
In app/containers/MessageComposer/components/ComposerInput.tsx around line 443,
the prop `thread: thread || tmid` can evaluate to undefined because tmid is
typed string | undefined and thread may be null, violating ShareView's expected
TThreadModel | string; fix by ensuring a non-undefined value is passed — either
provide a safe fallback like `thread || tmid || ''` or add a guard that prevents
calling/navigating to ShareView when both thread and tmid are falsy so ShareView
always receives a valid string or TThreadModel.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/containers/MessageComposer/components/ComposerInput.tsx(8 hunks)
🔇 Additional comments (10)
app/containers/MessageComposer/components/ComposerInput.tsx (10)
2-2: LGTM! Imports support the new Android paste flow.All new imports (Platform, Alert, TypeRichTextInput, paste event types, upload validation helpers, and data retrieval functions) are properly used in the Android image-paste implementation.
Also applies to: 6-6, 8-8, 20-20, 47-49
76-81: LGTM! Upload settings properly initialized.The upload permission check and file size/type constraints are correctly retrieved from the room context and settings, and the platform check appropriately gates Android-specific behavior.
190-194: LGTM! Input value updates correctly handle both input types.The conditional correctly uses
setValuefor Android's TypeRichTextInput andsetNativePropsfor the standard TextInput on other platforms, respecting each component's API.
214-218: LGTM! Draft message handling is correct.The
onChangeTexthandler properly passesforceUpdateDraftMessage: trueto ensure draft persistence is updated when the user types, addressing the previous review feedback.
224-228: LGTM! Selection tracking handles TypeRichTextInput events correctly.The
onChangeSelectionhandler correctly extractsstartandenddirectly from the TypeRichTextInput event (rather than fromnativeEvent.selectionas in standard TextInput), properly adapting to the library's event structure.
230-250: LGTM! Focus/blur handling correctly adapts to different event signatures.The layered handlers (
handleFocus/handleBlurwithout parameters,onFocus/onBlurwith parameters) properly accommodate TypeRichTextInput's no-argument callbacks (line 468 comment) while maintaining compatibility with TextInput's event-based callbacks. The iOS back-swipe check inhandleBluris preserved.
396-401: LGTM! Share view helpers are correctly implemented.The
startShareViewandfinishShareViewhelpers provide clean callbacks for initializing and finalizing the share flow, properly integrating with the existingselectedMessagesandsetQuotesAndTextcontext.
449-451: LGTM! Error handling is user-friendly.The
handleErrorhelper properly displays localized error messages viaAlert, checking whether the error string is a translation key and falling back to the raw message when needed.
454-502: LGTM! Platform-specific rendering correctly implements image paste for Android.The conditional rendering properly branches on
isAndroidto render:
- Android: TypeRichTextInput with
onPasteImageData={handleOnImagePaste}(line 477) to enable image paste, usinghandleFocus/handleBlurdirectly (no event parameters), andonChangeSelectionfor TypeRichTextInput's event structure- Non-Android: Standard TextInput with
onFocus/onBlurwrappers (accepting event parameters) andonSelectionChangefor React Native's event structureBoth paths share common props (placeholder, colors, testID, keyboard appearance) while correctly adapting event handlers to each component's API as documented in the line 468 comment.
516-517: LGTM! Style reordering has no functional impact.Swapping
lineHeightandfontSizeorder is a cosmetic change with no effect on rendering behavior.
|
converting to draft as app is crashing on opening on ios |
|
IOS app crashing issue is fixed in |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json(1 hunks)
divyanshu-patil
changed the title
divyanshu-patil
changed the title
|
converting to draft as lib is in beta stage |
|
lib is stable and tested with |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/containers/MessageComposer/components/ComposerInput.tsx (1)
405-449: Fix the thread parameter type violation.Past reviews flagged this as critical and marked it as addressed in commit 855ee28, but the code at line 440 still shows the unfixed issue.
Problem: The expression
thread || tmidcan evaluate toundefined:
- When pasting in the main room (not in a thread),
tmidisundefinedthreadremainsundefinedbecause the conditional block at lines 420-422 doesn't executegetThreadByIdreturnsTThreadModel | null, andtmidis typedstring | undefined- ShareView expects
thread: TThreadModel | string, excludingundefinedImpact: This type contract violation can cause runtime errors when ShareView attempts to access thread properties.
🔎 Recommended fix
Apply one of these solutions:
Solution 1 (simplest): Provide a fallback empty string:
Navigation.navigate('ShareView', { room, - thread: thread || tmid, + thread: thread || tmid || '', attachments: [file], action, finishShareView, startShareView });Solution 2: Add a guard condition if ShareView cannot handle empty strings for the main room case:
+ // When pasting in main room (not thread), thread param may be undefined + if (!thread && !tmid) { + // Handle main room paste - consider if thread should be optional in ShareView + // or if empty string is acceptable + } + Navigation.navigate('ShareView', { room, thread: thread || tmid, attachments: [file], action, finishShareView, startShareView });
🧹 Nitpick comments (1)
app/containers/MessageComposer/components/ComposerInput.tsx (1)
158-159: Address or remove the TODO comment.The TODO comment at line 158 (
// todo mention command here) appears to be leftover from development. If additional logic is needed for the mention command, please implement it or create a tracking issue. Otherwise, remove the comment.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
app/containers/MessageComposer/components/ComposerInput.tsxpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
app/containers/MessageComposer/components/ComposerInput.tsx (5)
app/containers/MessageComposer/hooks/useCanUploadFile.ts (1)
useCanUploadFile(6-18)app/lib/database/services/Subscription.ts (1)
getSubscriptionByRoomId(8-17)app/lib/database/services/Thread.ts (1)
getThreadById(7-16)app/definitions/IAttachment.ts (1)
IShareAttachment(78-92)app/lib/methods/helpers/media.ts (1)
canUploadFile(3-37)
🔇 Additional comments (4)
app/containers/MessageComposer/components/ComposerInput.tsx (4)
398-403: LGTM!The ShareView helper functions correctly integrate the paste flow with the existing share functionality.
451-453: LGTM!The error handler properly integrates with the I18n system and provides good user feedback.
192-196: The APIs used (setText, onChangeSelection, onPasteImageData) are properly imported with TypeScript types from react-native-typerich@^1.1.1, which is a stable release version. All methods are correctly typed and safely guarded with optional chaining. No API compatibility verification is needed.
6-6: The version number in this review is incorrect. The libraryreact-native-typerichis specified as version^1.1.1in package.json, not0.1.11+. Additionally, thehandleOnImagePasteimplementation already includes comprehensive error handling: it checks fore.error, validatesridand room existence, and usescanUploadFilevalidation with error callbacks. The suggested verification concerns about incomplete error handling are unfounded based on the actual code implementation.Likely an incorrect or invalid review comment.
| const text = textRef.current; | ||
| const newText = `${text.substr(0, start)}@${text.substr(start, end - start)}${text.substr(end)}`; | ||
| setInput(newText, { start: start + 1, end: start === end ? start + 1 : end + 1 }); | ||
| // todo mention command here |
There was a problem hiding this comment.
this comment is marked for future PR's to include live markdown directly inside the TextInput itself
1 participant
Proposed changes
TypeRichTextInputon android andTextInputon iosonPasteImageDataevent handler which opensShareViewto send imagesIssue(s)
closes #1691
closes #178
How to test or reproduce
Screenshots
react-native-typerich.demo.mp4
Types of changes
Checklist
Further comments
Future planning
use experimental message composeroption in preferences menu to toggle between legacy composer and new Typerich composerTextInputfunctionalityeg
i will update following checks once it supports all core features
Mentions,ChannelsandCommandsSummary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.