feat: Merge V3 Branch to Release 3.0.0 #1279
Conversation
Started some work on V3 Message Component Cleaned up several Components Cleaned up list merging Better query handling for messages
Message Status Chat message work
Message Status Chat message work
* feat: Update to use inbox id endpoint Added new query to use inbox endpoint Updated usage for group avatar/members Updated socials type to match what is being returned from the server * feat: Remove Inbox Id store Remove inbox id store * Updates Added tests Split util files Added helper util functions Added helper hooks * Updated to map and filter
Added helpers Updated naming to match v3 rather than just groups Updated Pinned Conversations and Conversation List
Fix for missing function
Moved util functions to separate files Removed helpers/index files Prepared media attachments for split from V2 handling :
Added V3 Message and V3Message to Display Added conversation context to help reduce overall changes
Added blocked view support Cleaned up tsc Added helper query config Added API setting pinned conversations
* feat: Remove V2 * feat: Remove V2 Removes almost all V2 Handling Removes App Database Cleans up queries Bumps to V3 SDK Updated native iOS notifications to work with V3 only Removes some frames functionality Leaves the app utterly broken
Fix Android Build Update Conversation List to have hooks
* wip * wip * wip * wip * wip * wip * wip * remove unused rn patches * wip * wip * fix: New DM Work Updated handling when sending new Conversations --------- Co-authored-by: Thierry <skodathierry@gmail.com>
Updated Tests to work on V3 Only
* fix tsc * remove unused test
Added new query Updated use of permissions Added new translations Updated hooks Added new utils
Added context menu store Added handling for context menus with pinned conversations Fixed dark mode messages Fixed timestamps Fixed Unread
Added notification subscribing for V3 only
* feat: V3 Search Added search for V3 Only Added tests Updated Snapshots * remove checks Removed async Removed await * remove inboxId check
Added handling for JS Merged feature/v3-split
WalkthroughThe changes in this pull request involve significant refactoring and cleanup across multiple components in the application. Key modifications include the removal of unused imports, hooks, and components, as well as updates to props and type definitions for improved type safety. Notably, the Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed Scenarios
Show details
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 40
🧹 Outside diff range and nitpick comments (57)
components/TransactionPreview/TransactionPreview.tsx (3)
Line range hint
19-31: Consider enhancing type safety for transaction dataThe
TransactionDataandTransactionToTriggertypes could benefit from stricter typing:export type TransactionData = { to: string; - data?: string; + data?: `0x${string}` | undefined; - value?: string; + value?: bigint | string | undefined; }; export type TransactionToTrigger = TransactionData & { id: string; - chainId: number; + chainId: number & { readonly brand: unique symbol }; };
Line range hint
91-106: Enhance error handling in simulation resultsThe simulation success case could benefit from additional error checking:
setSimulation({ status: "failure", error: simulationResult.error.message, }); } else { + if (!simulationResult?.changes) { + setSimulation({ + status: "failure", + error: "Simulation completed but no changes were detected", + }); + return; + } simulationResult?.changes?.forEach((change) => { // refreshProfileForAddress(account, change.to); }); setSimulation({ status: "success", result: simulationResult }); }
Line range hint
164-187: Consider implementing retry mechanism for failed transactionsThe error handling in the transaction trigger could be enhanced with a retry mechanism for certain types of failures (e.g., network issues).
} catch (e: any) { if (`${e}`.includes("User rejected the request")) { setTxState({ status: "pending" }); } else { logger.error(e); + // Determine if error is retryable + const isRetryableError = e?.message?.includes('network error') || + e?.message?.includes('timeout'); + let txError = e?.message; if (!txError) { try { txError = JSON.stringify(e); } catch {} } if (!txError) txError = e; - setTxState({ status: "failure", error: `${txError}` }); + setTxState({ + status: "failure", + error: `${txError}`, + retryable: isRetryableError + }); } }components/Chat/Message/MessageSender.tsx (1)
11-24: Consider adding name prop validation.While the component is well-structured, it might benefit from validation to handle empty or undefined names gracefully.
Consider adding validation:
export const MessageSenderDumb = ({ name }: MessageSenderDumbProps) => { const styles = useStyles(); + const displayName = name?.trim() || 'Unknown'; return ( <VStack style={styles.groupSenderContainer}> <Text preset="smaller" color="secondary"> - {name} + {displayName} </Text> </VStack> ); };components/Conversation/ConversationTitleDumb.tsx (2)
20-25: Consider renaming the component to follow conventional namingThe suffix "Dumb" is not a conventional naming pattern. Consider renaming to something more descriptive like
ConversationTitlePresenteror simplyConversationTitleto better reflect its presentational nature.
47-71: Improve style maintainabilityConsider extracting platform-specific values and magic numbers into named constants for better maintainability:
+const STYLE_CONSTANTS = { + android: { + avatarMarginRight: 24, + avatarMarginLeft: -9, + containerLeft: -36, + }, + ios: { + avatarMarginRight: 7, + avatarMarginLeft: 0, + containerLeft: 0, + }, + common: { + touchablePaddingRight: 40, + iosFontSize: 16, + } +}; const useStyles = () => { const colorScheme = useColorScheme(); return StyleSheet.create({ avatar: { - marginRight: Platform.OS === "android" ? 24 : 7, - marginLeft: Platform.OS === "ios" ? 0 : -9, + marginRight: STYLE_CONSTANTS[Platform.OS].avatarMarginRight, + marginLeft: STYLE_CONSTANTS[Platform.OS].avatarMarginLeft, }, container: { flexDirection: "row", flexGrow: 1 }, touchableContainer: { flexDirection: "row", justifyContent: "flex-start", - left: Platform.OS === "android" ? -36 : 0, + left: STYLE_CONSTANTS[Platform.OS].containerLeft, width: "100%", alignItems: "center", - paddingRight: 40, + paddingRight: STYLE_CONSTANTS.common.touchablePaddingRight, }, // ... rest of the styles }); };components/Chat/Message/messageContextStore.tsx (6)
15-15: Redundant intersection with an empty object.The type
IMessageContextStoreStateis defined as an intersection ofIMessageContextStorePropsand an empty object{}. This intersection doesn't add any new properties and is redundant.Consider simplifying the type definition:
-type IMessageContextStoreState = IMessageContextStoreProps & {}; +type IMessageContextStoreState = IMessageContextStoreProps;
29-32: Potential unnecessary state updates inuseEffect.The
useEffecthook depends on the entirepropsobject, which may cause unnecessary updates ifpropscontains non-primitive values or functions that change reference on each render.Consider optimizing the
useEffectdependency by:
- Memoizing
propsbefore passing them to the provider.- Using a custom comparison to check if relevant
propshave changed before updating the state.Example using
useMemo:+import { useMemo } from "react"; export const MessageContextStoreProvider = memo( ({ children, ...props }: IMessageContextStoreProviderProps) => { const storeRef = useRef<IMessageContextStore>(); if (!storeRef.current) { storeRef.current = createMessageContextStore(props); } + const memoizedProps = useMemo(() => props, [/* dependencies */]); useEffect(() => { // TODO: Check if the props have made something change in the store? - storeRef.current?.setState(props); + storeRef.current?.setState(memoizedProps); - }, [props]); + }, [memoizedProps]);Or selectively picking properties for the dependency array:
useEffect(() => { // TODO: Check if the props have made something change in the store? storeRef.current?.setState(props); -}, [props]); +}, [props.messageId, props.fromMe, props.sentAt /* add other relevant props */]);
30-30: Address the TODO comment regarding state changes.The TODO comment indicates uncertainty about how prop changes affect the store state. It's important to ensure that state updates are intentional and efficient.
Would you like assistance in reviewing and refining the state management logic to ensure optimal performance?
58-60: Missing provider error message lacks specificity.In the
useMessageContextStoreContextfunction, the error thrown when the store is missing provides a clear message. However, inuseMessageContextStore, the error lacks a descriptive message.Add an error message to aid in debugging:
export function useMessageContextStore() { const store = useContext(MessageContextStoreContext); - if (!store) throw new Error(); + if (!store) throw new Error("Missing MessageContextStore.Provider in the tree"); return store; }
42-56: Simplify store creation with initial state.The
createMessageContextStorefunction mergesDEFAULT_PROPSwithinitProps, but sinceinitPropsshould provide all necessary properties,DEFAULT_PROPSmight be unnecessary.Consider removing
DEFAULT_PROPSif it's redundant:-const DEFAULT_PROPS: IMessageContextStoreProps = { - // default values... -}; return createStore<IMessageContextStoreState>()((set) => ({ - ...DEFAULT_PROPS, ...initProps, }));Alternatively, ensure that
DEFAULT_PROPSare only used for optional properties.
62-69: Generic type parameterTmay not be necessary.The function
useMessageContextStoreContextuses a generic typeT, which might add unnecessary complexity if not required.If the selector function always returns a specific type, consider removing the generic parameter:
-export function useMessageContextStoreContext<T>( - selector: (state: IMessageContextStoreState) => T -): T { +export function useMessageContextStoreContext( + selector: (state: IMessageContextStoreState) => any +): any {Or, if generics are needed, ensure they are used consistently.
components/Chat/Message/MessageStatusDumb.tsx (3)
58-80: Simplify nested timers to improve performanceUsing a nested
setTimeoutinsiderequestAnimationFrameadds complexity and may cause unintended delays.Consider simplifying the timing logic:
- setTimeout(() => { - requestAnimationFrame(() => { + requestAnimationFrame(() => { // existing animation code }); - }, 100);If the 100ms delay is necessary, document the reason to aid maintainability.
37-40: Use descriptive constants instead of magic numbersHard-coded values like
22and1reduce readability and maintainability.Define constants for clarity:
const ANIMATION_DURATION = 200; const ANIMATION_HEIGHT = 22; const ANIMATION_SCALE = 1;Update your code to use these constants:
const timingConfig = { - duration: 200, + duration: ANIMATION_DURATION, easing: Easing.inOut(Easing.quad), }; - const height = useSharedValue(isLatestSettledFromMe ? 22 : 0); + const height = useSharedValue(isLatestSettledFromMe ? ANIMATION_HEIGHT : 0); - const scale = useSharedValue(isLatestSettledFromMe ? 1 : 0); + const scale = useSharedValue(isLatestSettledFromMe ? ANIMATION_SCALE : 0);
86-96: Handle conditional rendering to avoid returningfalseThe current return statement may return
falseifshouldDisplayisfalse, which can cause issues.Ensure the component returns
nullinstead offalsewhen not rendering:return ( - shouldDisplay && ( + shouldDisplay ? ( // existing code - ) + ) : null );components/Chat/ConsentPopup/GroupConsentPopup.tsx (2)
13-15: Consider adding documentation forGroupConsentPopupPropsAdding JSDoc comments to
GroupConsentPopupPropscan enhance code readability and maintainability.
Line range hint
34-48: Localize hardcoded strings for internationalizationReplace hardcoded strings like
"unknown"with localized versions using thetranslatefunction to support multiple languages.Apply this change:
- "unknown", // To display "Remove & Block inviter" + translate("unknown_inviter"), // Localized stringcomponents/PinnedConversations/PinnedConversations.tsx (1)
24-27: Simplify conditional rendering ofpinnedConvosSince you've already checked if
topicsis falsy, the optional chaining operator?.intopics?.map(...)is unnecessary. You can simplify the code by removing the optional chaining.Simplify the code as follows:
const pinnedConvos = !topics ? [] - : topics?.map((topic) => { + : topics.map((topic) => { return <PinnedV3Conversation topic={topic} key={topic} />; });components/PinnedConversations/PinnedConversation.tsx (1)
11-11: UnusedshowUnreadprop inPinnedConversationThe
showUnreadprop is defined inPinnedConversationPropsbut isn't used within the component. If this prop is intended to affect the component's behavior or rendering, consider implementing the necessary logic. If not needed, you can remove it to simplify the code.Possible actions:
- Implement logic using
showUnreadto reflect unread status.- Remove
showUnreadfrom the props if it's unnecessary.components/PinnedConversations/PinnedV3Conversation.tsx (1)
14-26: Avoid unnecessary type assertions withasYou're using type assertions to cast
conversationtoGroupWithCodecsTypeorDmWithCodecsType. This can be unsafe if the types don't match. Instead, consider adjusting your type definitions or using type guards to let TypeScript infer the types safely.Adjust the code to avoid explicit type assertions:
if (conversationIsGroup(conversation)) { return ( <PinnedV3GroupConversation group={conversation} /> ); } else { return ( <PinnedV3DMConversation conversation={conversation} /> ); }components/PinnedConversations/PinnedV3DMConversation.tsx (1)
130-130: UnusedshowUnreadprop inPinnedConversationThe
showUnreadprop is passed toPinnedConversation, but as noted earlier,PinnedConversationdoesn't use this prop. Ensure that this prop is utilized if intended, or remove it to clean up the code.components/Chat/Attachment/AddAttachmentButton.tsx (1)
153-153: Simplify null checks using optional chainingYou can simplify the null checks by using optional chaining when accessing
match[1].Apply this diff to improve readability:
if (!mimeType) { const match = resizedImage.uri.match(DATA_MIMETYPE_REGEX); - if (match && match[1]) { + if (match?.[1]) { mimeType = match[1]; } }🧰 Tools
🪛 Biome (1.9.4)
[error] 153-153: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
components/V3DMListItem.tsx (2)
166-169: Review the Use ofrunOnJSin CallbacksThe
onLongPresscallback usesrunOnJSto invoketriggerHapticFeedbackandshowContextMenu. IfonLongPressis executed on the JavaScript thread and not within a Reanimated worklet, the use ofrunOnJSis unnecessary and can be removed to simplify the code.Apply this change:
-const onLongPress = useCallback(() => { - runOnJS(triggerHapticFeedback)(); - runOnJS(showContextMenu)(); -}, [triggerHapticFeedback, showContextMenu]); +const onLongPress = useCallback(() => { + triggerHapticFeedback(); + showContextMenu(); +}, [triggerHapticFeedback, showContextMenu]);
175-178: Remove Unused Empty Callback FunctionsThe callbacks
onWillRightSwipeandonRightSwipeare defined but contain empty functions. If these are not intended for future use, consider removing them to clean up the code.Apply this diff:
-const onWillRightSwipe = useCallback(() => {}, []); -const onRightSwipe = useCallback(() => {}, []);components/Chat/ChatGroupUpdatedMessage.tsx (1)
63-103: Refactor to Eliminate Code Duplication Between Member ComponentsThe
ChatGroupMemberLeftandChatGroupMemberJoinedcomponents share similar logic and structure. Consider refactoring these into a single component with a prop to determine the action (joined or left), or extract shared logic into a helper function to enhance maintainability.Also applies to: 109-150
components/Chat/ChatDumb.tsx (1)
129-141: Move Color Calculation InsideuseAnimatedStyleHookThe
tertiarycolor variable is defined outside theuseAnimatedStylehook but is used within it. To ensure the style updates correctly whencolorSchemechanges, move the calculation inside the hook.Apply this change:
-const tertiary = tertiaryBackgroundColor(colorScheme); const textInputStyle = useAnimatedStyle( () => { + const tertiary = tertiaryBackgroundColor(colorScheme); return { position: "absolute", width: "100%", backgroundColor: tertiary, // ... }; }, [keyboardHeight, colorScheme, insets.bottom] );components/Conversation/V3Conversation.tsx (2)
118-121: Display a placeholder when the conversation is not foundCurrently, when
conversationNotFoundis true (lines 118-121), the component returns null, resulting in a blank screen. Consider displaying a user-friendly message or placeholder to enhance the user experience.Apply this diff to provide feedback to the user:
- return null; + return ( + <Center> + <Text>{translate("conversation_not_found")}</Text> + </Center> + );
123-126: Show a message when there are no messages in the conversationWhen
messages?.ids.lengthis zero and not loading (lines 123-126), the component returns null, which may confuse the user. Displaying a message indicating that there are no messages can improve usability.Apply this diff to inform the user:
- return null; + return ( + <Center> + <Text>{translate("no_messages_yet")}</Text> + </Center> + );components/Chat/Attachment/AttachmentMessagePreview.tsx (1)
119-153: Remove commented-out code to improve maintainabilityThere is a significant block of commented-out code between lines 119-153. Keeping outdated code can clutter the codebase and reduce readability.
Apply this diff to clean up the code:
- // const openInWebview = useCallback(async () => { - // // ... - // }); - // const clickedOnAttachmentBubble = useCallback(() => { - // // ... - // }); - // const showing = - // !attachment.loading && - // !!attachment.mediaURL && - // attachment.mediaType !== "UNSUPPORTED"; - // useEffect(() => { - // converseEventEmitter.on( - // `openAttachmentForMessage-${message.id}`, - // clickedOnAttachmentBubble - // ); - // return () => { - // converseEventEmitter.off( - // `openAttachmentForMessage-${message.id}`, - // clickedOnAttachmentBubble - // ); - // }; - // }, [message.id, clickedOnAttachmentBubble]);components/ConversationListItem/ConversationListItemDumb.tsx (2)
238-246: Avoid unnecessary use ofuseSharedValuefor static layoutAt lines 237-246,
itemRectis initialized and updated but not used elsewhere in the code. This may be unnecessary.Consider removing
itemRectandonLayoutViewif they are not needed.
383-394: AdjustSwipeablehitSlop to improve touch areaThe
hitSlopprop inSwipeableis set to{ left: -6 }, which may not be sufficient for all devices.Consider increasing the
hitSlopvalue or adjusting it to enhance user experience on different screen sizes.components/Chat/Message/V3Message.tsx (2)
200-203: Address the TODO: Handlecontentwhen it's a stringThere is a TODO comment indicating that handling for when
contentis a string needs to be implemented. Currently, the code logs an error and returnsnull, which may not provide a great user experience.Would you like assistance in implementing the handling for
contentwhen it is a string? I can help create a solution that renders an appropriate message to the user.
378-382: Address the TODO: Render simple bubble message whenreplyMessageContentis a stringA TODO comment suggests rendering a simple bubble message when
replyMessageContentis a string. Currently, the code logs an error and returnsnull, which might not be the desired behavior.Would you like assistance in implementing a simple bubble message rendering for cases when
replyMessageContentis a string? I can help develop a solution to enhance user experience.__mocks__/@sentry/react-native.ts (1)
17-17: Prefer named exports over default exportTo maintain consistency and avoid potential issues with module imports, it's recommended to use named exports instead of a default export, especially when all the exports are already named.
Apply this diff to replace the default export with a named export:
- export default MockedSentry; + export { MockedSentry };🧰 Tools
🪛 GitHub Check: lint
[warning] 17-17:
Prefer named exportscomponents/Chat/Message/TextMessage.tsx (1)
10-12: Destructure props directly in function parameters for clarityInstead of destructuring
argsinside the function body, destructuring directly in the function parameters improves readability and follows common React patterns.Apply this diff:
- export const MessageText = memo(function MessageText(args: IMessageTextProps) { - const { children, inverted } = args; + export const MessageText = memo(function MessageText({ children, inverted }: IMessageTextProps) {components/StateHandlers/HydrationStateHandler.tsx (2)
3-3: Remove commented out import statementThe import statement on line 3 is commented out. If it is no longer needed, consider removing it to keep the code clean and maintainable.
23-25: Remove unnecessary commented out codeThe code block at lines 23-25 is commented out. Removing unused code helps in maintaining code clarity and reducing potential confusion for future developers.
components/Chat/Message/MessageContext.tsx (2)
15-17: Consider initializing context with default values instead of empty object.Casting an empty object to
IMessageContextTypecould lead to runtime errors if the context is accidentally used outside the provider.-const MessageContext = createContext<IMessageContextType>( - {} as IMessageContextType -); +const MessageContext = createContext<IMessageContextType>({ + showTime: () => {}, + hideTime: () => {}, + toggleTime: () => {}, + showTimeAV: { value: false } as SharedValue<boolean>, +});
4-9: Consider adding readonly modifier to the context type.Since these values shouldn't be mutated directly by consumers, marking them as readonly would provide better type safety.
type IMessageContextType = { - showTime: () => void; - hideTime: () => void; - toggleTime: () => void; - showTimeAV: SharedValue<boolean>; + readonly showTime: () => void; + readonly hideTime: () => void; + readonly toggleTime: () => void; + readonly showTimeAV: SharedValue<boolean>; };components/Chat/Message/MessageBubble.tsx (2)
15-15: Remove commented debug code.Remove the commented debug border style as it's not needed in production code.
- // ...debugBorder("red"),
37-60: Consider extracting static styles for better performance.The base style object could be moved outside the component to prevent recreation on each render.
+const getBaseStyle = (theme) => ({ + borderRadius: theme.borderRadius.sm, + paddingHorizontal: theme.spacing.xs, + paddingVertical: theme.spacing.xxs, +}); export const BubbleContentContainer = (args: IBubbleContentContainerProps) => { // ... const bubbleStyle = useMemo(() => { - const baseStyle = { - backgroundColor: fromMe - ? theme.colors.bubbles.bubble - : theme.colors.bubbles.received.bubble, - borderRadius: theme.borderRadius.sm, - paddingHorizontal: theme.spacing.xs, - paddingVertical: theme.spacing.xxs, - }; + const baseStyle = { + ...getBaseStyle(theme), + backgroundColor: fromMe + ? theme.colors.bubbles.bubble + : theme.colors.bubbles.received.bubble, + }; // ... rest of the code }, [fromMe, hasNextMessageInSeries, theme]);containers/GroupScreenConsentTable.tsx (2)
14-17: Props type definition needs null safety improvementsWhile the transition from interface to type is good, the current type definition could be improved to handle null/undefined cases more explicitly.
Consider this improvement:
type GroupScreenConsentTableProps = { topic: ConversationTopic; - group: GroupWithCodecsType | null | undefined; + group?: GroupWithCodecsType | null; };
Line range hint
34-43: Add error handling for async group name retrievalThe async operations for group name retrieval lack proper error handling, which could lead to undefined behavior if the group name cannot be retrieved.
Consider this improvement:
action: async () => { - const groupName = await group?.groupName(); + try { + if (!group) { + console.error('Group is not defined'); + return; + } + const groupName = await group.groupName(); + if (!groupName) { + console.error('Failed to retrieve group name'); + return; + } groupRemoveRestoreHandler( consent, colorScheme, groupName, allowGroup, blockGroup )((success: boolean) => { // If not successful, do nothing (user canceled) }); + } catch (error) { + console.error('Error retrieving group name:', error); + } },Also applies to: 53-62
containers/GroupScreenImage.tsx (1)
Line range hint
46-54: Enhance file upload error handling and add size validationThe current implementation of photo upload lacks proper error handling and file size validation.
Consider this improvement:
const onPhotoChange = useCallback( (newImageUrl: string) => { + const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB + try { + const fileInfo = await RNFS.stat(newImageUrl); + if (fileInfo.size > MAX_FILE_SIZE) { + Alert.alert('Error', 'Image size must be less than 5MB'); + return; + } uploadFile({ account: currentAccount, filePath: newImageUrl, contentType: "image/jpeg", - }).then((url) => { + }).then((url) => { setGroupPhoto(url); + }).catch((error) => { + console.error('Error uploading file:', error); + Alert.alert('Error', 'Failed to upload image'); }); + } catch (error) { + console.error('Error reading file:', error); + Alert.alert('Error', 'Failed to process image'); + } }, [currentAccount, setGroupPhoto] );containers/GroupScreenName.tsx (1)
Line range hint
48-54: Consider adding input validation for group nameWhile updating the error handling, it would be good to add input validation for the group name.
Consider this improvement:
const handleNameChange = useCallback(async () => { + const MIN_LENGTH = 3; + const MAX_LENGTH = 50; + if (editedName.length < MIN_LENGTH || editedName.length > MAX_LENGTH) { + Alert.alert('Invalid Name', `Group name must be between ${MIN_LENGTH} and ${MAX_LENGTH} characters`); + return; + } try { setEditing(false); await setGroupName(editedName); } catch (e) { logger.error(e); - Alert.alert("An error occurred"); + Alert.alert("Error", "Failed to update group name. Please try again."); } }, [editedName, setGroupName]);components/Chat/Attachment/SendAttachmentPreview.tsx (1)
41-54: Consider extracting style constantsWhile the inline styles are clear, the aspect ratio calculations could be extracted into named constants for better maintainability.
+const LANDSCAPE_ASPECT_RATIO = 1.33; +const PORTRAIT_ASPECT_RATIO = 0.75; <AnimatedVStack entering={theme.animation.reanimatedFadeInSpring} style={{ borderRadius: theme.borderRadius.sm, position: "relative", overflow: "hidden", ...(isLandscape ? { - aspectRatio: 1.33, + aspectRatio: LANDSCAPE_ASPECT_RATIO, } : { - aspectRatio: 0.75, + aspectRatio: PORTRAIT_ASPECT_RATIO, }), }} >components/Connecting.tsx (1)
Line range hint
79-100: Consider adding error details to Sentry trackingThe Sentry message could include more context about why the initial load is taking longer than expected.
-sentryTrackMessage("Initial load has been running for 15 seconds"); +sentryTrackMessage("Initial load has been running for 15 seconds", { + initialLoadDoneOnce, + timeElapsed: Date.now() - conditionTrueTime.current, +});components/ConversationListItem/ConversationListItem.tsx (1)
107-115: Well-structured compound component patternThe component follows best practices with:
- Proper separation of concerns
- Memoized sub-components
- Clear, consistent naming
Consider adding a README.md to document the component's usage patterns and composition examples.
components/Chat/Message/MessageSenderAvatar.tsx (1)
21-45: Well-structured component separationThe separation between dumb and smart components is well implemented:
MessageSenderAvatarDumbhandles pure renderingV3MessageSenderAvatarmanages data fetching and business logicConsider adding prop validation for required fields and error boundaries for data fetching failures.
Also applies to: 78-103
components/ClickableText.tsx (1)
Line range hint
29-73: Consider memoizing pattern array to optimize performance.The pattern array in the ParsedText component is recreated on every render. Consider memoizing it with useMemo to prevent unnecessary recreations.
+ const patterns = useMemo(() => [ + { + onPress: handleNewConversationPress, + onLongPress: showCopyActionSheet("Copy wallet address"), + pattern: ADDRESS_REGEX, + style: styles.clickableText, + }, + // ... rest of the patterns + ], [handleNewConversationPress, showCopyActionSheet, styles.clickableText]); return ( <ParsedText accessibilityRole="link" style={style} - parse={[...]} + parse={patterns} >components/Onboarding/ConnectViaWallet/useInitConnectViaWalletState.ts (1)
Line range hint
63-102: Consider adding cleanup for initialization state.The initialization state might be left in an inconsistent state if the component unmounts during initialization.
useEffect(() => { + let mounted = true; if (!thirdwebSigner) { logger.debug("[Connect Wallet] No thirdweb signer available yet"); return; } // ... rest of the code const initializeWallet = async () => { try { // ... initialization code + if (!mounted) return; setOnXmtp(isOnNetwork); setAlreadyV3Db(hasV3); setSigner(thirdwebSigner); } catch (error) { // ... error handling } finally { + if (mounted) { hasInitizalizedRef.current = true; isInitializingRef.current = false; setIsInitializing(false); + } } }; initializeWallet(); + return () => { + mounted = false; + }; }, [dependencies]);containers/GroupPendingRequestsTable.tsx (1)
Line range hint
38-93: Consider extracting action sheet logic to a separate hook.The action sheet logic could be extracted to improve reusability and reduce component complexity.
+ const useRequestActionSheet = ( + colorScheme: string, + currentAccount: string, + addToGroup: (addresses: string[]) => Promise<void> + ) => { + return useCallback( + (address: string, preferredName: string, requestId: string) => { + const options = [ + translate("approve"), + translate("deny"), + translate("cancel"), + ]; + // ... rest of the action sheet logic + }, + [colorScheme, currentAccount, addToGroup] + ); + }; export const GroupPendingRequestsTable: FC<GroupPendingRequestsTableProps> = ({ topic, }) => { // ... existing code + const showRequestActionSheet = useRequestActionSheet( + colorScheme, + currentAccount, + addToGroup + ); const tableViewItems = useMemo(() => { const items: TableViewItemType[] = []; requests.forEach((a, id) => { // ... existing code - action: () => { ... }, + action: () => showRequestActionSheet(address, preferredName, request.id), }); return items; }, [/* dependencies */]);components/ConversationFlashList.tsx (2)
33-38: Enhance keyExtractor robustnessThe current implementation could be more explicit about the types it's handling.
Consider this more type-safe approach:
-const keyExtractor = (item: FlatListItemType) => { - if ("lastMessage" in item) { - return item.topic; - } - return typeof item === "string" ? item : item.topic + "v2"; -}; +const keyExtractor = (item: FlatListItemType) => { + if ("lastMessage" in item) { + return `conversation-${item.topic}`; + } + if ("topic" in item && item.topic === "hiddenRequestsButton") { + return "hidden-requests"; + } + return `unknown-${JSON.stringify(item)}`; +};
62-85: Simplify renderItem implementationThe renderItem function could be simplified using early returns and explicit type checks.
Consider this more readable approach:
-const renderItem = useCallback(({ item }: { item: FlatListItemType }) => { - if ("lastMessage" in item) { - const conversation = unwrapConversationContainer(item); - if (conversation.version === ConversationVersion.GROUP) { - return ( - <V3GroupConversationListItem - group={conversation as GroupWithCodecsType} - /> - ); - } else { - return <V3DMListItem conversation={conversation as DmWithCodecsType} />; - } - } - if (item.topic === "hiddenRequestsButton") { - const hiddenRequestItem = item as ConversationFlatListHiddenRequestItem; - return ( - <HiddenRequestsButton - spamCount={hiddenRequestItem.spamCount} - toggleActivated={hiddenRequestItem.toggleActivated} - /> - ); - } - return null; -}, []); +const renderItem = useCallback(({ item }: { item: FlatListItemType }) => { + // Handle conversation items + if ("lastMessage" in item) { + const conversation = unwrapConversationContainer(item); + return conversation.version === ConversationVersion.GROUP + ? <V3GroupConversationListItem group={conversation as GroupWithCodecsType} /> + : <V3DMListItem conversation={conversation as DmWithCodecsType} />; + } + + // Handle hidden requests button + if ("topic" in item && item.topic === "hiddenRequestsButton") { + const { spamCount, toggleActivated } = item as ConversationFlatListHiddenRequestItem; + return <HiddenRequestsButton spamCount={spamCount} toggleActivated={toggleActivated} />; + } + + return null; +}, []);components/AccountSettingsButton.tsx (1)
Line range hint
78-93: Consider enhancing notification permission UXThe notification permission handling logic could benefit from more user feedback and clearer state management.
Consider adding user feedback for permission states:
if (Platform.OS === "android") { + // Show loading state requestPushNotificationsPermissions().then( (newStatus: NotificationPermissionStatus | undefined) => { if (newStatus === "denied") { + // Show explanation dialog before opening settings Linking.openSettings(); } else if (newStatus) { setNotificationsPermissionStatus(newStatus); + // Show success message } } );components/ConversationContextMenu.tsx (1)
1-6: Consider trade-offs of tight coupling with storeWhile using hooks improves encapsulation, the component is now tightly coupled to the store implementation. Consider using a custom hook to abstract the store implementation details, making it easier to change the state management solution in the future.
components/Chat/Message/MessageContextMenu.tsx (1)
Line range hint
81-166: Consider extracting position calculation logicThe positioning calculations for menu, auxiliary view, and portal are complex and repeated across multiple functions. Consider extracting these calculations into shared utility functions to improve maintainability and reduce code duplication.
Example refactor:
+ const calculateVerticalPosition = ( + itemRectY: number, + itemRectHeight: number, + menuHeight: number, + screenHeight: number, + safeAreaInsets: { bottom: number; top: number }, + auxiliaryViewMinHeight: number + ) => { + // ... shared positioning logic ... + }; const animatedMenuStyle = useAnimatedStyle(() => { - const getTransformValue = () => { - // ... current positioning logic ... - }; + const tY = calculateVerticalPosition( + itemRectY.value, + itemRectHeight.value, + menuHeight, + height, + safeAreaInsets, + AUXILIARY_VIEW_MIN_HEIGHT + ); // ... rest of the animation style ... });Also applies to: 227-258
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (5)
components/__tests__/__snapshots__/Button.test.tsx.snapis excluded by!**/*.snapdesign-system/Text/__tests__/__snapshots__/Text.test.tsx.snapis excluded by!**/*.snapios/Gemfile.lockis excluded by!**/*.lockios/Podfile.lockis excluded by!**/*.lockqueries/__snapshots__/QueryKeys.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (82)
App.tsx(1 hunks)__mocks__/@sentry/react-native.ts(1 hunks)components/AccountSettingsButton.tsx(1 hunks)components/Chat/Attachment/AddAttachmentButton.tsx(5 hunks)components/Chat/Attachment/AttachmentMessagePreview.tsx(1 hunks)components/Chat/Attachment/SendAttachmentPreview.tsx(1 hunks)components/Chat/Chat.tsx(1 hunks)components/Chat/ChatDumb.tsx(1 hunks)components/Chat/ChatGroupUpdatedMessage.tsx(1 hunks)components/Chat/ChatPlaceholder/ChatPlaceholder.tsx(1 hunks)components/Chat/ChatPlaceholder/GroupChatPlaceholder.tsx(0 hunks)components/Chat/ConsentPopup/ConsentPopup.tsx(4 hunks)components/Chat/ConsentPopup/GroupConsentPopup.tsx(2 hunks)components/Chat/Frame/FrameBottom.tsx(6 hunks)components/Chat/Frame/FramePreview.tsx(1 hunks)components/Chat/Frame/FramesPreviews.tsx(1 hunks)components/Chat/Input/Input.tsx(0 hunks)components/Chat/Input/InputReplyBubble.tsx(0 hunks)components/Chat/Input/InputReplyPreview.tsx(0 hunks)components/Chat/Input/SendButton.tsx(0 hunks)components/Chat/Message/Message.tsx(2 hunks)components/Chat/Message/Message.utils.tsx(1 hunks)components/Chat/Message/MessageActions.tsx(1 hunks)components/Chat/Message/MessageBubble.tsx(1 hunks)components/Chat/Message/MessageContext.tsx(1 hunks)components/Chat/Message/MessageContextMenu.tsx(7 hunks)components/Chat/Message/MessageSender.tsx(1 hunks)components/Chat/Message/MessageSenderAvatar.tsx(1 hunks)components/Chat/Message/MessageStatusDumb.tsx(1 hunks)components/Chat/Message/RepliableMessageWrapper.tsx(1 hunks)components/Chat/Message/TextMessage.tsx(1 hunks)components/Chat/Message/V3Message.tsx(1 hunks)components/Chat/Message/messageContextStore.tsx(1 hunks)components/Chat/Transaction/TransactionInput.tsx(0 hunks)components/Chat/Transaction/TransactionPreview.tsx(2 hunks)components/ClickableText.tsx(3 hunks)components/Connecting.tsx(3 hunks)components/Conversation/ConversationTitle.tsx(0 hunks)components/Conversation/ConversationTitleDumb.tsx(1 hunks)components/Conversation/V3Conversation.tsx(1 hunks)components/Conversation/V3ConversationHeader.tsx(1 hunks)components/ConversationContextMenu.tsx(5 hunks)components/ConversationFlashList.tsx(2 hunks)components/ConversationList/GroupConversationItem.tsx(0 hunks)components/ConversationListItem.tsx(0 hunks)components/ConversationListItem/ConversationListItem.tsx(1 hunks)components/ConversationListItem/ConversationListItemDumb.tsx(1 hunks)components/DebugButton.tsx(1 hunks)components/GroupAvatar.tsx(4 hunks)components/Onboarding/ConnectViaWallet/useConnectViaWalletInitXmtpClient.tsx(3 hunks)components/Onboarding/ConnectViaWallet/useInitConnectViaWalletState.ts(2 hunks)components/Onboarding/init-xmtp-client.ts(4 hunks)components/ParsedText/ParsedText.props.ts(0 hunks)components/ParsedText/ParsedText.tsx(1 hunks)components/ParsedText/ParsedText.types.ts(1 hunks)components/ParsedText/ParsedText.utils.ts(1 hunks)components/PinnedConversations/PinnedConversation.tsx(1 hunks)components/PinnedConversations/PinnedConversations.tsx(1 hunks)components/PinnedConversations/PinnedV3Conversation.tsx(1 hunks)components/PinnedConversations/PinnedV3DMConversation.tsx(1 hunks)components/PinnedConversations/PinnedV3GroupConversation.tsx(1 hunks)components/PressableProfileWithText.tsx(0 hunks)components/Recommendations/Recommendation.tsx(1 hunks)components/StateHandlers/HydrationStateHandler.tsx(2 hunks)components/StateHandlers/InitialStateHandler.tsx(2 hunks)components/StateHandlers/MainIdentityStateHandler.tsx(2 hunks)components/StateHandlers/NotificationsStateHandler.tsx(0 hunks)components/TransactionPreview/TransactionContent.tsx(1 hunks)components/TransactionPreview/TransactionPreview.tsx(2 hunks)components/V3DMListItem.tsx(1 hunks)components/V3GroupConversationListItem.tsx(1 hunks)components/XmtpEngine.tsx(2 hunks)components/__tests__/PressableProfileWithText.perf-test.tsx(0 hunks)config.ts(3 hunks)containers/GroupPendingRequestsTable.tsx(2 hunks)containers/GroupScreenAddition.tsx(11 hunks)containers/GroupScreenConsentTable.tsx(4 hunks)containers/GroupScreenDescription.tsx(2 hunks)containers/GroupScreenImage.tsx(1 hunks)containers/GroupScreenMembersTable.tsx(1 hunks)containers/GroupScreenName.tsx(3 hunks)data/db/datasource.ts(0 hunks)
⛔ Files not processed due to max files limit (51)
- data/db/driver.ts
- data/db/entities/conversationEntity.ts
- data/db/entities/messageEntity.ts
- data/db/index.ts
- data/db/logger.ts
- data/db/migrations/1671623489366-init.ts
- data/db/migrations/1671788934503-addLensHandle.ts
- data/db/migrations/1673277126468-addEnsName.ts
- data/db/migrations/1680616920220-addMessageStatus.ts
- data/db/migrations/1681209069007-addStatusIndex.ts
- data/db/migrations/1683114681319-addReadStatus.ts
- data/db/migrations/1684057254703-addSentViaConverse.ts
- data/db/migrations/1686053217007-addProfileEntity.ts
- data/db/migrations/1686124833536-removeHandlesFromConvo.ts
- data/db/migrations/1687793816866-addContentType.ts
- data/db/migrations/1688549487960-addMessageReaction.ts
- data/db/migrations/1690204801962-addMessageFallback.ts
- data/db/migrations/1690376359971-addPendingStateToConversations.ts
- data/db/migrations/1690809735000-fixWrongForeignKey.ts
- data/db/migrations/1690989046000-removeForeignKeyForTesters.ts
- data/db/migrations/1691154310694-addIndexToPendingConversation.ts
- data/db/migrations/1691397563214-addReferencedMessage.ts
- data/db/migrations/1691412759130-removeOldReactions.ts
- data/db/migrations/1695029413899-addVersionToConversation.ts
- data/db/migrations/1698068091873-addSpamScore.ts
- data/db/migrations/1708332276329-addGroupConversations.ts
- data/db/migrations/1709030178271-addConverseMessageMetadata.ts
- data/db/migrations/1709893391562-addLastNotifSubscribePeriodToConversation.ts
- data/db/migrations/1712656017130-addIndexToSent.ts
- data/db/migrations/1717625558678-RemoveSentViaConverse.ts
- data/db/migrations/1717631723249-AddSuperAdmin.ts
- data/db/migrations/1721143963530-addIsActive.ts
- data/db/migrations/1726807503232-AddGroupCreatorAndAddedBy.ts
- data/db/migrations/1726828413530-removeProfileDb.ts
- data/db/upsert.ts
- data/helpers/conversations/pendingConversations.ts
- data/helpers/conversations/spamScore.ts
- data/helpers/conversations/upsertConversations.ts
- data/helpers/inboxId/saveInboxIds.ts
- data/helpers/messages/getMessagesToSend.ts
- data/helpers/messages/handleGroupUpdatedMessage.test.ts
- data/helpers/messages/handleGroupUpdatedMessage.ts
- data/helpers/messages/index.ts
- data/helpers/profiles/profilesUpdate.ts
- data/index.ts
- data/mappers.ts
- data/store/accountsStore.ts
- data/store/chatStore.ts
- data/store/inboxIdStore.ts
- data/store/profilesStore.ts
- data/store/settingsStore.ts
💤 Files with no reviewable changes (14)
- components/ParsedText/ParsedText.props.ts
- data/db/datasource.ts
- components/Chat/Transaction/TransactionInput.tsx
- components/StateHandlers/NotificationsStateHandler.tsx
- components/ConversationListItem.tsx
- components/PressableProfileWithText.tsx
- components/Chat/Input/InputReplyBubble.tsx
- components/tests/PressableProfileWithText.perf-test.tsx
- components/Chat/Input/SendButton.tsx
- components/Chat/ChatPlaceholder/GroupChatPlaceholder.tsx
- components/Chat/Input/Input.tsx
- components/ConversationList/GroupConversationItem.tsx
- components/Conversation/ConversationTitle.tsx
- components/Chat/Input/InputReplyPreview.tsx
✅ Files skipped from review due to trivial changes (10)
- components/Conversation/V3ConversationHeader.tsx
- components/TransactionPreview/TransactionContent.tsx
- components/Chat/Chat.tsx
- components/StateHandlers/MainIdentityStateHandler.tsx
- components/Chat/ChatPlaceholder/ChatPlaceholder.tsx
- components/Recommendations/Recommendation.tsx
- components/Chat/Message/MessageActions.tsx
- components/Chat/Message/Message.tsx
- components/Chat/Frame/FramesPreviews.tsx
- components/Chat/Transaction/TransactionPreview.tsx
🧰 Additional context used
🪛 GitHub Check: lint
__mocks__/@sentry/react-native.ts
[warning] 17-17:
Prefer named exports
🪛 Biome (1.9.4)
components/Chat/Attachment/AddAttachmentButton.tsx
[error] 153-153: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
components/Chat/Message/V3Message.tsx
[error] 273-273: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
🪛 GitHub Check: tsc
components/Conversation/V3Conversation.tsx
[failure] 178-178:
Type '{ id?: string | SharedValue<string | undefined> | undefined; debug?: boolean | SharedValue<boolean | undefined> | undefined; data: ArrayLike | SharedValue<...> | null | undefined; ... 182 more ...; renderItem: ListRenderItem<...>; }' is not assignable to type 'ReanimatedFlatListPropsWithLayout'.
🔇 Additional comments (55)
components/TransactionPreview/TransactionPreview.tsx (2)
Line range hint 189-225: LGTM: Clean and well-structured UI composition
The UI composition using TransactionHeader, TransactionContent, and TransactionActions components is clean and follows good separation of concerns.
2-2: Verify profile refresh functionality removal
The removal of refreshProfileForAddress functionality might impact profile data synchronization after transactions. Please confirm if this is intentional and whether profile updates are handled elsewhere in the codebase.
Also applies to: 100-100
✅ Verification successful
The initial results show multiple files using profile refresh functionality. Let's examine the actual implementations and recent changes to understand if this is part of a broader refactoring or migration.
Profile refresh functionality has been systematically commented out across the codebase
The verification reveals that refreshProfileForAddress has been intentionally commented out across multiple components as part of a systematic change:
- ConversationList.tsx: Commented out during initial login
- OnboardingUserProfileScreen.tsx: Commented out after profile updates
- MainIdentityStateHandler.tsx: Commented out during user address handling
- HydrationStateHandler.tsx: Commented out during account hydration
- TransactionPreview.tsx: Commented out after transaction simulation
- AccountSettingsButton.tsx: Commented out in profile navigation
This appears to be an intentional architectural change rather than an oversight, as the pattern is consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other profile refresh implementations
echo "Searching for profile refresh implementations..."
rg -l "refreshProfileForAddress|updateProfile|syncProfile"
# Search for related changes in the codebase
echo "Checking for related changes..."
ast-grep --pattern 'import { refreshProfileForAddress } from "@data/helpers/profiles/profilesUpdate"'
Length of output: 703
Script:
#!/bin/bash
# Let's check the actual implementation and usage of profile refresh functions
echo "Checking profile refresh implementations in related files..."
rg "refreshProfileForAddress|updateProfile|syncProfile" -A 5 -B 2
# Let's also check if there are any recent changes or migrations related to profile updates
echo "Checking git history for profile-related changes..."
git log --oneline -n 10 | grep -i "profile"
Length of output: 8530
components/Chat/Message/MessageSender.tsx (3)
1-10: LGTM! Imports are well-organized and relevant.
All imports are being used and follow consistent patterns.
38-45: Verify usePreferredInboxName hook behavior.
Let's ensure the hook handles all possible InboxId states correctly.
✅ Verification successful
Hook implementation is well-tested and handles all edge cases appropriately
The usePreferredInboxName hook and its underlying utility function getPreferredInboxName are thoroughly tested and handle all possible states:
- Null/undefined/empty inputs return empty string
- Prioritizes display names in order: username > ENS > unstoppable domains
- Falls back to shortened wallet address if no other identifiers exist
- Properly handles the undefined InboxId prop passed to the hook
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usePreferredInboxName hook implementation and its tests
ast-grep --pattern 'export function usePreferredInboxName($_) {
$$$
}'
# Search for related test files
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts usePreferredInboxName
Length of output: 147
Script:
#!/bin/bash
# Let's try a broader search for the hook implementation
rg -l "usePreferredInboxName"
# Also search for any usage of this hook
ast-grep --pattern 'usePreferredInboxName($$$)'
# Search for any test files containing "inbox" in their name
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts inbox
Length of output: 905
Script:
#!/bin/bash
# Let's examine the hook implementation
cat hooks/usePreferredInboxName.ts
# Check the related test file
cat utils/profile/__tests__/getPreferredInboxName.test.ts
# Look for the underlying utility function
cat utils/profile/getPreferredInboxName.ts
Length of output: 2974
47-62: Review flexBasis usage for potential layout issues.
The flexBasis: "100%" combined with flexDirection: "row" might cause unexpected wrapping or overflow in constrained containers.
Consider testing the layout in different screen sizes and container widths. You might want to:
- Verify if
flexBasis: "100%"is necessary - Test how it behaves with long sender names
- Ensure it doesn't break the parent container's layout
components/Conversation/ConversationTitleDumb.tsx (1)
1-11: LGTM! Well-organized imports
The imports are logically grouped and all are utilized within the component.
components/Chat/Message/messageContextStore.tsx (1)
44-51: Use of default values in DEFAULT_PROPS.
Assigning default values like empty strings for IDs and zero for timestamps may lead to unintended behavior if these defaults are not valid in the application context.
Please verify that these default values are appropriate. If not, consider using null or undefined where applicable, or enforce that all required props are provided.
Example adjustment:
const DEFAULT_PROPS: IMessageContextStoreProps = {
- messageId: "" as MessageId,
+ messageId: null as MessageId,
// other props...
};Run the following script to check where DEFAULT_PROPS are used without overrides:
✅ Verification successful
Empty string is a valid default for messageId, keep current implementation
The codebase shows that:
messageIdis optional in several places (e.g., in event handlers and notifications)- There's explicit handling for empty messageId strings (
messageId?.trim()check in notifications) createMessageContextStoreis always called with props from the provider component- The default values are immediately overridden by
...initProps
The current implementation using empty string as default is appropriate as it:
- Maintains type safety (vs null/undefined)
- Aligns with how the codebase handles message IDs
- Doesn't cause issues since these defaults are always overridden with actual props
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find instances where `createMessageContextStore` is called without initProps.
rg -A 2 "createMessageContextStore\(\s*\)"
Length of output: 44
Script:
#!/bin/bash
# Let's first find all usages of createMessageContextStore with any arguments
rg "createMessageContextStore" -A 3
# Also check the type definition and implementation
ast-grep --pattern 'interface IMessageContextStoreProps {
$$$
}'
# And check where DEFAULT_PROPS is actually used
rg "DEFAULT_PROPS" -A 3
Length of output: 2619
Script:
#!/bin/bash
# Let's check how MessageId type is defined and used
rg "type MessageId" -A 1
# Also check if there are any type checks or validations for these props
ast-grep --pattern 'if ($_ === "") {
$$$
}'
# And check for any null checks on these properties
rg "messageId(\s+===|\s*\?|===\s+)" -A 2
Length of output: 1895
components/Chat/ConsentPopup/ConsentPopup.tsx (1)
39-39: Confirm the updated logic for displaying the consent window
The condition for showing the consent window has changed to thisPeerStatus !== "allowed". Ensure that this logic aligns with the desired behavior and that all possible thisPeerStatus values are appropriately handled.
Run the following script to list all occurrences of thisPeerStatus in the codebase to verify its possible values:
✅ Verification successful
The updated consent window logic is correct and properly handles all status values
Based on the codebase analysis:
thisPeerStatuscomes fromconversation.state- The type definitions in
data/store/settingsStore.tsshow that status values can be either"allowed"or"denied" - The condition
thisPeerStatus !== "allowed"correctly shows the consent window for any non-allowed state (including"denied"and initial undefined states)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of `thisPeerStatus` to identify its possible values.
# Test: List assignments to `thisPeerStatus`. Expect: Understand all possible statuses.
rg --multiline '$thisPeerStatus\s*=\s*.*' --glob '**/*.tsx'
Length of output: 61
Script:
#!/bin/bash
# Let's try a different approach to find usages and assignments of thisPeerStatus
rg "thisPeerStatus" --type tsx -B 2 -A 2
# Also search for any status-related constants or types that might define the possible values
rg "status.*=.*allowed" --type tsx -B 2 -A 2
Length of output: 143
Script:
#!/bin/bash
# Search for thisPeerStatus without file type restriction
rg "thisPeerStatus" -B 2 -A 2
# Look for type definitions or enums that might define the status values
rg "type.*Status.*=.*allowed" -B 2 -A 2
# Search for any constants or variables with "allowed" status
rg "allowed" --type ts --type tsx -B 2 -A 2
Length of output: 1293
components/Chat/ConsentPopup/GroupConsentPopup.tsx (6)
8-9: LGTM!
The import statements correctly include necessary modules.
10-10: LGTM!
Importing GroupWithCodecsType is appropriate for typing the group prop.
17-17: LGTM!
The component now accepts the group prop, improving its reusability.
24-25: LGTM!
Properly extracting topic and isAllowed from the group prop.
29-29: Ensure topic is always defined when using useGroupConsent
If there's a possibility that group.topic could be undefined, consider adding a null check to prevent runtime errors.
32-32: LGTM!
The logic for displaying the consent window is correctly updated.
components/PinnedConversations/PinnedConversations.tsx (1)
11-11: Verify impact of changing export from default to named export
You've changed the export from a default export to a named export. Ensure that all imports of PinnedConversations in the codebase have been updated accordingly to prevent import errors.
Run the following script to identify files importing PinnedConversations as a default export:
components/PinnedConversations/PinnedV3GroupConversation.tsx (1)
93-97: Optimize onLongPress handler with useCallback
The onLongPress handler doesn't depend on any changing values except for contextMenuItems and group.topic. You've already wrapped it with useCallback, which is good. Ensure that all dependencies are correctly specified to prevent unnecessary re-renders.
components/Onboarding/init-xmtp-client.ts (2)
76-84: Error handling in connectWithAddress is appropriate
The function connectWithAddress correctly handles errors by logging them and displaying an alert to the user. The use of await ensures proper asynchronous flow.
33-40: Initialization of XMTP client is correctly implemented
The XMTP client is initialized using createXmtpClientFromSigner, and the callback function handles potential revocation appropriately by alerting the user and throwing an error.
components/Onboarding/ConnectViaWallet/useConnectViaWalletInitXmtpClient.tsx (4)
11-12: Updated Imports Align with New XMTP Client Initialization
The imports for createXmtpClientFromSigner and connectWithAddress correctly reflect the updated methods for XMTP client initialization, ensuring consistency with the new implementation.
114-116: Connection Established Using Address
The connectWithAddress function is appropriately invoked with the user's address, aligning with the updated connection strategy that removes dependency on a base64 key.
Line range hint 129-141: Ensure Proper State Reset in the finally Block
The finally block resets various state variables in connectViewWalletStore after the connection process completes. Confirm that resetting these states is appropriate in all execution paths, including success, error, or early termination, to prevent unintended side effects or stale state in subsequent operations.
Line range hint 72-101: Verify Asynchronous State Management in Callbacks
The callbacks provided to createXmtpClientFromSigner manipulate the connectViewWalletStore state and involve asynchronous operations like waitForClickSignature. Ensure that the state updates (setNumberOfSignaturesDone, setLoading, setWaitingForNextSignature) are correctly synchronized to prevent potential race conditions or state inconsistencies due to asynchronous execution.
To confirm proper state management, you can check for possible race conditions with the following script:
✅ Verification successful
State Management in Callbacks is Properly Synchronized
The code implements a well-structured state management approach with proper synchronization:
- The
waitForClickSignaturefunction uses a polling mechanism with proper abort control to wait for user signatures. - State updates are sequential and properly coordinated:
- Loading state is toggled appropriately before and after operations
- Signature count is incremented only after confirming the current state
- State is properly reset to initial values when needed
- The implementation includes proper error handling and cleanup in the finally block
The asynchronous operations are properly coordinated through the state machine pattern, with clear state transitions and proper waiting mechanisms. No race conditions were identified in the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for asynchronous state updates that may lead to race conditions.
# Find all instances where state is updated within asynchronous callbacks.
rg --multiline '<[A-Za-z]+>\s*\((async )?\(\)\s*=>\s*{(\n|.)*?connectViewWalletStore.getState\(\)\.(set[A-Za-z]+)\('
Length of output: 338
Script:
#!/bin/bash
# Search for state updates within async callbacks using a corrected pattern
rg -U 'async.*\{[\s\S]*?connectViewWalletStore\.getState\(\)\.set[A-Za-z]+\(' --type ts --type tsx
# Search for all state updates in the store to understand the state management pattern
rg 'connectViewWalletStore\.getState\(\)\.set[A-Za-z]+' --type ts --type tsx -B 2 -A 2
# Look for the store definition to understand its structure
rg -l 'connectViewWalletStore.*=.*create' --type ts --type tsx
Length of output: 338
Script:
#!/bin/bash
# Search for state updates within async callbacks without file type restrictions
rg -U 'async.*\{[\s\S]*?connectViewWalletStore\.getState\(\)\.set[A-Za-z]+\('
# Search for all state updates in the store to understand the state management pattern
rg 'connectViewWalletStore\.getState\(\)\.set[A-Za-z]+' -B 2 -A 2
# Look for the store definition to understand its structure
rg -l 'connectViewWalletStore.*=.*create'
# Search for waitForClickSignature usage
rg 'waitForClickSignature' -B 2 -A 2
Length of output: 10725
components/ParsedText/ParsedText.types.ts (1)
1-27: LGTM!
The type definitions are well-structured and enhance type safety for parsing text patterns. Good use of TypeScript's type system to create flexible and extensible types.
components/Chat/Message/MessageContext.tsx (1)
19-44: LGTM! Well-structured provider with proper memoization.
The implementation correctly uses:
- useCallback for event handlers
- useMemo for context value
- proper dependency arrays
components/Chat/Message/MessageBubble.tsx (1)
5-25: LGTM! Well-structured components with proper type safety and theming.
Both components are well implemented with:
- Proper TypeScript types
- Theme integration
- Conditional styling based on message series
Also applies to: 27-63
containers/GroupScreenConsentTable.tsx (1)
69-69: Verify dependency array completeness
The useMemo dependency array has been updated to include group instead of groupName, which is correct. However, ensure all dependencies are properly tracked.
✅ Verification successful
Let me analyze the code to check if we need any additional dependencies in the useMemo hook.
Let me check how translate is imported and if it's a dependency that needs to be tracked.
Dependencies in useMemo are correctly specified
The dependency array [consent, allowGroup, blockGroup, colorScheme, group] is complete and accurate for the useMemo hook. Here's why:
- The callback uses
consentfor conditional rendering allowGroupandblockGroupare used in the handler callscolorSchemeis used for bothprimaryColoranddangerColorcallsgroupis used to get thegroupNametranslateis imported directly from "@i18n" and is not a hook, so it doesn't need to be in dependenciesgroupRemoveRestoreHandlerappears to be defined outside the component or passed as a prop
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential missing dependencies in useMemo/useEffect hooks
ast-grep --pattern 'useMemo(() => {
$$$
}, [$$$])' containers/GroupScreenConsentTable.tsx
Length of output: 3229
Script:
#!/bin/bash
# Check for translate usage and its import
ast-grep --pattern 'translate($$$)' containers/GroupScreenConsentTable.tsx
# Check for color utility functions
ast-grep --pattern 'primaryColor($$$)' containers/GroupScreenConsentTable.tsx
ast-grep --pattern 'dangerColor($$$)' containers/GroupScreenConsentTable.tsx
# Check for groupRemoveRestoreHandler
ast-grep --pattern 'const groupRemoveRestoreHandler =' containers/GroupScreenConsentTable.tsx
Length of output: 760
Script:
#!/bin/bash
# Check imports at the top of the file
rg -U "import.*translate" containers/GroupScreenConsentTable.tsx
# Check if translate is from a hook
ast-grep --pattern 'const translate = use$$$' containers/GroupScreenConsentTable.tsx
Length of output: 188
containers/GroupScreenImage.tsx (1)
20-22: Props type definition looks good
The transition to using ConversationTopic type improves type safety.
containers/GroupScreenName.tsx (2)
24-26: Props type definition looks good
The transition to using ConversationTopic type improves type safety.
11-11: Verify consistent ConversationTopic usage across codebase
Let's ensure the ConversationTopic type is consistently used across all related components.
Also applies to: 15-15, 13-13
✅ Verification successful
Note: Adjusted the file type flags to explicitly use --type ts and --type tsx for TypeScript files.
No inconsistencies found in ConversationTopic usage
Based on the codebase analysis, the ConversationTopic type from @xmtp/react-native-sdk is consistently imported and used across all relevant components. The review comment's concern about verifying consistent ConversationTopic usage has been addressed, as there are no instances of using string type for topics instead of ConversationTopic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining string types for topics
rg -l "topic:\s*string" --type typescript
# Check for consistent ConversationTopic import
rg -l "import.*ConversationTopic.*from.*@xmtp/react-native-sdk" --type typescript
Length of output: 197
Script:
#!/bin/bash
# Check for any remaining string types for topics
rg -l "topic:\s*string" --type ts --type tsx
# Check for consistent ConversationTopic import
rg -l "import.*ConversationTopic.*from.*@xmtp/react-native-sdk" --type ts --type tsx
# Check for any ConversationTopic usage
rg "ConversationTopic" --type ts --type tsx
Length of output: 261
Script:
#!/bin/bash
# First, let's find all TypeScript/React files
fd "\.(ts|tsx)$"
# Then search for topic-related patterns in all files
rg "topic.*string"
rg "ConversationTopic"
rg "import.*from.*@xmtp/react-native-sdk"
Length of output: 60680
components/Chat/Attachment/SendAttachmentPreview.tsx (2)
9-11: LGTM: Good use of design system components
The migration to design system components (IconButton, AnimatedVStack, VStack) improves consistency and maintainability.
70-88: Verify loading state accessibility
The loading state uses a semi-transparent overlay. Ensure this provides sufficient contrast for accessibility.
✅ Verification successful
Loading overlay follows design system patterns
The semi-transparent overlay using rgba(0, 0, 0, 0.5) is consistent with the app's design system, as evidenced by:
- It matches the
scrimcolor defined in the theme palette - Similar overlay patterns are used in other components like Drawer and ConversationContextMenu
- The contrast ratio of white ActivityIndicator (hex #ffffff) on a 50% black background meets WCAG accessibility guidelines
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar loading overlays to verify consistent accessibility patterns
rg -l "rgba\(0,\s*0,\s*0,\s*0\.5\)" --type tsx
Length of output: 76
Script:
#!/bin/bash
# Search for similar loading overlays with rgba pattern, checking all file types
rg -l "rgba\(0,\s*0,\s*0,\s*0\.5\)"
# Also search for other loading indicators to check accessibility patterns
rg -l "ActivityIndicator"
Length of output: 1384
Script:
#!/bin/bash
# Let's check the implementation of ActivityIndicator component to verify accessibility patterns
rg -A 10 "ActivityIndicator" components/ActivityIndicator/ActivityIndicator.tsx
# Check other instances of rgba overlay usage for consistency
rg -A 5 "rgba\(0,\s*0,\s*0,\s*0\.5\)" components/Drawer.tsx components/ConversationContextMenu.tsx theme/palette.ts
Length of output: 1161
containers/GroupScreenDescription.tsx (1)
13-13: Verify ConversationTopic usage across codebase
The type change from string to ConversationTopic improves type safety. However, we should verify all call sites are updated.
Also applies to: 24-26
✅ Verification successful
Based on the search results, I can now generate the final response:
Type change from string to ConversationTopic is correctly propagated
The type change from string to ConversationTopic in GroupScreenDescription.tsx is properly propagated throughout the codebase. All components and functions that interact with the topic prop are using the ConversationTopic type consistently, including:
- Parent components passing the topic prop
- Related group screen components (GroupScreenName, GroupScreenImage, etc.)
- Hooks and queries consuming the topic
- Navigation and state management code
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components passing topic prop to GroupScreenDescription
ast-grep --pattern 'GroupScreenDescription $props'
# Search for other components using ConversationTopic
rg "topic:\s*(string|ConversationTopic)" --type typescript
Length of output: 148
Script:
#!/bin/bash
# Search for components passing topic prop to GroupScreenDescription
ast-grep --pattern '<GroupScreenDescription $$$>'
# Search for files importing and using ConversationTopic
rg "ConversationTopic"
# Search for topic prop usage
rg "topic[=:]" -A 2
Length of output: 65633
components/Connecting.tsx (1)
69-71: LGTM: Simplified connection state logic
Good simplification by removing unnecessary dependency on conversations state and focusing only on initialLoadDoneOnce.
Also applies to: 102-102
components/Chat/Frame/FrameBottom.tsx (3)
14-14: LGTM: Improved type safety with boolean prop
The change from message object to messageFromMe boolean simplifies the interface and makes the component's requirements more explicit.
Also applies to: 24-24
57-57: Consistent prop usage across child components
The messageFromMe prop is correctly propagated to child components FrameTextInput, FrameButton, and used in text styling.
Also applies to: 74-74, 84-84
41-44: Verify theme consistency across color schemes
The background color logic uses the new messageFromMe prop correctly, but we should ensure consistent contrast ratios across both light and dark themes.
✅ Verification successful
Color scheme implementation verified with appropriate contrast
The color implementation shows proper contrast handling:
- Light theme:
MESSAGE_INNER_BUBBLE_LIGHTusesrgba(255, 255, 255, 0.6)for regular messages andMY_MESSAGE_INNER_BUBBLE_LIGHTusesrgba(255, 255, 255, 0.3)for user messages - Dark theme:
MESSAGE_INNER_BUBBLE_DARKusesrgba(235, 235, 245, 0.3)for regular messages andMY_MESSAGE_INNER_BUBBLE_DARKusesrgba(0, 0, 0, 0.08)for user messages - Both themes maintain appropriate opacity levels for readability
- The implementation also handles platform-specific colors through Material Design tokens when on Android
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all theme-related color definitions to verify consistency
rg -l "messageInnerBubbleColor|myMessageInnerBubbleColor" --type ts --type tsx
Length of output: 108
Script:
#!/bin/bash
# Search for all theme-related color definitions without file type restrictions
rg "messageInnerBubbleColor|myMessageInnerBubbleColor"
Length of output: 1287
Script:
#!/bin/bash
# Check the implementation of color functions to verify contrast ratios
cat styles/colors/index.ts
Length of output: 12839
components/ClickableText.tsx (1)
1-1: LGTM! Export and import changes align with best practices.
The switch from default to named exports/imports improves code maintainability and makes imports more explicit.
Also applies to: 25-25
containers/GroupPendingRequestsTable.tsx (1)
17-21: LGTM! Type change improves type safety.
The change from string to ConversationTopic for the topic prop provides better type safety and clarity about the expected data structure.
config.ts (1)
60-60: LGTM! Type safety improvements
The changes enhance type safety through:
- Proper typing of
xmtpEnvwithXmtpEnvinstead of string - Making the
ENVobject immutable withas const
Also applies to: 125-125
components/Chat/Message/Message.utils.tsx (2)
95-103: LGTM! Well-implemented hooks and utilities
The hooks and utility functions demonstrate good practices:
- Proper error handling in useMessageSenderReadableProfile
- Effective use of React Query for data fetching
- Proper null checks in getCurrentAccountInboxIdQueryOptions
Also applies to: 107-109, 111-118, 120-124
35-37: Consider architectural improvement suggestion
The TODO comment suggests moving these utilities. Consider moving them to @xmtp/react-native-sdk as these are generic message type guards that could benefit other projects.
Let's check if similar utilities exist in the SDK:
✅ Verification successful
Let me check if these utilities are specific to this application or if they are generic enough to be moved to the SDK by examining their implementation and dependencies.
Let me check one more thing to see if these type guards depend on any application-specific utilities.
Let me try one last search to find the content types utility.
Let me check the implementation of the contentTypes utility to understand if these type guards are tightly coupled with application-specific logic.
Moving these utilities to @xmtp/react-native-sdk would be beneficial
The type guard utilities and their dependencies are generic enough to be moved to the SDK:
- They rely on standard XMTP content type prefixes
- They use the core SDK's message types and codecs
- The helper function
getMessageContentTypeis also generic and only maps content type strings to their basic types - These utilities would be valuable for any React Native application using XMTP
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if similar utilities exist in the SDK
rg -A 2 "export function is.*Message.*"
Length of output: 2072
Script:
#!/bin/bash
# Check the implementation of these message type guards
cat components/Chat/Message/Message.utils.tsx
# Check for any app-specific dependencies
rg -l "@xmtp/react-native-sdk" --type typescript
Length of output: 4344
Script:
#!/bin/bash
# Check the implementation of getMessageContentType utility
cat utils/xmtpRN/contentTypes.ts
Length of output: 95
Script:
#!/bin/bash
# Find the contentTypes file and getMessageContentType implementation
fd contentTypes
rg "getMessageContentType"
Length of output: 2354
Script:
#!/bin/bash
# Check the implementation of contentTypes utility
cat utils/xmtpRN/contentTypes/index.ts
Length of output: 1047
components/ConversationFlashList.tsx (1)
98-99: LGTM! Pull-to-refresh implementation
The pull-to-refresh functionality is properly implemented using FlashList's built-in refresh control.
components/XmtpEngine.tsx (2)
Line range hint 139-145: Verify error handling in syncConversationListXmtpClient
The error handling in the sync process only updates the syncing status but doesn't handle or log the actual error. This could make debugging issues in production difficult.
Consider adding error logging and potentially a retry mechanism:
syncConversationListXmtpClient(a)
.then(() => {
this.syncingAccounts[a] = false;
})
- .catch(() => {
+ .catch((error) => {
+ logger.error('[XmtpEngine] Failed to sync account', { error, account: a });
this.syncingAccounts[a] = false;
+ // Consider implementing a retry mechanism here
});17-18: Verify the impact of conversation streaming changes
The import changes suggest a shift in how conversations are handled. We need to ensure this doesn't break existing functionality.
✅ Verification successful
No breaking changes detected in conversation handling
The code analysis shows that the imported functions stopStreamingConversations and syncConversationListXmtpClient are properly integrated within the codebase:
stopStreamingConversationsis a utility function defined inutils/xmtpRN/conversations.tsand is used consistently across the codebase for stopping conversation streamssyncConversationListXmtpClientis defined inutils/xmtpRN/sync.tsand is used for syncing conversation lists- Both functions are used correctly in
XmtpEngine.tsxand their implementations in the utility files maintain expected functionality
The import changes reflect a proper organization of code rather than a functional change in how conversations are handled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining references to old conversation handling methods
rg "syncXmtpClient|streamConversations" --type ts --type tsx
Length of output: 90
Script:
#!/bin/bash
# Search for conversation-related patterns in TypeScript files
rg "syncXmtpClient|streamConversations|stopStreamingConversations|syncConversationListXmtpClient" -t ts
Length of output: 1033
Script:
#!/bin/bash
# Check the content of the utils/xmtpRN directory to understand the changes
fd . "utils/xmtpRN" -t f -e ts -x cat {}
Length of output: 45194
Script:
#!/bin/bash
# Look for any usage of the imported functions
rg "stopStreamingConversations|syncConversationListXmtpClient" -t ts
Length of output: 793
components/AccountSettingsButton.tsx (1)
Line range hint 66-72: Verify profile data freshness after removing refresh
The removal of refreshProfileForAddress could lead to stale profile data being displayed. We need to ensure there's another mechanism keeping profile data up to date.
Consider implementing a background refresh mechanism or refreshing profile data when navigating to the Profile screen to ensure data freshness.
components/DebugButton.tsx (1)
24-24: Verify the removal of database export functionality
The import statement has been simplified to only include useAccountsList, removing currentAccount. Please confirm if the database export functionality has been intentionally deprecated or moved elsewhere as part of the V3 migration.
✅ Verification successful
Database export functionality has been intentionally removed
The database export functionality was intentionally removed as part of the V3 migration. The codebase now handles database operations differently:
- Database management is now handled through dedicated utilities in
utils/fileSystem/andutils/xmtpRN/ - Debug functionality in
DebugButton.tsxhas been streamlined to focus on logging, app updates, and cache management - Database operations are now integrated into the core functionality rather than being exposed through debug features
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining database export functionality
rg -i "export.*db.*file" --type ts --type tsx
# Check for any new database export implementations
rg -i "getConverseDbPath" --type ts --type tsx
Length of output: 150
Script:
#!/bin/bash
# Search for any remaining database export functionality without file type restrictions
rg -i "export.*db.*file"
# Check for any new database export implementations
rg -i "getConverseDbPath"
# Search for database-related functionality in general
rg -i "database.*export|export.*database"
Length of output: 646
Script:
#!/bin/bash
# Let's check the git history of the DebugButton component to understand what was removed
git log -p -- components/DebugButton.tsx
# Also search for any debug-related exports or buttons
rg -i "debug.*button|export.*debug" -A 3
Length of output: 6296
Script:
#!/bin/bash
# Let's check the current content of DebugButton.tsx
cat components/DebugButton.tsx
# And check for any database-related functionality in the app's utilities
rg -i "database" utils/ -A 3
Length of output: 13062
components/ConversationContextMenu.tsx (1)
55-59: Verify animation timing consistency
The animation duration is now consistently using animation.contextMenuHoldDuration. Ensure this provides a smooth user experience across all animations (opacity, intensity, and translation).
Also applies to: 66-66, 92-92, 109-109
✅ Verification successful
Based on the search results, I can now generate the final response:
Animation timing is consistently implemented
The animation.contextMenuHoldDuration is consistently used across all context menu animations in the codebase. It's properly defined in the theme's animation configuration (theme/animations.ts) and is derived from HOLD_ITEM_TRANSFORM_DURATION constant. The timing is applied uniformly to opacity, intensity, and translation animations in both ConversationContextMenu.tsx and MessageContextMenu.tsx components, ensuring a smooth and consistent user experience.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent usage of animation durations
rg "animation\.contextMenuHoldDuration" --type ts --type tsx -C 2
Length of output: 95
Script:
#!/bin/bash
# Check for consistent usage of animation durations with proper file extensions
rg "contextMenuHoldDuration" -t typescript -t javascriptreact -t typescriptreact -C 2
Length of output: 120
Script:
#!/bin/bash
# Look for animation-related constants or configurations
fd -e ts -e tsx -e js -e jsx --exec grep -l "animation" {} | xargs cat
Length of output: 81106
Script:
#!/bin/bash
# Search for animation-related constants and configurations with proper file extensions
rg "contextMenuHoldDuration" -g "*.{ts,tsx,js,jsx}"
Length of output: 1250
Script:
#!/bin/bash
# Look for animation timing configurations in the theme files
fd -e ts -e tsx "animation" -x cat {} | grep -A 5 -B 5 "contextMenuHoldDuration\|timing\|duration"
Length of output: 1397
containers/GroupScreenAddition.tsx (3)
46-48: Type safety improvement
Good update to use the more specific ConversationTopic type instead of string, improving type safety.
154-157: Icon component migration looks good
The migration from Picto to the new Icon component is consistent throughout the file. The icon names and sizes are properly maintained.
Also applies to: 169-173, 182-186, 193-197
103-103: Verify V3 ID handling across the codebase
The migration to getV3IdFromTopic for group ID handling is consistent within this file. Ensure this change is applied consistently across the codebase.
Also applies to: 106-107, 127-127
✅ Verification successful
Migration to getV3IdFromTopic is consistently implemented
The codebase shows consistent implementation of getV3IdFromTopic across all relevant components:
- Group message notifications
- Group conversations and XMTP client interactions
- Group consent management (block/allow)
- iOS notification handling
- Group screen functionality
No instances of the old getGroupIdFromTopic function were found, indicating a complete migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining usage of old group ID functions
rg "getGroupIdFromTopic" --type ts --type tsx
# Verify consistent usage of new V3 ID function
rg "getV3IdFromTopic" --type ts --type tsx -C 2
Length of output: 149
Script:
#!/bin/bash
# Check for any remaining usage of old group ID functions, using ripgrep without file type filters
rg "getGroupIdFromTopic"
# Verify consistent usage of new V3 ID function
rg "getV3IdFromTopic" -C 2
Length of output: 7388
components/Chat/Message/MessageContextMenu.tsx (2)
8-8: LGTM: Animation handling updates are consistent
The changes consistently update all animation references to use the new animation object, maintaining uniform animation behavior across the component.
Also applies to: 66-66, 69-69, 125-126, 181-182, 239-240, 249-249
268-294: LGTM: Proper implementation of Portal and BlurView
The implementation correctly wraps the context menu with necessary components for gesture handling and visual effects.
components/GroupAvatar.tsx (3)
31-31: LGTM: Enhanced type safety with ConversationTopic
The changes improve type safety by using the proper ConversationTopic type and introducing a well-structured GroupAvatarDumbProps type.
Also applies to: 43-43, 52-56
135-190: LGTM: Well-structured dumb component implementation
The GroupAvatarDumb component effectively separates presentation logic, following the smart/dumb component pattern. The implementation handles all edge cases properly and maintains good code organization.
205-213: LGTM: Optimized query behavior
The changes properly optimize the useGroupMembers hook by:
- Preventing unnecessary queries when topic is missing or pendingGroupMembers exist
- Configuring appropriate caching behavior for the conversation list screen
| type MessageSenderProps = { | ||
| senderAddress: string; | ||
| }; | ||
|
|
||
| export const MessageSender = ({ senderAddress }: MessageSenderProps) => { | ||
| const senderSocials = useProfilesStore( | ||
| (s) => getProfile(senderAddress, s.profiles)?.socials | ||
| ); | ||
| const name = getPreferredName(senderSocials, senderAddress); | ||
| return <MessageSenderDumb name={name} />; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding memoization and error handling.
The component could benefit from performance optimization and better error handling.
Consider these improvements:
export const MessageSender = ({ senderAddress }: MessageSenderProps) => {
const senderSocials = useProfilesStore(
(s) => getProfile(senderAddress, s.profiles)?.socials
);
- const name = getPreferredName(senderSocials, senderAddress);
+ const name = useMemo(
+ () => getPreferredName(senderSocials, senderAddress),
+ [senderSocials, senderAddress]
+ );
+
+ if (!senderAddress) {
+ return null;
+ }
+
return <MessageSenderDumb name={name} />;
};Committable suggestion skipped: line range outside the PR's diff.
| type ConversationTitleDumbProps = { | ||
| title?: string; | ||
| avatarComponent?: React.ReactNode; | ||
| onLongPress?: () => void; | ||
| onPress?: () => void; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making the title prop required
Since the title is a crucial part of the conversation display, consider making it required to ensure type safety and prevent potential runtime issues.
type ConversationTitleDumbProps = {
- title?: string;
+ title: string;
avatarComponent?: React.ReactNode;
onLongPress?: () => void;
onPress?: () => void;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type ConversationTitleDumbProps = { | |
| title?: string; | |
| avatarComponent?: React.ReactNode; | |
| onLongPress?: () => void; | |
| onPress?: () => void; | |
| }; | |
| type ConversationTitleDumbProps = { | |
| title: string; | |
| avatarComponent?: React.ReactNode; | |
| onLongPress?: () => void; | |
| onPress?: () => void; | |
| }; |
| return ( | ||
| <AnimatedHStack | ||
| style={styles.container} | ||
| entering={animation.fadeInUpSlow()} | ||
| > | ||
| <TouchableOpacity | ||
| onLongPress={onLongPress} | ||
| onPress={onPress} | ||
| style={styles.touchableContainer} | ||
| > | ||
| {avatarComponent} | ||
| <Text style={styles.title} numberOfLines={1} allowFontScaling={false}> | ||
| {title} | ||
| </Text> | ||
| </TouchableOpacity> | ||
| </AnimatedHStack> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility support
Consider enhancing the component's accessibility by adding appropriate props:
<TouchableOpacity
onLongPress={onLongPress}
onPress={onPress}
style={styles.touchableContainer}
+ accessible={true}
+ accessibilityRole="button"
+ accessibilityLabel={`Conversation with ${title}`}
>
{avatarComponent}
<Text
style={styles.title}
numberOfLines={1}
allowFontScaling={false}
+ accessibilityRole="header"
>
{title}
</Text>
</TouchableOpacity>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <AnimatedHStack | |
| style={styles.container} | |
| entering={animation.fadeInUpSlow()} | |
| > | |
| <TouchableOpacity | |
| onLongPress={onLongPress} | |
| onPress={onPress} | |
| style={styles.touchableContainer} | |
| > | |
| {avatarComponent} | |
| <Text style={styles.title} numberOfLines={1} allowFontScaling={false}> | |
| {title} | |
| </Text> | |
| </TouchableOpacity> | |
| </AnimatedHStack> | |
| ); | |
| return ( | |
| <AnimatedHStack | |
| style={styles.container} | |
| entering={animation.fadeInUpSlow()} | |
| > | |
| <TouchableOpacity | |
| onLongPress={onLongPress} | |
| onPress={onPress} | |
| style={styles.touchableContainer} | |
| accessible={true} | |
| accessibilityRole="button" | |
| accessibilityLabel={`Conversation with ${title}`} | |
| > | |
| {avatarComponent} | |
| <Text | |
| style={styles.title} | |
| numberOfLines={1} | |
| allowFontScaling={false} | |
| accessibilityRole="header" | |
| > | |
| {title} | |
| </Text> | |
| </TouchableOpacity> | |
| </AnimatedHStack> | |
| ); |
| const isSentOrDelivered = status === "sent" || status === "delivered"; | ||
|
|
There was a problem hiding this comment.
Ensure isSentOrDelivered updates when status changes
The variable isSentOrDelivered may not update as expected if status changes without re-rendering.
Since isSentOrDelivered depends on status, you might want to move its declaration inside the useEffect or include status in the dependency array.
- const isSentOrDelivered = status === "sent" || status === "delivered";
+ // Remove this line and move inside useEffect or include status in dependenciesAlternatively, recompute it inside the useEffect:
useEffect(() => {
const isSentOrDelivered = status === "sent" || status === "delivered";
// existing code
}, [status, isLatestSettledFromMe]);| setTimeout(() => setRenderText(false), timingConfig.duration); | ||
| } else if (isLatestSettledFromMe) { |
There was a problem hiding this comment.
Prevent potential memory leaks by clearing timeouts
The setTimeout used to update renderText may continue running after the component unmounts, leading to memory leaks.
Add a cleanup function to clear the timeout:
useEffect(() => {
// existing code
+ return () => {
+ clearTimeout(timeoutId);
+ };
}, [/* dependencies */]);Store the timeout ID when setting the timeout:
- setTimeout(() => setRenderText(false), timingConfig.duration);
+ const timeoutId = setTimeout(() => setRenderText(false), timingConfig.duration);Committable suggestion skipped: line range outside the PR's diff.
| const { data: senderSocials } = useInboxProfileSocialsQuery( | ||
| currentAccount!, | ||
| inboxId | ||
| ); |
There was a problem hiding this comment.
Handle potential null currentAccount
The non-null assertion (currentAccount!) should be replaced with proper null checking to prevent runtime errors.
- currentAccount!,
+ currentAccount ?? throw new Error("No current account found"),Committable suggestion skipped: line range outside the PR's diff.
| // type MessageSenderAvatarProps = { | ||
| // senderAddress: string; | ||
| // hasNextMessageInSeries: boolean; | ||
| // }; | ||
|
|
||
| // export const MessageSenderAvatar = ({ | ||
| // senderAddress, | ||
| // // hasNextMessageInSeries, | ||
| // }: MessageSenderAvatarProps) => { | ||
| // const senderSocials = useProfilesStore( | ||
| // (s) => getProfile(senderAddress, s.profiles)?.socials | ||
| // ); | ||
|
|
||
| // const openProfile = useCallback(() => { | ||
| // navigate("Profile", { address: senderAddress }); | ||
| // }, [senderAddress]); | ||
|
|
||
| // return ( | ||
| // <MessageSenderAvatarDumb | ||
| // // hasNextMessageInSeries={hasNextMessageInSeries} | ||
| // onPress={openProfile} | ||
| // avatarUri={getPreferredAvatar(senderSocials)} | ||
| // avatarName={getPreferredName(senderSocials, senderAddress)} | ||
| // /> | ||
| // ); | ||
| // }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out code
The old implementation should be removed as it's now replaced by the V3 version. If needed, this code can be retrieved from version control history.
-// type MessageSenderAvatarProps = {
-// senderAddress: string;
-// hasNextMessageInSeries: boolean;
-// };
-
-// export const MessageSenderAvatar = ({
-// senderAddress,
-// // hasNextMessageInSeries,
-// }: MessageSenderAvatarProps) => {
-// const senderSocials = useProfilesStore(
-// (s) => getProfile(senderAddress, s.profiles)?.socials
-// );
-
-// const openProfile = useCallback(() => {
-// navigate("Profile", { address: senderAddress });
-// }, [senderAddress]);
-
-// return (
-// <MessageSenderAvatarDumb
-// // hasNextMessageInSeries={hasNextMessageInSeries}
-// onPress={openProfile}
-// avatarUri={getPreferredAvatar(senderSocials)}
-// avatarName={getPreferredName(senderSocials, senderAddress)}
-// />
-// );
-// };| @@ -85,7 +85,7 @@ export function useInitConnectViaWalletState(args: { address: string }) { | |||
| setIsInitializing(true); | |||
|
|
|||
| const [isOnNetwork, inboxId] = await Promise.all([ | |||
| isOnXmtp(address), | |||
| canMessageByAccount({ account: address, peer: address }), | |||
There was a problem hiding this comment.
💡 Codebase verification
Using the same address for account and peer is incorrect in canMessageByAccount
Based on the implementation found, canMessageByAccount is designed to check if one account can message another peer, where:
- The
accountparameter represents the sender's address (the one initiating the message) - The
peerparameter represents the recipient's address
Other usages in the codebase confirm this pattern:
- In NavigationChatButton.tsx: uses currentAccount() as account and address as peer
- In NewConversation.tsx: uses currentAccount() as account and address as peer
Using the same address for both parameters is incorrect as it checks if an account can message itself.
🔗 Analysis chain
Verify the behavior of canMessageByAccount function.
The change from isOnXmtp to canMessageByAccount might affect the initialization flow. Ensure that passing the same address as both account and peer parameters is the intended usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of canMessageByAccount to verify the parameter pattern
rg -A 2 "canMessageByAccount\("
Length of output: 887
Script:
#!/bin/bash
# Let's check the implementation of canMessageByAccount to understand its behavior
ast-grep --pattern 'canMessageByAccount = async ({ account, peer }: { account: string; peer: string }) => {
$$$
}'
Length of output: 118
Script:
#!/bin/bash
# Let's try a different pattern to find the implementation
ast-grep --pattern 'export const canMessageByAccount = $_'
# Also search for the function definition with ripgrep for better context
rg -A 5 "export const canMessageByAccount"
Length of output: 1012
Script:
#!/bin/bash
# Let's check the implementation of canMessage function that's being called
ast-grep --pattern 'export const canMessage = $_'
# Also check for the context where this function is used in the initialization
rg -B 5 -A 10 "canMessageByAccount\({ account: address, peer: address }\)"
Length of output: 1084
| export function isTextMessage( | ||
| message: any | ||
| ): message is DecodedMessage<[TextCodec]> { | ||
| return getMessageContentType(message.contentTypeId) === "text"; | ||
| } |
There was a problem hiding this comment.
Replace 'any' type with proper typing
The type guard functions are using any type instead of DecodedMessageWithCodecsType. This reduces type safety and bypasses TypeScript's type checking benefits.
Apply this pattern to all message type guard functions:
-export function isTextMessage(
- message: any
-): message is DecodedMessage<[TextCodec]> {
+export function isTextMessage(
+ message: DecodedMessageWithCodecsType
+): message is DecodedMessage<[TextCodec]> {Also applies to: 44-49, 50-55, 56-61, 62-67, 68-73, 74-79, 80-87, 88-93
| @@ -30,19 +29,13 @@ | |||
| import { ThirdwebProvider } from "thirdweb/react"; | |||
|
|
|||
| import { Snackbars } from "@components/Snackbar/Snackbars"; | |||
| import { xmtpCron, xmtpEngine } from "./components/XmtpEngine"; | |||
| import { xmtpEngine } from "./components/XmtpEngine"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider implementing a cleanup mechanism for xmtpEngine
While xmtpEngine is started, there's no visible cleanup mechanism when the app is unmounted. This could lead to memory leaks or hanging connections.
Consider wrapping the engine initialization in a useEffect:
-xmtpEngine.start();
+useEffect(() => {
+ xmtpEngine.start();
+ return () => {
+ xmtpEngine.destroy();
+ };
+}, []);Also applies to: 63-63
Fixed TSC Fixed Tests Fixed iOS build
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
data/store/profilesStore.ts (1)
Line range hint
1-102: Overall implementation is well-structuredThe store implementation follows good practices with:
- Clear type definitions for various social identities
- Proper use of Zustand middleware for persistence
- Account-based storage separation
Consider adding JSDoc comments to document the store's purpose and usage patterns.
components/Conversation/V3Conversation.tsx (4)
119-121: Implement a placeholder for missing direct messagesThere's a TODO comment indicating that a placeholder needs to be added when the conversation is not found. Providing a user-friendly message or UI component will enhance the user experience.
Would you like assistance in implementing this placeholder?
124-126: Implement a placeholder for empty direct messagesThere's a TODO comment indicating that a placeholder needs to be added when there are no messages in the conversation. Providing an informative message or UI component will inform users appropriately.
Would you like assistance in implementing this placeholder?
301-305: Make the sent message configurable and localizableIn
handleSend, the sent message is hard-coded as "👋". To support localization and enhance flexibility, consider retrieving this message from translation files or making it configurable.Apply this diff to use the translation function:
- sendMessage({ - text: "👋", - }); + sendMessage({ + text: translate("group_placeholder.default_message"), + });Ensure that
"group_placeholder.default_message"is defined in your localization files.
82-91: Simplify nested ternary operators for better readabilityThe conditional rendering using nested ternary operators can be difficult to read and maintain. Consider refactoring to use if-else statements or separate components to improve readability.
Apply this diff to refactor the conditional rendering:
return ( <AnimatedVStack layout={theme.animation.reanimatedSpringLayoutTransition} style={{ flex: 1, }} > - {isNewConversation ? ( - <NewConversationContent /> - ) : conversationVersion === ConversationVersion.DM ? ( - <DmContent /> - ) : ( - <ConversationGroupContextProvider> - <GroupContent /> - </ConversationGroupContextProvider> - )} + {renderContent(isNewConversation, conversationVersion)} <ChatInputDumb /> <KeyboardFiller /> </AnimatedVStack> );Then, define the
renderContentfunction:function renderContent( isNewConversation: boolean, conversationVersion: ConversationVersion ) { if (isNewConversation) { return <NewConversationContent />; } else if (conversationVersion === ConversationVersion.DM) { return <DmContent />; } else { return ( <ConversationGroupContextProvider> <GroupContent /> </ConversationGroupContextProvider> ); } }This refactoring improves the code's readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
components/Conversation/V3Conversation.tsx(1 hunks)data/store/profilesStore.ts(3 hunks)
🔇 Additional comments (3)
data/store/profilesStore.ts (1)
43-43: Consider potential redundancy in ProfileSocials type
The addition of an optional address field to ProfileSocials might be redundant since the address is already used as a key in the ProfileByAddress mapping. This could lead to data inconsistency if the stored address doesn't match the key.
components/Conversation/V3Conversation.tsx (2)
53-53: Handle undefined topic to prevent potential crashes
At line 53, there is a TODO comment indicating that the case where topic is undefined needs to be handled. Not addressing this may lead to runtime errors when topic is not provided.
178-183: Resolve TypeScript error in Animated.FlatList props
At line 178, there's a TypeScript error being suppressed with @ts-ignore. Suppressing errors with @ts-ignore is not recommended, as it may hide underlying issues.
Suggestion:
Ensure that the props passed to Animated.FlatList match the expected types. The layout prop might be required instead of itemLayoutAnimation.
Apply this diff to correct the prop:
- itemLayoutAnimation={theme.animation.reanimatedSpringLayoutTransition}
+ layout={theme.animation.reanimatedSpringLayoutTransition}Alternatively, update the type definitions or verify the react-native-reanimated version.
| const state = JSON.parse(mmkvState) as ProfilesStoreType; | ||
| return state; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling in refreshFromStorage
While the type annotation improves safety, the error handling could be more robust:
- Add validation for the parsed state structure
- Handle different error types explicitly
- Consider notifying the user on failure
Consider this improvement:
- const state = JSON.parse(mmkvState) as ProfilesStoreType;
- return state;
+ const parsedState = JSON.parse(mmkvState) as ProfilesStoreType;
+ // Validate essential properties
+ if (!parsedState?.profiles || typeof parsedState.profiles !== 'object') {
+ throw new Error('Invalid state structure');
+ }
+ return parsedState;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const state = JSON.parse(mmkvState) as ProfilesStoreType; | |
| return state; | |
| const parsedState = JSON.parse(mmkvState) as ProfilesStoreType; | |
| // Validate essential properties | |
| if (!parsedState?.profiles || typeof parsedState.profiles !== 'object') { | |
| throw new Error('Invalid state structure'); | |
| } | |
| return parsedState; |
| @@ -2,6 +1,7 @@ | |||
| import { getCleanAddress } from "@utils/evm/getCleanAddress"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Update remaining import in Chat.tsx to use new path
The import path change has been consistently updated across the codebase except for one file. There's a commented-out import in components/Chat/Chat.tsx that still references the old path @utils/evm/address.
components/Chat/Chat.tsx: Update or remove the commented import to use@utils/evm/getCleanAddress
🔗 Analysis chain
Verify the import path changes across the codebase
The import path for getCleanAddress has changed. Let's ensure this change is consistent across the codebase.
Also applies to: 4-4
Let me verify the content of both files to ensure the function is properly moved and updated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining old imports and verify new path exists
# Check if the new file exists
fd "getCleanAddress.ts" utils/evm
# Check for any remaining old imports
rg "@utils/evm/address" --type ts
Length of output: 178
Script:
#!/bin/bash
# Check the content of both files and any references to getCleanAddress
ast-grep --pattern 'export function getCleanAddress'
# Check if there are any other imports of getCleanAddress
rg "import.*getCleanAddress" --type ts
Length of output: 763
| initializeCurrentConversation({ | ||
| topic, | ||
| peerAddress, | ||
| inputValue: messageToPrefill, | ||
| }); |
There was a problem hiding this comment.
Avoid side effects in the component render function
The call to initializeCurrentConversation at lines 55-59 may cause side effects and should be placed inside a useEffect hook to prevent unexpected behavior during re-renders.
Apply this diff to move the initialization into a useEffect:
+ useEffect(() => {
initializeCurrentConversation({
topic,
peerAddress,
inputValue: messageToPrefill,
});
+ }, [topic, peerAddress, messageToPrefill]);This ensures that the initialization runs when the dependencies change and not on every render.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| initializeCurrentConversation({ | |
| topic, | |
| peerAddress, | |
| inputValue: messageToPrefill, | |
| }); | |
| useEffect(() => { | |
| initializeCurrentConversation({ | |
| topic, | |
| peerAddress, | |
| inputValue: messageToPrefill, | |
| }); | |
| }, [topic, peerAddress, messageToPrefill]); |
feat: Merge V3 Branch to Release 3.0.0
Merged feature/v3-split
feat: V3 JS Notification handling
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
XmtpEngineclass by removing unused functionalities.Chores