feature/BA-2626-chatroom-list #293
Conversation
gabrieleandro
requested review from
anicioalexandre and
priscilladeroode
as code owners
|
WalkthroughAdds a native ChatRooms UI and supporting primitives: chat container with search/tabs and live subscription, paginated RoomsList and ChatCard (with options/archive mutation), EmptyView states, NewChatButton, design-system FabButton/SearchInput/illustrations, GraphQL fragment, types/styles, and package metadata updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ChatRooms
participant Relay as Relay/GraphQL
participant Subscription as RoomListSubscription
participant RoomsList
participant AppState
User->>ChatRooms: open screen
ChatRooms->>Relay: useLazyLoadQuery(ChatRoomsQuery)
ChatRooms->>Subscription: subscribe to room list updates
AppState-->>ChatRooms: app focused / becomes active
ChatRooms->>Relay: refetch({ q, unread, archived })
ChatRooms->>RoomsList: render(targetRef, searchParam, selectedTab)
sequenceDiagram
autonumber
actor User
participant ChatCard
participant Options as ChatCardOptions
participant Relay as Relay/GraphQL
User->>ChatCard: long-press
ChatCard->>Options: open bottom sheet
User->>Options: tap Archive
Options->>Relay: mutate(archiveRoom)
Relay-->>Options: mutation result
Options->>ChatCard: close/update UI
sequenceDiagram
autonumber
actor User
participant ChatRooms
participant Router
User->>ChatRooms: tap NewChatButton
ChatRooms->>Router: router.push("/create-room")
Router-->>User: navigated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Files/areas needing extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gabrieleandro
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (33)
packages/design-system/components/native/inputs/SearchInput/styles.ts (1)
7-13: Minor style tweaks for clarity and accessibility.
- Use
marginVerticalinstead of separate margins.- Prefer
minHeightover fixedheightto better respect large text settings.Apply this diff:
- marginTop: 12, - marginBottom: 12, + marginVertical: 12, - height: 45, + minHeight: 45,packages/design-system/components/native/inputs/SearchInput/index.tsx (1)
26-37: AttachonPressto TextInput.Icon, not the nested Ionicon.Leverage Paper’s press handling/ripple; keep the icon render purely visual.
Apply this diff (pairs with the previous change):
searchParam && searchParam.length > 0 ? ( - <Ionicons - name="close" - size={24} - color={theme.colors.object.disabled} - onPress={resetInput} - /> + <Ionicons name="close" size={24} color={theme.colors.object.disabled} /> ) : null,packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/styles.ts (1)
19-26: MaketextWrapperresponsive (avoid fixed width).A fixed width of 393 can clip on smaller screens. Prefer full width with a maxWidth cap.
textWrapper: { flex: 1, - width: 393, + width: '100%', + maxWidth: 393, padding: 16, justifyContent: 'flex-end', alignItems: 'center', gap: 10, },packages/design-system/components/native/buttons/FabButton/styles.ts (1)
7-25: Tweak FAB positioning and iOS shadow color.
- Add right margin so the FAB isn’t flush to the edge.
- Provide
shadowColorfor iOS shadows.If the FAB can overlap the home indicator, consider adding bottom safe-area insets in the component.
button: { position: 'absolute', - right: 0, + right: 16, bottom: 16, backgroundColor: theme.colors.primary.main, width: 56, height: 56, borderRadius: 500, justifyContent: 'center', alignItems: 'center', zIndex: 10, + shadowColor: '#000', shadowOffset: { width: 0, height: 8, }, shadowOpacity: 0.24, shadowRadius: 8, elevation: 8, },packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
5-39: DRY up duplicated ChatCard styles.This largely duplicates styles in RoomsList/ChatCard/styles.ts. Extract shared base style objects (not StyleSheet-created) into a single module and reuse to avoid drift.
packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
5-44: Consolidate with ChatCardComponent styles to prevent divergence.These definitions overlap with ChatCardComponent/styles.ts. Recommend a shared base style factory to keep one source of truth.
packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
1-13: Strengthen typing of CHAT_TAB_LABEL withsatisfies.Enforce that all tab values have labels and catch missing/extra keys at compile time.
Please confirm the TS version supports
satisfies(TS ≥ 4.9).export const CHAT_TAB_VALUES = { active: 'active', unread: 'unread', archived: 'archived', groups: 'groups', } as const -export const CHAT_TAB_LABEL = { +type ChatTabValue = (typeof CHAT_TAB_VALUES)[keyof typeof CHAT_TAB_VALUES] + +export const CHAT_TAB_LABEL = { [CHAT_TAB_VALUES.active]: 'Active', [CHAT_TAB_VALUES.unread]: 'Unread', [CHAT_TAB_VALUES.archived]: 'Archived', [CHAT_TAB_VALUES.groups]: 'Groups', -} as const +} as const satisfies Record<ChatTabValue, string>packages/design-system/components/native/buttons/index.ts (1)
4-4: Re-export FabButton types for consumers.Expose the props type like other buttons to improve DX.
export { default as IconButton } from './IconButton' export { default as FabButton } from './FabButton' +export type * from './FabButton/types'packages/components/modules/messages/native/SearchNotFoundState/types.ts (1)
1-3: Allow richer content for messageConsider accepting ReactNode to support localized/styled content.
+import { type ReactNode } from 'react' export interface SearchNotFoundStateProps { - message?: string + message?: ReactNode }packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts (1)
1-9: AligncreateStylessignature with theme-based convention (and remove commented import)Follow the pattern used elsewhere: accept
Themeand ignore it if unused for now.-// import { Theme } from '@baseapp-frontend/design-system/styles/native' +import { Theme } from '@baseapp-frontend/design-system/styles/native' import { StyleSheet } from 'react-native' -export const createStyles = () => +export const createStyles = (_theme: Theme) => StyleSheet.create({ chatRoomsContainer: { flex: 1, }, })packages/components/modules/messages/native/RoomsList/types.ts (1)
3-7: TypeselectedTabmore strictlyUse the new
ChatTabValuesinstead ofstringto prevent invalid values.+import type { ChatTabValues } from './RoomsListComponent/types' export interface RoomsListProps { targetRef: ChatRoomsQuery$data searchParam: string - selectedTab: string + selectedTab: ChatTabValues }packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx (1)
8-24: Make message customizable and prep for i18nAccept an optional message prop with a sensible default to ease localization.
-const EmptyChatRoomsState = () => { +interface Props { + message?: string +} + +const EmptyChatRoomsState = ({ message }: Props) => { const theme = useTheme() const styles = createStyles(theme) return ( <View style={styles.container}> <View style={styles.imageWrapper}> <SocialMediaDiscussionImage /> </View> <View style={styles.textWrapper}> <Text variant="subtitle2" style={styles.text}> - No messages to be displayed. + {message ?? 'No messages to be displayed.'} </Text> </View> </View> ) }packages/design-system/components/native/buttons/FabButton/index.tsx (1)
11-25: Improve accessibility and touch ergonomics; keep consumer override precedenceSet an accessibility role and provide a default hitSlop without overriding consumer-provided props.
return ( - <TouchableOpacity style={[styles.button, style]} {...touchableProps}> + <TouchableOpacity + {...touchableProps} + accessibilityRole="button" + hitSlop={touchableProps.hitSlop ?? { top: 8, right: 8, bottom: 8, left: 8 }} + style={[styles.button, style]} + > <Ionicons name={iconName} size={iconSize} color={iconColor} /> </TouchableOpacity> )packages/components/modules/messages/native/SearchNotFoundState/index.tsx (2)
21-26: Localize hard‑coded title ("No results found").Expose it as a prop (or use your i18n util) to enable translations and customization.
- <Text variant="subtitle2" style={styles.text}> - No results found - </Text> + <Text variant="subtitle2" style={styles.text}> + {title} + </Text>Additionally update the props:
-const SearchNotFoundState = ({ - message = 'Check your spelling or try another search.', -}: SearchNotFoundStateProps) => { +const SearchNotFoundState = ({ + title = 'No results found', + message = 'Check your spelling or try another search.', +}: SearchNotFoundStateProps) => {And in types.ts:
export interface SearchNotFoundStateProps { title?: string message?: string }
16-21: Add basic accessibility and testID.Help screen readers and E2E tests.
- <View style={styles.container}> - <View style={styles.imageWrapper}> + <View style={styles.container} testID="searchNotFoundState"> + <View + style={styles.imageWrapper} + accessible + accessibilityRole="image" + accessibilityLabel="No search results illustration" + > <SearchingImage /> </View>packages/components/modules/messages/native/index.ts (1)
4-6: Consider re-exporting NewChatButton for convenience.Keeps the public API cohesive under the native messages namespace.
export { default as ChatRooms } from './ChatRooms' export { default as RoomsList } from './RoomsList' export { default as SearchNotFoundState } from './SearchNotFoundState' +export { NewChatButton } from './NewChatButton'packages/components/modules/messages/native/SearchNotFoundState/styles.ts (1)
21-26: Avoid hard‑coded width (393).Use responsive width with an optional maxWidth for large screens.
- width: 393, + width: '100%', + maxWidth: 393,packages/components/modules/messages/native/RoomsList/index.tsx (2)
10-14: Add an explicit Suspense fallback.Avoid a blank state while loading; even a null fallback declared explicitly improves intent. Prefer a DS loader if available.
-const SuspendedRoomsList = (props: RoomsListProps) => ( - <Suspense> +const SuspendedRoomsList = (props: RoomsListProps) => ( + <Suspense fallback={null}> <RoomsList {...props} /> </Suspense> )
6-8: Optionally export the non‑suspended variant.Useful for embedding in parent Suspense boundaries.
const RoomsList: FC<RoomsListProps> = ({ targetRef, searchParam, selectedTab }) => ( <RoomsListComponent targetRef={targetRef} searchParam={searchParam} selectedTab={selectedTab} /> ) const SuspendedRoomsList = (props: RoomsListProps) => ( @@ -export default SuspendedRoomsList +export default SuspendedRoomsList +export { RoomsList }Also applies to: 16-16
packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx (1)
7-15: Align prop type with runtime null guard.Since you allow a falsy roomRef, reflect it in the type to prevent misuse upstream.
-}: { - roomRef: RoomsListFragment$key +}: { + roomRef: RoomsListFragment$key | null | undefined isArchived: boolean }) => { if (!roomRef) { return null }packages/components/modules/messages/native/ChatRooms/styles.ts (2)
7-15: Remove redundant flexGrow; prefer consistent flex usage.flex: 1 already implies flexGrow: 1. Also prefer flex: 1 for chatRoomsContainer to match other styles.
container: { backgroundColor: theme.colors.surface.background, - flexGrow: 1, flex: 1, position: 'relative', }, chatRoomsContainer: { - flexGrow: 1, + flex: 1, },
16-19:gap: 0is a no‑op; remove or replace if spacing is needed.If gaps are intended later, ensure RN version supports it.
packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx (1)
17-18: Consider memoizing the color computation.The color calculation is recomputed on every render. Since this is an SVG component that might be rendered frequently in lists, consider memoizing the result.
+import { FC, useMemo } from 'react' -import { FC } from 'react' const SocialMediaDiscussionImage: FC<SvgIconProps> = ({ isActive = false, color, width = '200', height = '200', ...props }) => { const { colors } = useTheme() - const defaultColor = color ?? colors.object.high - const svgColor = isActive ? colors.primary.high : defaultColor + const svgColor = useMemo(() => { + const defaultColor = color ?? colors.object.high + return isActive ? colors.primary.high : defaultColor + }, [color, colors.object.high, colors.primary.high, isActive])packages/design-system/components/native/illustrations/SearchingImage/index.tsx (1)
1-58: Component mirrors SocialMediaDiscussionImage pattern correctly.The implementation is consistent with the SocialMediaDiscussionImage component structure and follows the same theming approach.
Consider applying the same memoization optimization suggested for SocialMediaDiscussionImage to maintain consistency across illustration components.
packages/components/modules/messages/native/ChatRooms/index.tsx (2)
57-66: Consider optimizing the refetch on app state changes.The refetch occurs on every app resume, which might be unnecessary if the data is fresh. Consider implementing a time-based cache invalidation.
+import { useRef } from 'react' const ChatRooms: FC = () => { + const lastRefetchTime = useRef<number>(Date.now()) // ... existing code ... useAppStateSubscription(() => { + const timeSinceLastRefetch = Date.now() - lastRefetchTime.current + // Only refetch if more than 30 seconds have passed + if (timeSinceLastRefetch < 30000) return + + lastRefetchTime.current = Date.now() refetch( { q: searchParam, unreadMessages: selectedTab === CHAT_TAB_VALUES.unread, archived: selectedTab === CHAT_TAB_VALUES.archived, }, { fetchPolicy: 'store-and-network' }, ) })
104-107: Track the Groups tab implementation.The TODO comment indicates the Groups tab is not yet implemented.
Would you like me to create an issue to track the implementation of the Groups tab functionality?
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (2)
87-102: Replace placeholder alerts with proper implementation tracking.Multiple features are marked with alert placeholders. These should be tracked properly for implementation.
Would you like me to create issues for the following unimplemented features:
- Mark as Unread functionality
- Chat Details functionality
- Go to Profile functionality
- Delete Chat functionality
Each issue can include implementation requirements and acceptance criteria.
43-54: Consider type safety improvement for fragment casting.The multiple type castings to
unknownsuggest potential type misalignment. Consider creating properly typed fragment references.- const lastMessageFragment = useFragment<LastMessageFragment$key>( - LastMessageFragment, - roomRef as unknown as LastMessageFragment$key, - ) - const headerFragment = useFragment<TitleFragment$key>( - TitleFragment, - roomRef as unknown as TitleFragment$key, - ) - const unreadMessagesCountFragment = useFragment<UnreadMessagesCountFragment$key>( - UnreadMessagesCountFragment, - roomRef as unknown as UnreadMessagesCountFragment$key, - ) + // If roomRef contains all these fragments, consider using a union type or interface + const lastMessageFragment = useFragment(LastMessageFragment, roomRef) + const headerFragment = useFragment(TitleFragment, roomRef) + const unreadMessagesCountFragment = useFragment(UnreadMessagesCountFragment, roomRef)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/index.tsx (4)
29-33: EnsurehandleSheetChangesmatches BottomDrawer's(index:number) => voidsignatureWrap your zero‑arg handler to accept the index so it always matches the drawer API.
Apply this diff:
-import { FC, useMemo } from 'react' +import { FC, useCallback, useMemo } from 'react' @@ const styles = createStyles(theme) const snapPoints = useMemo(() => ['50%', '90%'], []) + const onSheetChange = useCallback((_index: number) => { + handleSheetChanges() + }, [handleSheetChanges]) + return ( <BottomDrawer bottomDrawerRef={bottomDrawerRef} - handleSheetChanges={handleSheetChanges} + handleSheetChanges={onSheetChange} snapPoints={snapPoints} >
35-43: Add basic accessibility and disabled state hints to PressablesExpose role/labels and disabled state for assistive tech; keep consistent across items.
Apply this diff:
- <Pressable - onPress={handleArchiveChat} - style={styles.modalItem} - disabled={isArchiveMutationInFlight} - > + <Pressable + onPress={handleArchiveChat} + style={styles.modalItem} + disabled={isArchiveMutationInFlight} + accessibilityRole="button" + accessibilityLabel={isArchived ? 'Unarchive chat' : 'Archive chat'} + accessibilityState={{ disabled: isArchiveMutationInFlight }} + > @@ - <Pressable onPress={handleMarkAsUnread} style={styles.modalItem}> + <Pressable + onPress={handleMarkAsUnread} + style={styles.modalItem} + accessibilityRole="button" + accessibilityLabel="Mark as unread" + > @@ - <Pressable onPress={handleChatDetails} style={styles.modalItem}> + <Pressable + onPress={handleChatDetails} + style={styles.modalItem} + accessibilityRole="button" + accessibilityLabel="Open chat details" + > @@ - <Pressable onPress={handleGoToProfile} style={styles.modalItem}> + <Pressable + onPress={handleGoToProfile} + style={styles.modalItem} + accessibilityRole="button" + accessibilityLabel="Go to profile" + > @@ - <Pressable onPress={handleDeleteChat} style={styles.modalItem}> + <Pressable + onPress={handleDeleteChat} + style={styles.modalItem} + accessibilityRole="button" + accessibilityLabel="Delete chat" + >Also applies to: 44-58, 61-65
40-44: Externalize labels for i18nThese strings should live in your translations bundle to support localization.
Also applies to: 45-58, 62-64
62-64: Prefer theme/Text props over inline color style (if supported)Keeps styling consistent with the DS.
Apply this diff if Text supports semantic color tokens:
- <Text variant="body2" style={{ color: theme.colors.error.main }}> + <Text variant="body2" color="error"> Delete Chat </Text>packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (1)
30-39: Replace placeholder empty-state logicUse concrete conditions and return
nullinstead ofundefined.Apply this diff:
- const handleEmptyState = () => { - const hasEmptyStates = true //! isPending && data?.chatRooms?.edges.length === 0 - - if (hasEmptyStates) { - if (searchParam) return <SearchNotFoundState /> - return <EmptyChatRoomsState /> - } - - return undefined - } + const handleEmptyState = () => { + const count = + (data?.chatRooms?.edges?.filter((e) => e?.node)?.length ?? 0) + const isEmpty = !isPending && count === 0 + if (!isEmpty) return null + return searchParam ? <SearchNotFoundState /> : <EmptyChatRoomsState /> + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
packages/components/modules/messages/native/ChatRooms/index.tsx(1 hunks)packages/components/modules/messages/native/ChatRooms/styles.ts(1 hunks)packages/components/modules/messages/native/NewChatButton/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts(1 hunks)packages/components/modules/messages/native/RoomsList/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/types.ts(1 hunks)packages/components/modules/messages/native/SearchNotFoundState/index.tsx(1 hunks)packages/components/modules/messages/native/SearchNotFoundState/styles.ts(1 hunks)packages/components/modules/messages/native/SearchNotFoundState/types.ts(1 hunks)packages/components/modules/messages/native/index.ts(1 hunks)packages/design-system/components/native/buttons/FabButton/index.tsx(1 hunks)packages/design-system/components/native/buttons/FabButton/styles.ts(1 hunks)packages/design-system/components/native/buttons/FabButton/types.ts(1 hunks)packages/design-system/components/native/buttons/index.ts(1 hunks)packages/design-system/components/native/illustrations/SearchingImage/index.tsx(1 hunks)packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx(1 hunks)packages/design-system/components/native/illustrations/index.ts(1 hunks)packages/design-system/components/native/illustrations/types.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/index.tsx(1 hunks)packages/design-system/components/native/inputs/SearchInput/styles.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/types.ts(1 hunks)packages/design-system/components/native/inputs/index.ts(1 hunks)packages/design-system/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (20)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (2)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts (1)
createStyles(5-23)packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
createStyles(5-44)
packages/design-system/components/native/buttons/FabButton/index.tsx (2)
packages/design-system/components/native/buttons/FabButton/types.ts (1)
FabButtonProps(4-8)packages/design-system/components/native/buttons/FabButton/styles.ts (1)
createStyles(5-26)
packages/design-system/components/native/inputs/SearchInput/index.tsx (2)
packages/design-system/components/native/inputs/SearchInput/types.ts (1)
SearchInputProps(3-6)packages/design-system/components/native/inputs/SearchInput/styles.ts (1)
createStyles(5-14)
packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/index.tsx (2)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts (1)
ChatCardOptionsProps(5-16)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts (1)
createStyles(5-23)
packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx (1)
packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/styles.ts (1)
createStyles(5-30)
packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx (1)
packages/design-system/components/native/illustrations/types.ts (1)
SvgIconProps(3-5)
packages/components/modules/messages/native/RoomsList/index.tsx (1)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/styles.ts (3)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts (1)
createStyles(4-9)packages/components/modules/messages/native/SearchNotFoundState/styles.ts (1)
createStyles(5-30)
packages/components/modules/messages/native/ChatRooms/index.tsx (3)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (2)
CHAT_TAB_VALUES(1-6)CHAT_TAB_LABEL(8-13)packages/utils/hooks/useAppStateSubscription/index.ts (1)
useAppStateSubscription(7-23)
packages/components/modules/messages/native/ChatRooms/styles.ts (4)
packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/styles.ts (1)
createStyles(5-30)packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts (1)
createStyles(4-9)packages/design-system/components/native/buttons/FabButton/styles.ts (1)
createStyles(5-26)packages/design-system/components/native/inputs/SearchInput/styles.ts (1)
createStyles(5-14)
packages/components/modules/messages/native/SearchNotFoundState/index.tsx (2)
packages/components/modules/messages/native/SearchNotFoundState/types.ts (1)
SearchNotFoundStateProps(1-3)packages/components/modules/messages/native/SearchNotFoundState/styles.ts (1)
createStyles(5-30)
packages/design-system/components/native/inputs/SearchInput/styles.ts (3)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)packages/design-system/components/native/buttons/FabButton/styles.ts (1)
createStyles(5-26)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts (1)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts (2)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
createStyles(5-44)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts (2)
packages/utils/types/typescript.ts (1)
ValueOf(3-3)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
packages/components/modules/messages/native/SearchNotFoundState/styles.ts (1)
packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/styles.ts (1)
createStyles(5-30)
packages/design-system/components/native/illustrations/SearchingImage/index.tsx (1)
packages/design-system/components/native/illustrations/types.ts (1)
SvgIconProps(3-5)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (4)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts (1)
createStyles(5-23)packages/components/modules/messages/common/graphql/mutations/ArchiveChatRoom.ts (1)
useArchiveChatRoomMutation(24-63)packages/components/modules/messages/common/utils.ts (1)
useNameAndAvatar(53-58)
🪛 GitHub Actions: Main Workflow
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
[error] 62-62: Type '{ readonly id: string; readonly " $fragmentSpreads": FragmentRefs<"LastMessageFragment" | "TitleFragment" | "UnreadMessagesCountFragment">; } | null | undefined' is not assignable to type 'RoomsListFragment$key'.
🔇 Additional comments (15)
packages/design-system/components/native/inputs/SearchInput/types.ts (1)
3-6: LGTM on the props contract.Extends Paper’s TextInputProps cleanly;
searchParam+resetInputare clear.packages/design-system/components/native/inputs/index.ts (1)
4-5: Public exports look good.SearchInput and its types are correctly re-exported.
packages/design-system/components/native/inputs/SearchInput/index.tsx (1)
3-3: Confirm@expo/vector-iconsusage in design-system is acceptable.This couples the DS to Expo. If the DS is used outside Expo, prefer Paper’s default icons or
react-native-vector-icons.packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts (1)
7-22: LGTM.Styles look consistent with the design system and RN best practices.
packages/design-system/components/native/illustrations/types.ts (1)
3-5: LGTM; ensureisActiveis not forwarded to<Svg>If consuming illustrations spread
SvgIconPropsinto<Svg {...props} />,isActivewill leak to native and may warn. Destructure it out before spreading:
const { isActive, ...svgProps } = props; return <Svg {...svgProps} />packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts (1)
1-5: Type alias looks good — import path is validValueOf is defined in packages/utils/types/typescript.ts and re‑exported from packages/utils/index.ts (
export type * from './types/typescript'), so importing from '@baseapp-frontend/utils' is correct.packages/design-system/components/native/illustrations/index.ts (1)
1-3: LGTM.Clear and minimal re-exports; type-only export is appropriate.
packages/components/modules/messages/native/NewChatButton/index.tsx (1)
12-20: Add accessibility props to FabButton and verify /create-room routeAdd screen-reader props + testID and ensure FabButton forwards them. I couldn't find a file-based Expo Router route for /create-room — confirm an app/create-room route exists or update the navigation target.
File: packages/components/modules/messages/native/NewChatButton/index.tsx
return ( <FabButton onPress={navigateToCreateRoom} iconName="add" iconSize={28} iconColor={theme.colors.primary.contrast} + accessibilityRole="button" + accessibilityLabel="Start new chat" + accessibilityHint="Opens the new chat screen" + testID="newChatButton" /> )packages/components/modules/messages/native/SearchNotFoundState/styles.ts (1)
11-12: Verify RN support forgapor provide fallback.styles.ts uses
gapin two places but package.json entries showcatalog:react-native(actual RN version unclear) — confirm the RN version supportsgapor replace with explicit sibling margins / a small Spacer component.
- packages/components/modules/messages/native/SearchNotFoundState/styles.ts — container:
gap: 12(≈lines 11–12); textWrapper:gap: 10(≈line 25)packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx (1)
1-14: LGTM! Clean component structure with proper typing.The component follows React Native best practices with typed props and appropriate default values.
packages/components/modules/messages/native/ChatRooms/index.tsx (1)
38-43: Prevent search updates during pending transitions.Good use of transitions for non-blocking UI updates. The early return prevents race conditions.
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (2)
72-85: Good implementation of archive mutation with proper null checks.The archive functionality correctly checks for required data before committing the mutation.
115-116: Address the TODO for lastMessageTime formatting.There's a TODO comment about fixing date formatting. This should be resolved before production.
Can you provide more details about what date formatting fixes are needed? Is this related to timezone handling, relative time display (e.g., "2 hours ago"), or date format localization?
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
5-5: Confirm FlatList sourceUnless you rely on RNGH’s wrappers, prefer
FlatListfromreact-nativeto avoid subtle prop differences.
15-19: No change required — fragment type matches useRoomsList parameterRoomsListFragment is defined on ChatRoomsInterface and useRoomsList explicitly accepts RoomsListFragment$key; ChatRoomsQuery spreads ...RoomsListFragment into profile, so casting targetRef?.me?.profile as RoomsListFragment$key is correct.
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
Outdated
Show resolved
Hide resolved
packages/design-system/components/native/inputs/SearchInput/styles.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/components/modules/__shared__/native/index.ts (1)
2-2: Prefer wider TS compatibility for type re-exports.
export type * fromrequires newer TS; many codebases still useexport type { ... } from. Consider switching to avoid build breaks.Apply:
-export type * from './EmptyView/types' +export type { EmptyViewProps } from './EmptyView/types'packages/components/modules/__shared__/native/EmptyView/styles.ts (1)
22-22: Avoid hard-coded width; make it responsive.
width: 393will misrender on smaller/larger screens. Stretch to container width (center content with alignItems as you already do).Apply:
- width: 393, + alignSelf: 'stretch', + width: '100%',packages/components/modules/__shared__/native/EmptyView/index.tsx (1)
8-8: Forward thestyleprop to the root container.
styleexists in the props type but isn’t applied, preventing consumers from customizing layout.Apply:
-const EmptyView = ({ icon, title, message }: EmptyViewProps) => { +const EmptyView = ({ icon, title, message, style }: EmptyViewProps) => { ... - <View style={styles.container}> + <View style={[styles.container, style]}>Also applies to: 13-13
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/components/modules/__shared__/native/EmptyView/index.tsx(1 hunks)packages/components/modules/__shared__/native/EmptyView/styles.ts(1 hunks)packages/components/modules/__shared__/native/EmptyView/types.ts(1 hunks)packages/components/modules/__shared__/native/index.ts(1 hunks)packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx(1 hunks)packages/components/modules/messages/native/SearchNotFoundState/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx
- packages/components/modules/messages/native/SearchNotFoundState/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-09T22:06:40.026Z
Learnt from: davidbermudez-tsl
PR: silverlogic/baseapp-frontend#209
File: packages/components/modules/content-feed/web/ContentFeed/types.ts:1-1
Timestamp: 2025-04-09T22:06:40.026Z
Learning: The empty ContentFeedProps interface in packages/components/modules/content-feed/web/ContentFeed/types.ts will be populated with properties in a future PR rather than being converted to a type alias.
Applied to files:
packages/components/modules/__shared__/native/EmptyView/types.ts
🧬 Code graph analysis (1)
packages/components/modules/__shared__/native/EmptyView/index.tsx (2)
packages/components/modules/__shared__/native/EmptyView/types.ts (1)
EmptyViewProps(5-10)packages/components/modules/__shared__/native/EmptyView/styles.ts (1)
createStyles(5-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Component Test Packages
🔇 Additional comments (2)
packages/components/modules/__shared__/native/EmptyView/types.ts (1)
5-10: Props shape looks good.Clear, flexible API (icon/title/message/style) for an empty state component.
packages/components/modules/__shared__/native/EmptyView/styles.ts (1)
12-12: Verify RN version supportsgap. Provide fallback if not.
gap/rowGapsupport lands in newer RN; on older versions it’s ignored, collapsing spacing.If you need a fallback, replace
gapwith explicit margins (e.g., addmarginTop: 12to the second block wheniconis present, andmarginTop: 10between texts), or use conditional styles in the component. I can draft that if needed.Also applies to: 26-26
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx (2)
3-8: Replaceany+ non-null assertion with inferred, type-safe props.Avoid
anyandroomRef!. Derive props from the inner component so types stay in sync and the null check narrows correctly.Apply this diff:
import { ChatCard as ChatCardComponent } from './ChatCardComponent' +import type { ComponentProps } from 'react' -const ChatCard = ({ roomRef, isArchived }: { roomRef: any | null; isArchived: boolean }) => { - if (!roomRef) { +type InnerProps = ComponentProps<typeof ChatCardComponent> +type Props = { + roomRef: NonNullable<InnerProps['roomRef']> | null + isArchived: InnerProps['isArchived'] +} + +const ChatCard = ({ roomRef, isArchived }: Props) => { + if (roomRef == null) { return null } - return <ChatCardComponent roomRef={roomRef!} isArchived={isArchived} /> + return <ChatCardComponent roomRef={roomRef} isArchived={isArchived} /> } -export { ChatCard } +export { ChatCard } +export type { Props as ChatCardProps }
3-9: Optional: memoize the wrapper to cut FlatList re-renders.Small wrapper, but in long lists this helps avoid needless re-renders when props don’t change.
Apply on top of the previous suggestion:
-import type { ComponentProps } from 'react' +import { memo } from 'react' +import type { ComponentProps } from 'react' @@ -const ChatCard = ({ roomRef, isArchived }: Props) => { +const ChatCard = memo(({ roomRef, isArchived }: Props) => { if (roomRef == null) { return null } return <ChatCardComponent roomRef={roomRef} isArchived={isArchived} /> -} +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and Lint Packages
- GitHub Check: Component Test Packages
🔇 Additional comments (1)
packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx (1)
1-1: Confirm consumers import named ChatCard (not default).ChatCardComponent exports a named ChatCard and the wrapper re-exports it as named:
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx — export { ChatCard }
packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx — imports { ChatCard as ChatCardComponent } and exports { ChatCard }Search found no consumers importing the wrapper; verify all consumers use a named import: import { ChatCard } from '.../RoomsList/ChatCard' (not a default import).
gabrieleandro
force-pushed
the
feature/BA-2626-rooms-list
branch
from
236e56d to
397c3cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (18)
packages/design-system/components/native/illustrations/SearchingImage/index.tsx (3)
1-1: Memoize the component to avoid unnecessary re-renders.Apply:
-import { FC } from 'react' +import { FC, memo } from 'react' @@ -export default SearchingImage +export default memo(SearchingImage)Also applies to: 57-57
21-28: Add basic accessibility defaults.Set an explicit role and make it non-focusable by default.
Apply:
<Svg width={width} height={height} viewBox="0 0 200 200" color={svgColor} fill="none" + accessibilityRole="image" + focusable={false} {...props} >
11-12: Prefer numeric defaults for width/height.Avoid string defaults unless required for consistency. Numeric is typical for RN.
Apply:
- width = '200', - height = '200', + width = 200, + height = 200,packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (5)
38-50: Avoid double useFragment(TitleFragment); reuse headerFragment for id.Read the header once and derive roomId from it.
- const roomId = useFragment(TitleFragment, roomRef)?.id + // headerFragment read below; derive id from itAnd after defining headerFragment:
- const headerFragment = useFragment(TitleFragment, roomRef) + const headerFragment = useFragment(TitleFragment, roomRef) + const roomId = headerFragment?.id
39-39: Ref typing: prefer useRef(null).Aligns with common RN patterns and BottomSheet’s examples.
- const bottomDrawerRef = useRef<BottomSheetModal | undefined>(undefined) + const bottomDrawerRef = useRef<BottomSheetModal>(null)
64-66: Guard navigation when roomId is absent.Prevents a malformed route when data is stale/not yet available.
- const onChatCardPress = () => { - router.push(`/rooms/${roomId}`) - } + const onChatCardPress = () => { + if (!roomId) return + router.push(`/rooms/${roomId}`) + }
59-63: Avoid rendering “undefined • …”; build the subtitle safely.Compose the caption with available parts only.
const lastMessage = lastMessageFragment.lastMessage?.content const hasUnreadMessages = unreadMessagesCountFragment?.unreadMessages?.markedUnread || !!unreadMessagesCountFragment?.unreadMessages?.count + + const subtitle = [lastMessageTime, lastMessage].filter(Boolean).join(' • ')- <Text + <Text variant="caption" style={styles.lastMessageText} numberOfLines={1} ellipsizeMode="tail" > - {lastMessageTime} • {lastMessage} + {subtitle} </Text>Also applies to: 116-123
105-109: Remove redundant key on Pressable or use a stable id.FlatList manages keys; the current key interpolates an object and is not stable/unique.
<Pressable - key={`chat-card-${roomRef}`} onPress={onChatCardPress} onLongPress={onChatCardLongPress} >packages/design-system/components/native/buttons/FabButton/index.tsx (2)
6-6: Import safe-area hookRequired for the FAB bottom offset.
-import { TouchableOpacity } from 'react-native' +import { TouchableOpacity } from 'react-native' +import { useSafeAreaInsets } from 'react-native-safe-area-context'
1-1: Memoize styles to avoid re-creating StyleSheet per renderSmall perf win; theme changes still re-compute.
-import { FC } from 'react' +import { FC, useMemo } from 'react' @@ - const styles = createStyles(theme) + const styles = useMemo(() => createStyles(theme), [theme])Also applies to: 19-19
packages/design-system/components/native/buttons/index.ts (1)
4-4: Also export FabButton types from the barrelImproves DX and consistency with Button type exports.
export { default as IconButton } from './IconButton' export { default as FabButton } from './FabButton' +export type { FabButtonProps } from './FabButton/types'packages/design-system/components/native/inputs/SearchInput/index.tsx (1)
26-37: Remove now-unused renderClearIcon.If you switch to conditional right TextInput.Icon, this function becomes dead code.
- const renderClearIcon = useCallback( - () => - searchParam && searchParam.length > 0 ? ( - <Ionicons - name="close" - size={24} - color={theme.colors.object.disabled} - onPress={resetInput} - /> - ) : null, - [resetInput, searchParam, theme.colors.object.disabled], - )packages/components/modules/messages/native/ChatRooms/index.tsx (6)
28-29: Tighten selectedTab typing to ChatTabValues.Keeps type safety across tabs and filters.
- const [selectedTab, setSelectedTab] = useState<string>(CHAT_TAB_VALUES.active) + const [selectedTab, setSelectedTab] = useState<ChatTabValues>(CHAT_TAB_VALUES.active)
53-55: Align onChange signature with ChatTabValues.Avoid unnecessary casts and ensure correctness.
- const handleChange = (newValue: string) => { - setSelectedTab(newValue as ChatTabValues) + const handleChange = (newValue: ChatTabValues) => { + setSelectedTab(newValue) }
1-1: Stabilize app-resume callback to avoid listener churn.Inline arrow recreates on every render and resubscribes. Memoize with useCallback.
-import { FC, Suspense, useState, useTransition } from 'react' +import { FC, Suspense, useState, useTransition, useCallback } from 'react' @@ - useAppStateSubscription(() => { - refetch( - { - q: searchParam, - unreadMessages: selectedTab === CHAT_TAB_VALUES.unread, - archived: selectedTab === CHAT_TAB_VALUES.archived, - }, - { fetchPolicy: 'store-and-network' }, - ) - }) + const handleAppResume = useCallback(() => { + refetch( + { + q: searchParam, + unreadMessages: selectedTab === CHAT_TAB_VALUES.unread, + archived: selectedTab === CHAT_TAB_VALUES.archived, + }, + { fetchPolicy: 'store-and-network' }, + ) + }, [refetch, searchParam, selectedTab]) + useAppStateSubscription(handleAppResume)Also applies to: 57-66
27-27: Avoid mixing RHF with local state for this search field.You already manage searchParam; passing control/name adds complexity and risks leaking unknown props downstream. Drop them and avoid an unused control variable.
- const { control, reset } = useForm({ defaultValues: { search: '' } }) + const { reset } = useForm({ defaultValues: { search: '' } })<SearchInput placeholder="Search" onChangeText={handleSearchChange} - control={control} - name="search" searchParam={searchParam} resetInput={resetInput} />Also applies to: 72-79
45-48: resetInput doesn’t need RHF reset once the input is controlled.If you bind value to searchParam in SearchInput, this reset is redundant.
const resetInput = () => { setSearchParam('') - reset({ search: '' }) }
103-107: Groups tab TODO — offer help.Happy to scaffold the Groups tab and guard behind a feature flag until ready.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
packages/components/CHANGELOG.md(1 hunks)packages/components/modules/__shared__/native/EmptyView/index.tsx(1 hunks)packages/components/modules/__shared__/native/EmptyView/styles.ts(1 hunks)packages/components/modules/__shared__/native/EmptyView/types.ts(1 hunks)packages/components/modules/__shared__/native/index.ts(1 hunks)packages/components/modules/messages/native/ChatRooms/index.tsx(1 hunks)packages/components/modules/messages/native/ChatRooms/styles.ts(1 hunks)packages/components/modules/messages/native/NewChatButton/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts(1 hunks)packages/components/modules/messages/native/RoomsList/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/types.ts(1 hunks)packages/components/modules/messages/native/SearchNotFoundState/index.tsx(1 hunks)packages/components/modules/messages/native/index.ts(1 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/native/buttons/FabButton/index.tsx(1 hunks)packages/design-system/components/native/buttons/FabButton/styles.ts(1 hunks)packages/design-system/components/native/buttons/FabButton/types.ts(1 hunks)packages/design-system/components/native/buttons/index.ts(1 hunks)packages/design-system/components/native/illustrations/SearchingImage/index.tsx(1 hunks)packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx(1 hunks)packages/design-system/components/native/illustrations/index.ts(1 hunks)packages/design-system/components/native/illustrations/types.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/index.tsx(1 hunks)packages/design-system/components/native/inputs/SearchInput/styles.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/types.ts(1 hunks)packages/design-system/components/native/inputs/index.ts(1 hunks)packages/design-system/package.json(2 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (34)
- packages/design-system/components/native/inputs/SearchInput/types.ts
- packages/components/modules/messages/native/ChatRooms/styles.ts
- packages/components/modules/messages/native/RoomsList/index.tsx
- packages/components/modules/shared/native/EmptyView/styles.ts
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts
- packages/design-system/components/native/inputs/SearchInput/styles.ts
- packages/wagtail/CHANGELOG.md
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts
- packages/design-system/components/native/inputs/index.ts
- packages/design-system/components/native/illustrations/index.ts
- packages/design-system/package.json
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/index.tsx
- packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts
- packages/design-system/components/native/buttons/FabButton/styles.ts
- packages/components/modules/messages/native/NewChatButton/index.tsx
- packages/components/modules/messages/native/SearchNotFoundState/index.tsx
- packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts
- packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx
- packages/components/modules/messages/native/RoomsList/types.ts
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts
- packages/components/package.json
- packages/components/modules/shared/native/EmptyView/index.tsx
- packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx
- packages/components/modules/shared/native/EmptyView/types.ts
- packages/design-system/components/native/illustrations/types.ts
- packages/components/modules/shared/native/index.ts
- packages/components/CHANGELOG.md
- packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx
- packages/design-system/CHANGELOG.md
- packages/components/modules/messages/native/index.ts
- packages/design-system/components/native/buttons/FabButton/types.ts
- packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts
- packages/wagtail/package.json
- packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts
🧰 Additional context used
🧬 Code graph analysis (6)
packages/components/modules/messages/native/ChatRooms/index.tsx (3)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (2)
CHAT_TAB_VALUES(1-6)CHAT_TAB_LABEL(8-13)packages/utils/hooks/useAppStateSubscription/index.ts (1)
useAppStateSubscription(7-23)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (3)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)packages/components/modules/messages/common/graphql/mutations/ArchiveChatRoom.ts (1)
useArchiveChatRoomMutation(24-63)packages/components/modules/messages/common/utils.ts (1)
useNameAndAvatar(53-58)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
packages/design-system/components/native/buttons/FabButton/index.tsx (2)
packages/design-system/components/native/buttons/FabButton/types.ts (1)
FabButtonProps(4-8)packages/design-system/components/native/buttons/FabButton/styles.ts (1)
createStyles(5-26)
packages/design-system/components/native/inputs/SearchInput/index.tsx (2)
packages/design-system/components/native/inputs/SearchInput/types.ts (1)
SearchInputProps(3-6)packages/design-system/components/native/inputs/SearchInput/styles.ts (1)
createStyles(5-14)
packages/design-system/components/native/illustrations/SearchingImage/index.tsx (1)
packages/design-system/components/native/illustrations/types.ts (1)
SvgIconProps(3-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Lint Packages
- GitHub Check: Component Test Packages
🔇 Additional comments (9)
packages/design-system/components/native/illustrations/SearchingImage/index.tsx (3)
8-14: Confirm color precedence:isActivecurrently overrides a providedcolor.With
isActive = true, the passedcoloris ignored in favor ofcolors.primary.high. Is this intended DS behavior? If not, compute color ascolor ?? (isActive ? colors.primary.high : colors.object.high).Also applies to: 17-19
20-28: LGTM: Prop forwarding order.Spreading
{...props}last lets consumers override width/height/viewBox/fill while keepingcolorcontrolled by the component—good DS ergonomics.If the intent is to allow overriding
colortoo, don't destructurecolorand computesvgColorfromprops.colordirectly, or move{...props}before thecolorprop.
20-53: Confirm react-native-svg supports currentColor — pin/upgrade to >=15.x or test on iOS & Android.package.json declares react-native-svg as "catalog:react-native" (no concrete lockfile version found); currentColor is reliably supported in react-native-svg 15.x+ while older releases had platform bugs — either pin/upgrade to a 15.x+ release or verify fill="currentColor" behavior on both iOS and Android.
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (3)
136-141: Remove commented-out prop.Leftover dev artifact.
<ChatCardOptions bottomDrawerRef={bottomDrawerRef} handleSheetChanges={() => {}} - // type={fieldType} handleArchiveChat={handleArchiveChat}
28-34: Fix ChatCard roomRef type to node-level fragment keys (remove RoomsListFragment$key).This prop type is the source of the fragment mismatch and the unsafe casts below. Accept the node’s fragment keys directly.
Apply this diff:
-const ChatCard = ({ - roomRef, - isArchived, -}: { - roomRef: RoomsListFragment$key - isArchived: boolean -}) => { +const ChatCard = ({ + roomRef, + isArchived, +}: { + roomRef: LastMessageFragment$key & TitleFragment$key & UnreadMessagesCountFragment$key + isArchived: boolean +}) => {Add this helper type near the imports to keep it DRY (optional):
// e.g., just above the component type ChatRoomNodeRef = LastMessageFragment$key & TitleFragment$key & UnreadMessagesCountFragment$key // Then: roomRef: ChatRoomNodeRef
43-54: Remove unsafe casts to fragment keys; use roomRef directly.Casting to unknown to satisfy useFragment defeats Relay’s safety. With the prop type fixed, call useFragment directly.
- const lastMessageFragment = useFragment<LastMessageFragment$key>( - LastMessageFragment, - roomRef as unknown as LastMessageFragment$key, - ) - const headerFragment = useFragment<TitleFragment$key>( - TitleFragment, - roomRef as unknown as TitleFragment$key, - ) - const unreadMessagesCountFragment = useFragment<UnreadMessagesCountFragment$key>( - UnreadMessagesCountFragment, - roomRef as unknown as UnreadMessagesCountFragment$key, - ) + const lastMessageFragment = useFragment(LastMessageFragment, roomRef) + const headerFragment = useFragment(TitleFragment, roomRef) + const unreadMessagesCountFragment = useFragment(UnreadMessagesCountFragment, roomRef)packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (1)
60-75: Harden FlatList: filter null nodes, safe keyExtractor, and guard onEndReached.Prevents passing undefined to ChatCard, avoids unsafe casts, and reduces duplicate loads.
- <FlatList - data={data?.chatRooms?.edges ?? []} - renderItem={({ item }) => { - if (!item?.node) return null - return ( - <ChatCard roomRef={item.node} isArchived={selectedTab === CHAT_TAB_VALUES.archived} /> - ) - }} - onContentSizeChange={(width, height) => loadNextBasedOnHeight(height)} - keyExtractor={(item) => item?.node?.id as string} - onEndReached={() => { - if (hasNext) { - loadNext(10) - } - }} - ListEmptyComponent={handleEmptyState} - /> + <FlatList + data={(data?.chatRooms?.edges ?? []).filter((e) => e?.node)} + renderItem={({ item }) => { + const node = item!.node! + return ( + <ChatCard + roomRef={node} + isArchived={selectedTab === CHAT_TAB_VALUES.archived} + /> + ) + }} + onContentSizeChange={(_w, h) => loadNextBasedOnHeight(h)} + keyExtractor={(item, index) => item?.node?.id ?? `room-edge-${index}`} + onEndReached={() => { + if (hasNext && !isPending) { + loadNext(10) + } + }} + ListEmptyComponent={handleEmptyState} + />packages/design-system/components/native/inputs/SearchInput/index.tsx (1)
39-49: Make input controlled and wire clear to TextInput.Icon (previous feedback not applied).Bind TextInput.value to searchParam so resetInput clears the field, and move onPress to TextInput.Icon while rendering the right icon only when there’s text.
Apply this diff:
return ( <TextInput placeholder={placeholder} mode="outlined" style={styles.searchInput} outlineColor={theme.colors.surface.active} left={<TextInput.Icon icon={renderSearchIcon} />} - right={<TextInput.Icon icon={renderClearIcon} />} + value={searchParam ?? ''} + right={ + searchParam?.length + ? ( + <TextInput.Icon + icon={() => ( + <Ionicons name="close" size={24} color={theme.colors.object.disabled} /> + )} + onPress={resetInput} + /> + ) + : undefined + } onChangeText={onChangeText} {...props} /> )packages/components/modules/messages/native/ChatRooms/index.tsx (1)
50-52: No action needed — empty connections array is safe hereThe hook's updater derives connections from the store via getChatRoomConnections and performs delete/insert operations, so passing [] only makes the @deleteEdge directive a no-op but does not prevent the subscription updater from updating the store.
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
Show resolved
Hide resolved
gabrieleandro
force-pushed
the
feature/BA-2626-rooms-list
branch
from
397c3cd to
d853924
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
packages/design-system/components/native/inputs/SearchInput/index.tsx (1)
39-50: Address the uncontrolled input issue from the previous review.The TextInput remains uncontrolled without binding the
valueprop tosearchParam, which means callingresetInput()won't visually clear the field. This issue was already flagged in a previous review comment.Please implement the suggested fix from the previous review to make this a controlled component and conditionally render the clear icon.
packages/components/modules/messages/native/RoomsList/index.tsx (1)
10-14: Add required fallback prop to Suspense.React.Suspense requires a
fallbackprop. Without it, TypeScript will produce a compile error.Apply this diff:
const SuspendedRoomsList = (props: RoomsListProps) => ( - <Suspense> + <Suspense fallback={null}> <RoomsList {...props} /> </Suspense> )packages/design-system/components/native/buttons/FabButton/index.tsx (1)
11-26: Apply accessibility, theming, and safe-area improvements from previous review.The previous review identified several issues that should be addressed:
- Hardcoded
iconColordefault should derive from theme- Missing
accessibilityRole,activeOpacity, andhitSlopprops- Button position should offset for safe-area insets (home indicator)
- Consumers must provide
accessibilityLabelThese improvements ensure the FAB is theme-aware, accessible, and respects device safe areas.
Refer to the previous review comment for the complete diff and verification script.
packages/components/modules/messages/native/NewChatButton/index.tsx (1)
15-20: Add accessibilityLabel for screen reader support.The FabButton is missing an
accessibilityLabel, making it inaccessible to screen reader users.Apply this diff:
<FabButton + accessibilityLabel="Start new chat" onPress={navigateToCreateRoom} iconName="add" iconSize={28} iconColor={theme.colors.primary.contrast} />packages/components/modules/messages/native/ChatRooms/index.tsx (2)
38-43: Remove isPending guard to prevent dropped keystrokes.The
isPendingcheck prevents user input from being captured during transitions, resulting in lost keystrokes. Users expect immediate visual feedback when typing.Apply this diff:
const handleSearchChange = (text: string) => { - if (isPending) return startTransition(() => { setSearchParam(text) }) }
121-125: Add required fallback prop to Suspense.React.Suspense requires a
fallbackprop. Without it, TypeScript will produce a compile error.Apply this diff:
const SuspendedChatRooms = () => ( - <Suspense> + <Suspense fallback={null}> <ChatRooms /> </Suspense> )packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (1)
61-65: Fix Relay fragment type mismatch for ChatCard.The
item.nodepassed toChatCardhas type issues. According to previous reviews,ChatCardexpectsRoomsListFragment$key, but the node only containsLastMessageFragment,TitleFragment, andUnreadMessagesCountFragment. This type mismatch will cause runtime errors withuseFragment.Update
ChatCardprop types to accept the actual node-level fragment keys instead ofRoomsListFragment$key, and remove any unsafe casts.Refer to the previous review for detailed guidance on fixing the fragment type definitions in ChatCard and ChatCardComponent.
🧹 Nitpick comments (5)
packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
5-44: Consider consolidating duplicate styles to reduce maintenance burden.This file shares 7 out of 8 style definitions with
ChatCardComponent/styles.ts(the only difference being theprofileImagestyle present here). Consider extracting the common styles into a shared module or determining whether both files are necessary, as this duplication violates DRY principles and increases the risk of inconsistencies during future updates.For example, you could create a shared styles module:
// ChatCard/shared-styles.ts export const createSharedStyles = (theme: Theme) => ({ profileCard: { flexDirection: 'row' as const, alignItems: 'center' as const, gap: 12, paddingVertical: 12, }, profileInfo: { flex: 1, }, lastMessageContainer: { flexDirection: 'row' as const, gap: 4, }, profileCounterContainer: { justifyContent: 'center' as const, alignItems: 'center' as const, padding: 4, }, lastMessageText: { color: theme.colors.object.low, }, profileCounter: { backgroundColor: theme.colors.error.main, width: 20, height: 20, borderRadius: 12, justifyContent: 'center' as const, alignItems: 'center' as const, }, profileCounterText: { color: theme.colors.error.contrast, }, });Then import and extend in each file as needed:
// ChatCard/styles.ts import { createSharedStyles } from './shared-styles' export const createStyles = (theme: Theme) => StyleSheet.create({ ...createSharedStyles(theme), profileImage: { width: 48, height: 48, borderRadius: 24, }, })packages/components/modules/messages/native/ChatRooms/styles.ts (1)
7-12: Remove redundant flex property.Using both
flex: 1andflexGrow: 1is redundant. Also,position: 'relative'is the default positioning in React Native and can be removed.Apply this diff:
container: { backgroundColor: theme.colors.surface.background, flexGrow: 1, - flex: 1, - position: 'relative', },packages/design-system/components/native/buttons/FabButton/styles.ts (1)
14-14: Use proper borderRadius for circular button.For a 56x56 button,
borderRadius: 500is excessive. Use28(half the width/height) for a perfect circle.Apply this diff:
- borderRadius: 500, + borderRadius: 28,packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
41-56: Remove isPending guard from useFocusEffect callback.The
isPendingcheck on Line 44 prevents the refetch from occurring when needed. ThestartTransitionwrapper already handles the pending state, so blocking the entire callback is unnecessary.Apply this diff:
useFocusEffect( useCallback(() => { layoutTriggeredRef.current = false - if (isPending) return startTransition(() => { refetch( { q: searchParam, unreadMessages: selectedTab === CHAT_TAB_VALUES.unread, archived: selectedTab === CHAT_TAB_VALUES.archived, }, { fetchPolicy: 'store-and-network' }, ) }) }, [refetch, searchParam, selectedTab, isPending]), )
68-73: Improve keyExtractor safety and add isPending guard to onEndReached.The keyExtractor uses an unsafe type assertion (
as string), andonEndReachedshould checkisPendingbefore loading more data to prevent redundant fetches.Apply this diff:
- keyExtractor={(item) => item?.node?.id as string} + keyExtractor={(item, index) => item?.node?.id ?? `room-${index}`} onEndReached={() => { - if (hasNext) { + if (hasNext && !isPending) { loadNext(10) } }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
packages/components/CHANGELOG.md(1 hunks)packages/components/modules/__shared__/native/EmptyView/index.tsx(1 hunks)packages/components/modules/__shared__/native/EmptyView/styles.ts(1 hunks)packages/components/modules/__shared__/native/EmptyView/types.ts(1 hunks)packages/components/modules/__shared__/native/index.ts(1 hunks)packages/components/modules/messages/native/ChatRooms/index.tsx(1 hunks)packages/components/modules/messages/native/ChatRooms/styles.ts(1 hunks)packages/components/modules/messages/native/NewChatButton/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts(1 hunks)packages/components/modules/messages/native/RoomsList/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/types.ts(1 hunks)packages/components/modules/messages/native/SearchNotFoundState/index.tsx(1 hunks)packages/components/modules/messages/native/index.ts(1 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/native/buttons/FabButton/index.tsx(1 hunks)packages/design-system/components/native/buttons/FabButton/styles.ts(1 hunks)packages/design-system/components/native/buttons/FabButton/types.ts(1 hunks)packages/design-system/components/native/buttons/index.ts(1 hunks)packages/design-system/components/native/illustrations/SearchingImage/index.tsx(1 hunks)packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx(1 hunks)packages/design-system/components/native/illustrations/index.ts(1 hunks)packages/design-system/components/native/illustrations/types.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/index.tsx(1 hunks)packages/design-system/components/native/inputs/SearchInput/styles.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/types.ts(1 hunks)packages/design-system/components/native/inputs/index.ts(1 hunks)packages/design-system/package.json(2 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/wagtail/package.json
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts
- packages/design-system/CHANGELOG.md
- packages/components/modules/messages/native/index.ts
- packages/components/CHANGELOG.md
- packages/design-system/components/native/inputs/SearchInput/styles.ts
- packages/design-system/components/native/illustrations/index.ts
- packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts
- packages/components/modules/shared/native/EmptyView/types.ts
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts
- packages/components/modules/shared/native/index.ts
- packages/components/modules/messages/native/RoomsList/types.ts
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/styles.ts
- packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx
- packages/design-system/components/native/inputs/SearchInput/types.ts
- packages/design-system/components/native/buttons/FabButton/types.ts
- packages/design-system/components/native/illustrations/types.ts
- packages/wagtail/CHANGELOG.md
- packages/design-system/package.json
- packages/components/modules/messages/native/SearchNotFoundState/index.tsx
- packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/index.tsx
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts
- packages/components/modules/messages/native/RoomsList/EmptyChatRoomsState/index.tsx
- packages/components/package.json
🧰 Additional context used
🧬 Code graph analysis (12)
packages/design-system/components/native/inputs/SearchInput/index.tsx (2)
packages/design-system/components/native/inputs/SearchInput/types.ts (1)
SearchInputProps(3-6)packages/design-system/components/native/inputs/SearchInput/styles.ts (1)
createStyles(5-14)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts (1)
createStyles(4-9)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
packages/components/modules/messages/native/RoomsList/index.tsx (1)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)
packages/design-system/components/native/buttons/FabButton/index.tsx (2)
packages/design-system/components/native/buttons/FabButton/types.ts (1)
FabButtonProps(4-8)packages/design-system/components/native/buttons/FabButton/styles.ts (1)
createStyles(5-26)
packages/design-system/components/native/illustrations/SearchingImage/index.tsx (1)
packages/design-system/components/native/illustrations/types.ts (1)
SvgIconProps(3-5)
packages/components/modules/__shared__/native/EmptyView/styles.ts (2)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)
packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx (1)
packages/design-system/components/native/illustrations/types.ts (1)
SvgIconProps(3-5)
packages/components/modules/__shared__/native/EmptyView/index.tsx (2)
packages/components/modules/__shared__/native/EmptyView/types.ts (1)
EmptyViewProps(5-10)packages/components/modules/__shared__/native/EmptyView/styles.ts (1)
createStyles(5-30)
packages/design-system/components/native/buttons/FabButton/styles.ts (2)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts (1)
createStyles(4-9)
packages/components/modules/messages/native/ChatRooms/index.tsx (3)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (2)
CHAT_TAB_VALUES(1-6)CHAT_TAB_LABEL(8-13)packages/utils/hooks/useAppStateSubscription/index.ts (1)
useAppStateSubscription(7-23)
packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (2)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)packages/components/modules/messages/native/RoomsList/RoomsListComponent/styles.ts (1)
createStyles(4-9)
🔇 Additional comments (13)
packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
5-44: LGTM! Well-structured styles with proper theming.The style definitions are well-organized and follow React Native best practices. The use of theme colors ensures consistency, and the dimensions for profile images (48×48 circular) and notification badges (20×20 circular) are appropriate for a chat list UI.
packages/design-system/components/native/illustrations/SearchingImage/index.tsx (3)
1-6: LGTM! Clean and necessary imports.The imports are well-organized and all necessary for the component implementation. The use of the shared
SvgIconPropstype ensures consistency across illustration components.
8-55: LGTM! Well-implemented illustration component.The component implementation is solid:
- Properly typed with
FC<SvgIconProps>for type safety- Theming integration follows design system conventions
- Color logic correctly handles both the
isActivestate and custom colors- The
viewBox="0 0 200 200"aligns with the default dimensions- The use of
fill="currentColor"on Path elements correctly inherits color from the parent SvgThis follows the established pattern for illustration components in the design system.
57-57: LGTM! Proper default export.The default export is correct and enables the component to be re-exported by the illustrations index file, as indicated in the AI summary.
packages/design-system/components/native/illustrations/SocialMediaDiscussionImage/index.tsx (1)
1-125: LGTM! Clean implementation following design-system patterns.The component is well-structured:
- Props are correctly typed using
SvgIconProps- Theming integration with
useThemeprovides consistent color handling- The
isActiveprop correctly switches betweenprimary.highand the default color- SVG attributes (width, height, viewBox, color) are properly configured
fill="currentColor"on Path elements correctly inherits the color from the parent SvgThe implementation follows the design-system conventions and will integrate seamlessly with the rest of the codebase.
packages/design-system/components/native/inputs/index.ts (1)
4-5: LGTM!The SearchInput exports follow the established pattern for other input components and correctly expose both the component and its types to the public API.
packages/design-system/components/native/inputs/SearchInput/index.tsx (3)
1-17: LGTM!The imports and component signature are well-structured and correctly typed against the SearchInputProps interface.
18-24: LGTM!Theme setup and search icon rendering are correctly implemented with proper memoization.
53-53: LGTM!The component is correctly wrapped with
withNativeControllerfor export.packages/design-system/components/native/buttons/index.ts (1)
4-4: LGTM!The FabButton export is correctly added to the public API.
packages/components/modules/__shared__/native/EmptyView/index.tsx (1)
21-25: LGTM!The optional message rendering is correctly implemented with appropriate null checking and styling.
packages/components/modules/__shared__/native/EmptyView/styles.ts (2)
5-14: LGTM!The container styling provides appropriate flex layout with centering and spacing.
15-18: Verify layout behavior when icon is absent.Both
imageWrapperandtextWrapperhaveflex: 1, which would normally split space equally. However, since the parent component conditionally renders theimageWrapper(line 14 in index.tsx), the layout should adapt correctly. Please verify that the empty state displays as expected when no icon is provided.
packages/components/modules/__shared__/native/EmptyView/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/__shared__/native/EmptyView/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/__shared__/native/EmptyView/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/__shared__/native/EmptyView/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/ChatRooms/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/ChatRooms/index.tsx
Outdated
Show resolved
Hide resolved
packages/design-system/components/native/buttons/FabButton/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
Outdated
Show resolved
Hide resolved
packages/design-system/components/native/inputs/SearchInput/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/design-system/components/native/inputs/SearchInput/constants.ts (1)
1-4: LGTM with a minor naming suggestion.The constants are well-defined and serve their purpose. The
DEFAULT_FORM_VALUESclearly represents initial form state. However,FORM_VALUESwith{ search: 'search' }reads more like a field name mapping than actual values—consider renaming toFORM_FIELD_NAMESorFORM_FIELDSfor clarity.export const DEFAULT_FORM_VALUES = { search: '' } -export const FORM_VALUES = { +export const FORM_FIELD_NAMES = { search: 'search', }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/components/CHANGELOG.md(1 hunks)packages/components/modules/__shared__/native/EmptyView/index.tsx(1 hunks)packages/components/modules/__shared__/native/EmptyView/styles.ts(1 hunks)packages/components/modules/messages/native/ChatRooms/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts(1 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/native/inputs/SearchInput/constants.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/styles.ts(1 hunks)packages/design-system/components/native/inputs/index.ts(1 hunks)packages/design-system/components/native/tabs/Tab/styles.ts(1 hunks)packages/design-system/package.json(2 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/design-system/CHANGELOG.md
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardOptions/type.ts
- packages/design-system/package.json
- packages/components/CHANGELOG.md
- packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
- packages/wagtail/CHANGELOG.md
- packages/design-system/components/native/inputs/SearchInput/styles.ts
- packages/components/modules/messages/native/ChatRooms/index.tsx
- packages/components/modules/shared/native/EmptyView/index.tsx
- packages/components/package.json
- packages/wagtail/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
packages/components/modules/__shared__/native/EmptyView/styles.ts (3)
packages/design-system/components/native/inputs/SearchInput/styles.ts (1)
createStyles(5-13)packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
createStyles(5-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (3)
packages/design-system/components/native/inputs/index.ts (1)
4-6: LGTM! Consistent barrel export pattern.The SearchInput re-exports follow the established pattern used by PasswordInput and TextInput, properly exposing the component, types, and constants through the module's public API.
packages/design-system/components/native/tabs/Tab/styles.ts (1)
8-8: Change is appropriate for equal tab distribution in the new ChatRooms feature.The
flexGrow: 1property enables tabs to expand equally across available space, which is the intended behavior for the new four-tab chat interface. Verification confirms only one native Tab usage exists in the codebase (the new ChatRooms component), so no breaking changes to existing implementations.packages/components/modules/__shared__/native/EmptyView/styles.ts (1)
19-26: Previous width issue successfully resolved!The hard-coded
width: 393has been correctly replaced withmaxWidth: '100%', addressing the responsive layout concern raised in the previous review. The layout will now adapt properly across different screen sizes.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (1)
59-71: Consider using InfiniteScrollerView for consistency.As suggested in past reviews, the notifications feature uses
InfiniteScrollerViewwhich provides a more consistent pattern for infinite scrolling lists in this codebase. This would also handle the height-based loading (lines 23-28, 65) and end-reached logic (lines 67-69) more elegantly.Note: Lines 60-66 address previous review concerns about null filtering and key extraction. However, the underlying fragment type mismatch flagged in previous reviews still needs resolution (see earlier comment on lines 16-19).
Based on past review comments. Would you like me to generate a refactor using
InfiniteScrollerViewbased on the notifications pattern?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/components/modules/messages/native/ChatRooms/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)packages/design-system/components/native/buttons/FabButton/index.tsx(1 hunks)packages/design-system/components/native/illustrations/index.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/constants.ts(1 hunks)packages/design-system/components/native/inputs/SearchInput/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/design-system/components/native/illustrations/index.ts
- packages/design-system/components/native/inputs/SearchInput/index.tsx
- packages/components/modules/messages/native/ChatRooms/index.tsx
- packages/design-system/components/native/buttons/FabButton/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (1)
packages/design-system/components/native/inputs/SearchInput/constants.ts (1)
1-11: LGTM! Well-structured form constants.The implementation follows solid TypeScript patterns:
as constprovides proper type narrowing for the form keys- Computed property names ensure type safety between constants and types
- Clear separation of concerns: constants → types → defaults
- The pattern is maintainable and extensible for future form fields
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/design-system/components/native/buttons/FabButton/types.ts (1)
1-8: Runtime bundling issue remains unresolved (duplicate).The value import of
Ioniconson line 1 still pulls the icon module into the runtime bundle whenever this types file is imported, as previously flagged. The consumer atpackages/design-system/components/native/buttons/FabButton/index.tsx:9importsFabButtonPropsas a value, triggering this bundling side-effect.Apply one of these fixes:
Option 1 (recommended): Make this file types-only by using an import type expression:
-import { Ionicons } from '@expo/vector-icons' import type { TouchableOpacityProps } from 'react-native' export interface FabButtonProps extends TouchableOpacityProps { - iconName: keyof typeof Ionicons.glyphMap + iconName: keyof typeof import('@expo/vector-icons').Ionicons.glyphMap iconSize?: number iconColor?: string }And ensure the consumer uses a type-only import in
packages/design-system/components/native/buttons/FabButton/index.tsx:9:import type { FabButtonProps } from './types'Option 2: Keep the current structure but enforce type-only imports at all consumer sites (more fragile).
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
17-20: RemoveuseTransitionand fix the unsafe cast.Two persistent issues:
Line 17:
useTransitionis for synchronous UI updates, not async operations. Wrappingrefetch()instartTransition(line 46) won't track the fetch state;isPendingwill be unreliable. UseisLoadingNextor add a separate loading flag instead.Line 19: The
as RoomsListFragment$keycast bypasses type safety and has caused pipeline errors in past reviews. The fragment mismatch between whatuseRoomsListprovides and whatChatCardexpects needs proper resolution in the type hierarchy.
42-57: Fix dependency array and removeisPendingguard.Two issues:
Line 45: The
isPendingguard can block refetch when a transition is active, potentially showing stale data on focus.Line 56:
startTransitionis used in the callback but missing from dependencies—ESLint's exhaustive-deps would flag this.Since
useTransitionshouldn't be used for async operations (see lines 17-20), the entire approach needs revision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)packages/design-system/components/native/buttons/FabButton/types.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-09T22:06:40.026Z
Learnt from: davidbermudez-tsl
Repo: silverlogic/baseapp-frontend PR: 209
File: packages/components/modules/content-feed/web/ContentFeed/types.ts:1-1
Timestamp: 2025-04-09T22:06:40.026Z
Learning: The empty ContentFeedProps interface in packages/components/modules/content-feed/web/ContentFeed/types.ts will be populated with properties in a future PR rather than being converted to a type alias.
Applied to files:
packages/design-system/components/native/buttons/FabButton/types.ts
🧬 Code graph analysis (1)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Packages
🔇 Additional comments (2)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
24-29: LGTM!The height-based auto-loading logic is well-designed with proper guards.
31-40: LGTM!Empty state handling logic correctly differentiates between search-not-found and no-rooms scenarios.
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
19-19: Critical: Remove unsafe type cast — this breaks Relay's type contract.This unsafe cast
as RoomsListFragment$keyhas been flagged in multiple previous reviews and causes pipeline errors. The fragment type mismatch remains unresolved:targetRef?.me?.profiledoes not guarantee it satisfiesRoomsListFragment$key, and the data passed toChatCard(node-level fragments) doesn't match what the cast claims.Impact: Runtime failures when Relay attempts to read fragments, type safety bypass, and CI/CD pipeline errors.
Correct fix: Update the component hierarchy to use proper fragment types:
- Ensure the query/fragment selecting
targetRefspreads...RoomsListFragmenton the profile- Pass the correctly typed fragment reference without casts
- Update ChatCard to accept the actual node fragment keys (LastMessage, Title, UnreadMessagesCount) instead of RoomsListFragment$key
const { data, loadNext, hasNext, isLoadingNext, refetch } = useRoomsList( - targetRef?.me?.profile as RoomsListFragment$key, + targetRef?.me?.profile, )Then ensure the parent query includes the fragment spread and ChatCard prop types match the node fragments.
17-17: Major: RemoveuseTransition— it's designed for synchronous updates, not async operations.As flagged in previous reviews,
useTransitionis intended for synchronous state changes in React 18+, not for async operations likerefetch. Wrappingrefetch()instartTransitionprovides no benefit, andisPendingcannot accurately track async fetch state.Apply this diff to remove the incorrect pattern:
- const [, startTransition] = useTransition() const { data, loadNext, hasNext, isLoadingNext, refetch } = useRoomsList( targetRef?.me?.profile as RoomsListFragment$key, )useFocusEffect( useCallback(() => { layoutTriggeredRef.current = false - startTransition(() => { - refetch( - { - q: searchParam, - unreadMessages: selectedTab === CHAT_TAB_VALUES.unread, - archived: selectedTab === CHAT_TAB_VALUES.archived, - }, - { fetchPolicy: 'store-and-network' }, - ) - }) - }, [refetch, searchParam, selectedTab, startTransition]), + refetch( + { + q: searchParam, + unreadMessages: selectedTab === CHAT_TAB_VALUES.unread, + archived: selectedTab === CHAT_TAB_VALUES.archived, + }, + { fetchPolicy: 'store-and-network' }, + ) + }, [refetch, searchParam, selectedTab]), )If loading UI is needed during refetch, expose and use a loading/pending state from
useRoomsListinstead.Also applies to: 45-54
🧹 Nitpick comments (1)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (1)
58-73: Good improvements to list rendering.The component now addresses several items from previous reviews:
- Uses
InfiniteScrollerViewas suggested by anicioalexandre- Properly filters null/undefined edges (line 60)
- Includes fallback in key extractor (line 66)
- Uses
isLoadingNextconsistently for pagination (lines 68, 71)Optional refinement: Lines 62-63 use non-null assertions after filtering. While technically safe due to the filter, consider using a type guard for better type safety:
renderItem={({ item }) => { - const node = item!.node! + if (!item?.node) return null + const node = item.node return <ChatCard roomRef={node} isArchived={selectedTab === CHAT_TAB_VALUES.archived} /> }}This eliminates the assertions and provides explicit null handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)packages/components/modules/notifications/native/NotificationsList/index.tsx(0 hunks)packages/design-system/components/native/inputs/SearchInput/index.tsx(1 hunks)packages/design-system/components/native/views/InfiniteScrollerView/index.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/components/modules/notifications/native/NotificationsList/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-system/components/native/inputs/SearchInput/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Lint Packages
- GitHub Check: Component Test Packages
🔇 Additional comments (1)
packages/design-system/components/native/views/InfiniteScrollerView/index.tsx (1)
69-69: No performance regression—review comment misidentifies the change.The original review comment is based on an incorrect assumption. An analysis of the codebase reveals:
estimatedItemSizewas never explicitly set in InfiniteScrollerView—it does not appear anywhere in the codebase history.InfiniteScrollerViewPropsfully extendsFlashListProps<TItem>, meaning consumers can still passestimatedItemSizevia the spread props (...props).- Line 69's change only affects
ListFooterComponenthandling, notestimatedItemSize.- Both current consumers (RoomsListComponent and NotificationsList) don't pass
estimatedItemSize, indicating it was never part of the component's public API in practice.There is no breaking change, no performance regression, and no removal of
estimatedItemSize.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/modules/messages/common/graphql/queries/ChatRoomQuery.ts (1)
6-11: Remove redundant fragment spreads and field selections.The query now has duplicate selections since
ChatRoomFragmentalready spreadsTitleFragmentandMessagesListFragment(lines 13-14 of ChatRoom.ts) and selectsid,isArchived, andparticipantsCount(lines 5, 7, 12 of ChatRoom.ts). While GraphQL merges duplicates, this redundancy reduces maintainability.Apply this diff to remove the redundant direct selections:
query ChatRoomQuery($roomId: ID!) { chatRoom(id: $roomId) { - id - isArchived - participantsCount - ...TitleFragment - ...MessagesListFragment ...ChatRoomFragment } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/components/modules/messages/common/graphql/fragments/ChatRoom.ts(1 hunks)packages/components/modules/messages/common/graphql/queries/ChatRoomQuery.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/type.ts(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/modules/messages/native/RoomsList/ChatCard/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-09T22:06:40.026Z
Learnt from: davidbermudez-tsl
Repo: silverlogic/baseapp-frontend PR: 209
File: packages/components/modules/content-feed/web/ContentFeed/types.ts:1-1
Timestamp: 2025-04-09T22:06:40.026Z
Learning: The empty ContentFeedProps interface in packages/components/modules/content-feed/web/ContentFeed/types.ts will be populated with properties in a future PR rather than being converted to a type alias.
Applied to files:
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/type.ts
🧬 Code graph analysis (2)
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
packages/components/modules/messages/native/RoomsList/types.ts (1)
RoomsListProps(3-7)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (1)
CHAT_TAB_VALUES(1-6)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (5)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/type.ts (1)
ChatCardComponentProps(5-8)packages/components/modules/messages/native/RoomsList/ChatCard/styles.ts (1)
createStyles(5-44)packages/components/modules/messages/common/graphql/mutations/ArchiveChatRoom.ts (1)
useArchiveChatRoomMutation(24-63)packages/components/modules/messages/common/utils.ts (1)
useNameAndAvatar(53-58)packages/utils/functions/date/index.ts (1)
formatRelativeTime(33-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (7)
packages/components/modules/messages/common/graphql/fragments/ChatRoom.ts (1)
1-16: LGTM! Well-structured fragment definition.The ChatRoomFragment is properly defined with appropriate field selections and fragment composition. The use of
@refetchableenables efficient data refetching.packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx (2)
24-40: LGTM — helper functions are well-structured.The
loadNextBasedOnHeightandhandleEmptyStatehelpers are clear and correctly handle their respective responsibilities.
58-73: LGTM — list rendering is robust.The filtering of null edges/nodes (line 60), fallback key extraction (line 66), and pagination guard using
isLoadingNext(line 68) all follow best practices and address previous review concerns.packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (4)
1-28: LGTM — imports are clean and well-organized.All dependencies are properly imported and relevant to the component's functionality.
30-37: LGTM — component setup follows best practices.The hooks are initialized correctly, and the archive mutation is properly wired with the current profile.
52-81: LGTM — derived state and core handlers are well-implemented.The name/avatar derivation, unread logic, navigation, bottom sheet, and archive mutation handler are all correctly structured.
127-139: LGTM — bottom sheet options are properly wired.The
ChatCardOptionscomponent is correctly integrated with all necessary props and handlers.
| const lastMessageFragment = useFragment<LastMessageFragment$key>( | ||
| LastMessageFragment, | ||
| roomRef as unknown as LastMessageFragment$key, | ||
| ) | ||
| const headerFragment = useFragment<TitleFragment$key>( | ||
| TitleFragment, | ||
| roomRef as unknown as TitleFragment$key, | ||
| ) | ||
| const unreadMessagesCountFragment = useFragment<UnreadMessagesCountFragment$key>( | ||
| UnreadMessagesCountFragment, | ||
| roomRef as unknown as UnreadMessagesCountFragment$key, | ||
| ) |
There was a problem hiding this comment.
Remove unsafe type casts — they bypass Relay's type safety.
The three useFragment calls use as unknown as casts to force incompatible types. This is a code smell that indicates the roomRef prop type is incorrect (it should be an intersection, not a union — see the comment on type.ts).
Once you fix the prop type in type.ts (line 6) to use intersection (&), remove all three casts:
- const lastMessageFragment = useFragment<LastMessageFragment$key>(
+ const lastMessageFragment = useFragment(
LastMessageFragment,
- roomRef as unknown as LastMessageFragment$key,
+ roomRef,
)
- const headerFragment = useFragment<TitleFragment$key>(
+ const headerFragment = useFragment(
TitleFragment,
- roomRef as unknown as TitleFragment$key,
+ roomRef,
)
- const unreadMessagesCountFragment = useFragment<UnreadMessagesCountFragment$key>(
+ const unreadMessagesCountFragment = useFragment(
UnreadMessagesCountFragment,
- roomRef as unknown as UnreadMessagesCountFragment$key,
+ roomRef,
)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
around lines 39 to 50, the three useFragment calls currently use unsafe "as
unknown as" casts because the roomRef prop type is incorrect; update the roomRef
prop's type in type.ts (around line 6) from a union to an intersection (use &
instead of |) so it carries all fragment refs, then remove the three casts from
these useFragment calls so each call receives the correctly typed roomRef
directly; ensure any related imports/types are updated so the file compiles
without the forced casts.
| const handleMarkAsUnread = () => { | ||
| alert('Mark as Unread functionality not implemented yet.') | ||
| bottomDrawerRef.current?.close() | ||
| } | ||
| const handleChatDetails = () => { | ||
| alert('Chat Details functionality not implemented yet.') | ||
| bottomDrawerRef.current?.close() | ||
| } | ||
| const handleGoToProfile = () => { | ||
| alert('Go to Profile functionality not implemented yet.') | ||
| bottomDrawerRef.current?.close() | ||
| } | ||
| const handleDeleteChat = () => { | ||
| alert('Delete Chat functionality not implemented yet.') | ||
| bottomDrawerRef.current?.close() | ||
| } |
There was a problem hiding this comment.
Replace placeholder alert() calls with proper implementations or remove handlers.
Four handlers (handleMarkAsUnread, handleChatDetails, handleGoToProfile, handleDeleteChat) use alert() placeholders and are not functional. Either implement them or remove them from the options menu to avoid a degraded user experience.
Do you want me to help scaffold these implementations, or would you prefer to remove these options from the UI until they're ready?
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
Show resolved
Hide resolved
| import { UnreadMessagesCountFragment$key } from '../../../../../../__generated__/UnreadMessagesCountFragment.graphql' | ||
|
|
||
| export interface ChatCardComponentProps { | ||
| roomRef: LastMessageFragment$key | TitleFragment$key | UnreadMessagesCountFragment$key |
There was a problem hiding this comment.
Fix fragment type — use intersection (&), not union (|).
The roomRef must satisfy all three fragment keys simultaneously because ChatCardComponent calls useFragment three times on the same reference (lines 39–50 in index.tsx). A union type (A | B | C) means "one of these," but you need an intersection (A & B & C) meaning "all of these."
Apply this diff:
export interface ChatCardComponentProps {
- roomRef: LastMessageFragment$key | TitleFragment$key | UnreadMessagesCountFragment$key
+ roomRef: LastMessageFragment$key & TitleFragment$key & UnreadMessagesCountFragment$key
isArchived: boolean
}📝 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.
| roomRef: LastMessageFragment$key | TitleFragment$key | UnreadMessagesCountFragment$key | |
| export interface ChatCardComponentProps { | |
| roomRef: LastMessageFragment$key & TitleFragment$key & UnreadMessagesCountFragment$key | |
| isArchived: boolean | |
| } |
🤖 Prompt for AI Agents
In
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/type.ts
around line 6, the roomRef type is declared as a union of fragment keys but must
be an intersection so the same reference satisfies all three fragments; change
the type from "LastMessageFragment$key | TitleFragment$key |
UnreadMessagesCountFragment$key" to "LastMessageFragment$key & TitleFragment$key
& UnreadMessagesCountFragment$key" so useFragment calls on the same ref compile
correctly.
| @@ -0,0 +1,76 @@ | |||
| import { FC, useCallback, useRef, useTransition } from 'react' | |||
There was a problem hiding this comment.
Remove useTransition — it's unnecessary for async operations and isPending is unused.
React's useTransition is designed for synchronous state updates to mark them as non-blocking transitions, not for async operations like refetch. Wrapping refetch() in startTransition (line 45) provides no benefit, and the isPending flag (line 17) won't accurately reflect the fetch state. Additionally, including startTransition in the dependency array (line 55) is unnecessary.
Apply this diff:
-import { FC, useCallback, useRef, useTransition } from 'react'
+import { FC, useCallback, useRef } from 'react' const RoomsListComponent: FC<RoomsListProps> = ({ targetRef, searchParam, selectedTab }) => {
- const [, startTransition] = useTransition()
const { data, loadNext, hasNext, isLoadingNext, refetch } = useRoomsList( useFocusEffect(
useCallback(() => {
layoutTriggeredRef.current = false
- startTransition(() => {
- refetch(
- {
- q: searchParam,
- unreadMessages: selectedTab === CHAT_TAB_VALUES.unread,
- archived: selectedTab === CHAT_TAB_VALUES.archived,
- },
- { fetchPolicy: 'store-and-network' },
- )
- })
- }, [refetch, searchParam, selectedTab, startTransition]),
+ refetch(
+ {
+ q: searchParam,
+ unreadMessages: selectedTab === CHAT_TAB_VALUES.unread,
+ archived: selectedTab === CHAT_TAB_VALUES.archived,
+ },
+ { fetchPolicy: 'store-and-network' },
+ )
+ }, [refetch, searchParam, selectedTab]),
)Also applies to: 17-17, 45-54, 55-55
🤖 Prompt for AI Agents
In
packages/components/modules/messages/native/RoomsList/RoomsListComponent/index.tsx
(lines 1, 17, 45-54, 55), remove useTransition usage: delete the import of
useTransition, remove the isPending variable, stop wrapping refetch() in
startTransition and instead call refetch() directly, and remove startTransition
from any dependency arrays; ensure no other references to
startTransition/isPending remain so the component compiles.
| const RoomsListComponent: FC<RoomsListProps> = ({ targetRef, searchParam, selectedTab }) => { | ||
| const [, startTransition] = useTransition() | ||
| const { data, loadNext, hasNext, isLoadingNext, refetch } = useRoomsList( | ||
| targetRef?.me?.profile as RoomsListFragment$key, |
There was a problem hiding this comment.
Remove unsafe cast to RoomsListFragment$key.
The cast on line 19 bypasses TypeScript's type checking and may cause runtime errors if the fragment reference is incompatible. Verify that targetRef?.me?.profile provides the correct fragment key type expected by useRoomsList, and remove the cast.
If useRoomsList expects a different fragment type than what targetRef.me.profile provides, update the GraphQL query or fragment spreads in the parent component to ensure type compatibility.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (2)
39-50: Remove unsafe type casts by fixing the prop type to use intersection.The three
as unknown ascasts bypass Relay's type safety and indicate theroomRefprop type is incorrect. As flagged in the previous review,ChatCardComponentProps.roomRefshould be an intersection (&) rather than a union (|) so thatroomRefcarries all three fragment keys simultaneously.Step 1: Update the type definition in
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/type.ts(around line 5):export interface ChatCardComponentProps { - roomRef: LastMessageFragment$key | TitleFragment$key | UnreadMessagesCountFragment$key + roomRef: LastMessageFragment$key & TitleFragment$key & UnreadMessagesCountFragment$key isArchived: boolean }Step 2: Remove the casts from this file:
- const lastMessageFragment = useFragment<LastMessageFragment$key>( + const lastMessageFragment = useFragment( LastMessageFragment, - roomRef as unknown as LastMessageFragment$key, + roomRef, ) - const headerFragment = useFragment<TitleFragment$key>( + const headerFragment = useFragment( TitleFragment, - roomRef as unknown as TitleFragment$key, + roomRef, ) - const unreadMessagesCountFragment = useFragment<UnreadMessagesCountFragment$key>( + const unreadMessagesCountFragment = useFragment( UnreadMessagesCountFragment, - roomRef as unknown as UnreadMessagesCountFragment$key, + roomRef, )Based on learnings
83-94: Implement or remove the placeholder handlers.As flagged in the previous review, these four handlers (
handleMarkAsUnread,handleChatDetails,handleGoToProfile,handleDeleteChat) only close the drawer without performing their intended actions. This creates a degraded user experience where menu options appear functional but do nothing.Options:
- Remove these options from the ChatCardOptions menu until they're implemented.
- Implement the functionality for each handler.
Do you want me to help scaffold these implementations, or would you prefer to open an issue to track this work and temporarily remove the non-functional menu items?
🧹 Nitpick comments (3)
packages/components/modules/messages/native/ChatRooms/index.tsx (3)
33-35: Consider extracting form configuration to follow team patterns.Per the earlier feedback, the team pattern is to extract default values, field names, and schema validation to a separate
constants.tsfile (similar to the AccountSettings example referenced). WhileDEFAULT_FORM_VALUESandFORM_VALUESare already imported from the SearchInput module, housing the full form setup (including any validation schema if added later) in a local constants file would improve consistency and maintainability.Based on learnings
52-55: Remove redundantsetValuecall—resetalready handles it.Calling
setValue(FORM_VALUES.search, DEFAULT_FORM_VALUES.search)beforereset(DEFAULT_FORM_VALUES)is redundant becauseresetwill set all fields to their default values.Apply this diff:
const resetInput = () => { - setValue(FORM_VALUES.search, DEFAULT_FORM_VALUES.search) reset(DEFAULT_FORM_VALUES) }
90-106: Consider removing the unreachable Groups tab or adding a disabled tab button.The Tabs component renders Active, Unread, and Archived tabs but omits the Groups tab, even though lines 109–111 handle
CHAT_TAB_VALUES.groups. This makes the Groups tab completely inaccessible in the UI. Either add a disabled Groups tab button with a tooltip explaining it's coming soon, or remove the groups-check logic entirely until the feature is implemented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/components/modules/messages/common/graphql/queries/ChatRoomQuery.ts(1 hunks)packages/components/modules/messages/native/ChatRooms/index.tsx(1 hunks)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/modules/messages/common/graphql/queries/ChatRoomQuery.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-11T13:55:04.375Z
Learnt from: pt-tsl
Repo: silverlogic/baseapp-frontend PR: 212
File: packages/components/modules/messages/MessagesList/index.tsx:80-80
Timestamp: 2025-02-11T13:55:04.375Z
Learning: Avoid using non-null assertions (!.) in TypeScript code unless there's absolute certainty about non-nullability. Instead, prefer handling null types inside the relevant functions/hooks and use optional chaining (?.) for safer access.
Applied to files:
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx
🧬 Code graph analysis (2)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (5)
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/type.ts (1)
ChatCardComponentProps(5-8)packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/styles.ts (1)
createStyles(5-39)packages/components/modules/messages/common/graphql/mutations/ArchiveChatRoom.ts (1)
useArchiveChatRoomMutation(24-63)packages/components/modules/messages/common/utils.ts (1)
useNameAndAvatar(53-58)packages/utils/functions/date/index.ts (1)
formatRelativeTime(33-60)
packages/components/modules/messages/native/ChatRooms/index.tsx (5)
packages/components/modules/messages/native/ChatRooms/styles.ts (1)
createStyles(5-20)packages/design-system/components/native/inputs/SearchInput/constants.ts (3)
SearchInputFormValues(5-7)DEFAULT_FORM_VALUES(9-11)FORM_VALUES(1-3)packages/components/modules/messages/native/RoomsList/RoomsListComponent/constants.ts (2)
CHAT_TAB_VALUES(1-6)CHAT_TAB_LABEL(8-13)packages/components/modules/messages/native/RoomsList/RoomsListComponent/types.ts (1)
ChatTabValues(5-5)packages/utils/hooks/useAppStateSubscription/index.ts (1)
useAppStateSubscription(7-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Component Test Packages
- GitHub Check: Build and Lint Packages
- GitHub Check: Unit Test Packages
🔇 Additional comments (7)
packages/components/modules/messages/native/ChatRooms/index.tsx (4)
107-118: LGTM—conditional rendering is clear and includes a TODO.The conditional logic correctly routes to a placeholder for Groups and renders RoomsList for other tabs. The TODO comment provides clear context for future work.
125-135: LGTM—Suspense fallback requirement satisfied.The
fallbackprop is now properly provided with a LoadingScreen, resolving the TypeScript compile error flagged in the previous review.
57-58: No action needed—emptyconnectionsarray is intentional.The empty array is a consistent pattern across all callers in the codebase (native/ChatRooms, web/AllChatRoomsList, web/GroupChatEdit). Real-time updates to the rooms list happen via the
updaterfunction ingetRoomListSubscriptionConfig, not through Relay connection edge updates. This is the correct architectural design.
46-50: I need to verify the actual SearchInput component implementation to fully assess the concern. Let me query the codebase for the implementation:Based on my research, I need to clarify a critical issue with the original review comment. The function passed to startTransition is not delayed—unlike browser setTimeout, it does not run later. React executes your function immediately, but any state updates scheduled while it is running are marked as Transitions. This means that wrapping
setValueinstartTransitiondoes NOT cause input lag as the review implies.Because startTransition() wraps the setState function, React gives this state updating code a lower priority, ensuring that the input field remains responsive and responds to keystrokes instantly. This is the correct pattern for search/filter scenarios.
However, without access to the SearchInput component implementation in the codebase, I cannot definitively verify whether passing both
control(from react-hook-form) andonChangeTextcreates redundancy or conflicts. The review comment's core concern about input lag is technically incorrect, but there may still be a valid concern about unnecessary complexity or double updates if both props are being used.
Likely an incorrect or invalid review comment.
packages/components/modules/messages/native/RoomsList/ChatCard/ChatCardComponent/index.tsx (3)
30-37: LGTM—component initialization is clean.The setup correctly extracts roomId, initializes the drawer ref, and retrieves the mutation hook and current profile.
52-58: LGTM—data derivation logic is correct.The component properly extracts title, avatar, last message details, and unread status from the fragment data.
68-81: LGTM—archive handler is properly implemented.The handler correctly guards against null values, toggles the archive state, and commits the mutation with the appropriate variables.
|
Abstraction of RoomsList component for RN chat
Summary by CodeRabbit
New Features
Documentation
Chores