Skip to content

Commit f912506

Browse files
committed
feat: review fixes
1 parent 1f9d1a0 commit f912506

14 files changed

Lines changed: 107 additions & 115 deletions

package/src/components/ChannelDetailsScreen/__tests__/ChannelDetailsMemberSection.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ describe('ChannelDetailsMemberSection', () => {
252252
.reverse()
253253
.find((call) => call.member.user?.id === 'u-0');
254254
act(() => {
255-
lastCallForFirstMember?.onPress?.();
255+
lastCallForFirstMember?.onPress?.(lastCallForFirstMember.member);
256256
});
257257

258258
expect(screen.getByTestId('member-actions-sheet-probe')).toBeTruthy();
@@ -271,7 +271,7 @@ describe('ChannelDetailsMemberSection', () => {
271271
.reverse()
272272
.find((call) => call.member.user?.id === 'u-1');
273273
act(() => {
274-
lastCallForSecondMember?.onPress?.();
274+
lastCallForSecondMember?.onPress?.(lastCallForSecondMember.member);
275275
});
276276

277277
expect(onMemberPress).toHaveBeenCalledTimes(1);

package/src/components/ChannelDetailsScreen/__tests__/ChannelDetailsProfile.test.tsx

Lines changed: 7 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
import React from 'react';
22

33
import { render, screen } from '@testing-library/react-native';
4-
import type { Channel, ChannelMemberResponse } from 'stream-chat';
4+
import type { Channel } from 'stream-chat';
55

66
import { ChannelDetailsContextProvider } from '../../../contexts/channelDetailsContext/channelDetailsContext';
77
import { ChatProvider } from '../../../contexts/chatContext/ChatContext';
88
import { ThemeProvider } from '../../../contexts/themeContext/ThemeContext';
99
import { defaultTheme } from '../../../contexts/themeContext/utils/theme';
1010
import { TranslationProvider } from '../../../contexts/translationContext/TranslationContext';
1111
import * as useChannelMuteActiveModule from '../../../hooks/useChannelMuteActive';
12-
import * as useIsDirectChatModule from '../../../hooks/useIsDirectChat';
13-
import * as useChannelMembersStateModule from '../../ChannelList/hooks/useChannelMembersState';
1412
import * as useChannelPreviewDisplayNameModule from '../../ChannelPreview/hooks/useChannelPreviewDisplayName';
1513
import { ChannelDetailsProfile } from '../components/ChannelDetailsProfile';
1614
import * as useChannelDetailsMemberStatusTextModule from '../hooks/useChannelDetailsMemberStatusText';
@@ -28,13 +26,6 @@ jest.mock('../../ui/Avatar/ChannelAvatar', () => {
2826
});
2927

3028
const OWN_USER_ID = 'own-user';
31-
const OTHER_USER_ID = 'other-user';
32-
33-
const buildMember = (id: string, online = false): ChannelMemberResponse =>
34-
({
35-
user: { id, online },
36-
user_id: id,
37-
}) as unknown as ChannelMemberResponse;
3829

3930
const buildChannel = () =>
4031
({
@@ -64,20 +55,12 @@ const renderProfile = ({ channel = buildChannel() }: { channel?: Channel } = {})
6455
);
6556

6657
describe('ChannelDetailsProfile', () => {
67-
let useIsDirectChatSpy: jest.SpyInstance;
68-
let useChannelMembersStateSpy: jest.SpyInstance;
6958
let useChannelPreviewDisplayNameSpy: jest.SpyInstance;
7059
let useChannelDetailsMemberStatusTextSpy: jest.SpyInstance;
7160
let useChannelMuteActiveSpy: jest.SpyInstance;
7261

7362
beforeEach(() => {
7463
channelAvatarCalls.length = 0;
75-
useIsDirectChatSpy = jest
76-
.spyOn(useIsDirectChatModule, 'useIsDirectChat')
77-
.mockReturnValue(false);
78-
useChannelMembersStateSpy = jest
79-
.spyOn(useChannelMembersStateModule, 'useChannelMembersState')
80-
.mockReturnValue({});
8164
useChannelPreviewDisplayNameSpy = jest
8265
.spyOn(useChannelPreviewDisplayNameModule, 'useChannelPreviewDisplayName')
8366
.mockReturnValue('Display Name');
@@ -121,51 +104,20 @@ describe('ChannelDetailsProfile', () => {
121104
});
122105
});
123106

124-
describe('group chats', () => {
125-
beforeEach(() => {
126-
useIsDirectChatSpy.mockReturnValue(false);
127-
});
128-
129-
it('renders the group status text as the subtitle', () => {
107+
describe('subtitle', () => {
108+
it('renders the status text returned by useChannelDetailsMemberStatusText', () => {
130109
renderProfile();
131110
expect(screen.getByText('12 members, 3 online')).toBeTruthy();
132111
});
133112

134-
it('does not render a subtitle when the group status text is empty', () => {
135-
useChannelDetailsMemberStatusTextSpy.mockReturnValue('');
136-
renderProfile();
137-
expect(screen.queryByText('12 members, 3 online')).toBeNull();
138-
});
139-
});
140-
141-
describe('direct chats', () => {
142-
beforeEach(() => {
143-
useIsDirectChatSpy.mockReturnValue(true);
144-
});
145-
146-
it('renders "Online" when the other member is online', () => {
147-
useChannelMembersStateSpy.mockReturnValue({
148-
[OWN_USER_ID]: buildMember(OWN_USER_ID, true),
149-
[OTHER_USER_ID]: buildMember(OTHER_USER_ID, true),
150-
});
113+
it('renders a direct-chat status string from the hook', () => {
114+
useChannelDetailsMemberStatusTextSpy.mockReturnValue('Online');
151115
renderProfile();
152116
expect(screen.getByText('Online')).toBeTruthy();
153117
});
154118

155-
it('does not render a subtitle when the other member is offline', () => {
156-
useChannelMembersStateSpy.mockReturnValue({
157-
[OWN_USER_ID]: buildMember(OWN_USER_ID, true),
158-
[OTHER_USER_ID]: buildMember(OTHER_USER_ID, false),
159-
});
160-
renderProfile();
161-
expect(screen.queryByText('Online')).toBeNull();
162-
});
163-
164-
it('ignores the group status text in direct chats', () => {
165-
useChannelMembersStateSpy.mockReturnValue({
166-
[OWN_USER_ID]: buildMember(OWN_USER_ID, true),
167-
[OTHER_USER_ID]: buildMember(OTHER_USER_ID, false),
168-
});
119+
it('does not render a subtitle when the status text is empty', () => {
120+
useChannelDetailsMemberStatusTextSpy.mockReturnValue('');
169121
renderProfile();
170122
expect(screen.queryByText('12 members, 3 online')).toBeNull();
171123
});

package/src/components/ChannelDetailsScreen/__tests__/members/ChannelMemberList.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ describe('ChannelMemberList', () => {
221221
const captured = itemProbeCalls.find((p) => p.member.user?.id === 'bob');
222222

223223
expect(list.queryByTestId('member-actions-sheet-probe')).toBeNull();
224-
act(() => captured?.onPress?.());
224+
act(() => captured?.onPress?.(bob));
225225
expect(list.getByTestId('member-actions-sheet-probe').props.children).toBe('bob');
226226

227227
act(() => sheetProbeCalls[sheetProbeCalls.length - 1]?.onClose?.());
@@ -240,7 +240,7 @@ describe('ChannelMemberList', () => {
240240
render((renderItem as any)({ index: 0, item: alice, separators: {} as never }));
241241
const captured = itemProbeCalls.find((p) => p.member.user?.id === 'alice');
242242

243-
act(() => captured?.onPress?.());
243+
act(() => captured?.onPress?.(alice));
244244

245245
expect(onMemberPress).toHaveBeenCalledTimes(1);
246246
expect(onMemberPress.mock.calls[0][0].user?.id).toBe('alice');

package/src/components/ChannelDetailsScreen/__tests__/useChannelDetailsMemberStatusText.test.tsx

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { Channel } from 'stream-chat';
55

66
import { TranslationProvider } from '../../../contexts/translationContext/TranslationContext';
77
import { useChannelMemberCount } from '../../../hooks/useChannelMemberCount';
8+
import { useIsDirectChat } from '../../../hooks/useIsDirectChat';
89
import { useChannelMembersState } from '../../ChannelList/hooks/useChannelMembersState';
910
import { useChannelOnlineMemberCount } from '../../ChannelList/hooks/useChannelOnlineMemberCount';
1011
import { useChannelDetailsMemberStatusText } from '../hooks/useChannelDetailsMemberStatusText';
@@ -13,6 +14,10 @@ jest.mock('../../../hooks/useChannelMemberCount', () => ({
1314
useChannelMemberCount: jest.fn(),
1415
}));
1516

17+
jest.mock('../../../hooks/useIsDirectChat', () => ({
18+
useIsDirectChat: jest.fn(() => false),
19+
}));
20+
1621
jest.mock('../../ChannelList/hooks/useChannelMembersState', () => ({
1722
useChannelMembersState: jest.fn(() => ({})),
1823
}));
@@ -28,7 +33,11 @@ const t = ((key: string, options?: Record<string, unknown>) => {
2833
return key;
2934
}) as never;
3035

31-
const channel = { cid: 'messaging:test' } as unknown as Channel;
36+
const OWN_USER_ID = 'own-user';
37+
const channel = {
38+
cid: 'messaging:test',
39+
getClient: () => ({ userID: OWN_USER_ID }),
40+
} as unknown as Channel;
3241

3342
const renderStatusText = () => {
3443
const wrapper = ({ children }: { children: React.ReactNode }) => (
@@ -54,16 +63,6 @@ describe('useChannelDetailsMemberStatusText', () => {
5463
expect(result.current).toBe('5 members, 2 online');
5564
});
5665

57-
it('falls back to the loaded member map length when the reactive count is missing', () => {
58-
(useChannelMemberCount as jest.Mock).mockReturnValue(0);
59-
(useChannelOnlineMemberCount as jest.Mock).mockReturnValue(1);
60-
(useChannelMembersState as jest.Mock).mockReturnValue({ a: {}, b: {}, c: {} });
61-
62-
const { result } = renderStatusText();
63-
64-
expect(result.current).toBe('3 members, 1 online');
65-
});
66-
6766
it('recomputes when the online count changes', () => {
6867
(useChannelMemberCount as jest.Mock).mockReturnValue(4);
6968
(useChannelOnlineMemberCount as jest.Mock).mockReturnValue(0);
@@ -77,4 +76,42 @@ describe('useChannelDetailsMemberStatusText', () => {
7776

7877
expect(result.current).toBe('4 members, 3 online');
7978
});
79+
80+
describe('direct chats', () => {
81+
beforeEach(() => {
82+
(useIsDirectChat as jest.Mock).mockReturnValue(true);
83+
});
84+
85+
it('returns "Online" when the other member is online', () => {
86+
(useChannelMembersState as jest.Mock).mockReturnValue({
87+
[OWN_USER_ID]: { user: { id: OWN_USER_ID, online: true } },
88+
other: { user: { id: 'other-user', online: true } },
89+
});
90+
91+
const { result } = renderStatusText();
92+
93+
expect(result.current).toBe('Online');
94+
});
95+
96+
it('returns an empty string when the other member is offline', () => {
97+
(useChannelMembersState as jest.Mock).mockReturnValue({
98+
[OWN_USER_ID]: { user: { id: OWN_USER_ID, online: true } },
99+
other: { user: { id: 'other-user', online: false } },
100+
});
101+
102+
const { result } = renderStatusText();
103+
104+
expect(result.current).toBe('');
105+
});
106+
107+
it('ignores the current user when resolving the other member', () => {
108+
(useChannelMembersState as jest.Mock).mockReturnValue({
109+
[OWN_USER_ID]: { user: { id: OWN_USER_ID, online: true } },
110+
});
111+
112+
const { result } = renderStatusText();
113+
114+
expect(result.current).toBe('');
115+
});
116+
});
80117
});

package/src/components/ChannelDetailsScreen/components/ChannelDetailsMemberSection.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,7 @@ export const ChannelDetailsMemberSection = () => {
113113
{visible.map((member) => {
114114
if (!member.user?.id) return null;
115115
return (
116-
<ChannelMemberItem
117-
key={member.user.id}
118-
member={member}
119-
onPress={() => handleMemberPress(member)}
120-
/>
116+
<ChannelMemberItem key={member.user.id} member={member} onPress={handleMemberPress} />
121117
);
122118
})}
123119
</View>

package/src/components/ChannelDetailsScreen/components/ChannelDetailsProfile.tsx

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@ import { useChannelDetailsContext } from '../../../contexts/channelDetailsContex
66
import { useTheme } from '../../../contexts/themeContext/ThemeContext';
77
import { useTranslationContext } from '../../../contexts/translationContext/TranslationContext';
88
import { useChannelMuteActive } from '../../../hooks/useChannelMuteActive';
9-
import { useIsDirectChat } from '../../../hooks/useIsDirectChat';
109
import { Mute } from '../../../icons/mute';
1110
import { primitives } from '../../../theme';
12-
import { useChannelMembersState } from '../../ChannelList/hooks/useChannelMembersState';
1311
import { useChannelPreviewDisplayName } from '../../ChannelPreview/hooks/useChannelPreviewDisplayName';
1412
import { ChannelAvatar } from '../../ui/Avatar/ChannelAvatar';
1513
import { useChannelDetailsMemberStatusText } from '../hooks/useChannelDetailsMemberStatusText';
@@ -33,21 +31,11 @@ export const ChannelDetailsProfile = () => {
3331
semantics,
3432
},
3533
} = useTheme();
36-
const isDirect = useIsDirectChat(channel);
37-
const members = useChannelMembersState(channel);
3834
const displayName = useChannelPreviewDisplayName(channel);
39-
const groupStatusText = useChannelDetailsMemberStatusText(channel);
35+
const subtitle = useChannelDetailsMemberStatusText(channel);
4036
const muted = useChannelMuteActive(channel);
4137
const styles = useStyles();
4238

43-
const subtitle = useMemo(() => {
44-
if (!isDirect) return groupStatusText;
45-
const otherMember = Object.values(members).find(
46-
(member) => member.user?.id !== channel.getClient().userID,
47-
);
48-
return otherMember?.user?.online ? t('Online') : '';
49-
}, [channel, groupStatusText, isDirect, members, t]);
50-
5139
return (
5240
<View style={[styles.container, containerOverride]}>
5341
<ChannelAvatar channel={channel} showBorder={false} size='2xl' />

package/src/components/ChannelDetailsScreen/components/members/ChannelAllMembersModal.tsx

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import { useChannelDetailsContext } from '../../../../contexts/channelDetailsCon
44
import { useComponentsContext } from '../../../../contexts/componentsContext/ComponentsContext';
55
import { useOwnCapabilitiesContext } from '../../../../contexts/ownCapabilitiesContext/OwnCapabilitiesContext';
66
import { useTranslationContext } from '../../../../contexts/translationContext/TranslationContext';
7+
import { useChannelMemberCount } from '../../../../hooks';
78
import { UserAdd } from '../../../../icons/user-add';
89
import { Button } from '../../../ui/Button/Button';
9-
import { useChannelDetailsMembersPreview } from '../../hooks/useChannelDetailsMembersPreview';
1010
import { ChannelDetailsModal } from '../modal/Modal';
1111
import { ModalHeader } from '../modal/ModalHeader';
1212

@@ -16,22 +16,20 @@ export type ChannelAllMembersModalProps = {
1616
visible: boolean;
1717
};
1818

19-
/**
20-
* @experimental This component is experimental and is subject to change.
21-
*/
22-
export const ChannelAllMembersModal = ({
19+
type ChannelAllMembersModalContentProps = Omit<ChannelAllMembersModalProps, 'visible'>;
20+
21+
const ChannelAllMembersModalContent = ({
2322
onAddMembersPress,
2423
onClose,
25-
visible,
26-
}: ChannelAllMembersModalProps) => {
24+
}: ChannelAllMembersModalContentProps) => {
2725
const { channel } = useChannelDetailsContext();
2826
const { ChannelMemberList } = useComponentsContext();
2927
const { t } = useTranslationContext();
3028
const { updateChannelMembers } = useOwnCapabilitiesContext();
31-
const { total } = useChannelDetailsMembersPreview(channel);
29+
const total = useChannelMemberCount(channel);
3230

3331
return (
34-
<ChannelDetailsModal onClose={onClose} visible={visible}>
32+
<>
3533
<ModalHeader
3634
onClose={onClose}
3735
rightAction={
@@ -51,6 +49,22 @@ export const ChannelAllMembersModal = ({
5149
title={t('{{count}} members', { count: total })}
5250
/>
5351
<ChannelMemberList />
54-
</ChannelDetailsModal>
52+
</>
5553
);
5654
};
55+
56+
/**
57+
* @experimental This component is experimental and is subject to change.
58+
*/
59+
export const ChannelAllMembersModal = ({
60+
onAddMembersPress,
61+
onClose,
62+
visible,
63+
}: ChannelAllMembersModalProps) => (
64+
// The content lives in a child component so its hooks (member-state subscription,
65+
// member preview sort) only run while the modal is open. React Native's `Modal`
66+
// renders `null` when `visible` is false, so the child never mounts until then.
67+
<ChannelDetailsModal onClose={onClose} visible={visible}>
68+
<ChannelAllMembersModalContent onAddMembersPress={onAddMembersPress} onClose={onClose} />
69+
</ChannelDetailsModal>
70+
);

package/src/components/ChannelDetailsScreen/components/members/ChannelMemberItem.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export type ChannelMemberItemSize = 'sm' | 'lg';
1818

1919
export type ChannelMemberItemProps = {
2020
member: ChannelMemberResponse;
21-
onPress?: () => void;
21+
onPress?: (member: ChannelMemberResponse) => void;
2222
/**
2323
* Visual size of the row.
2424
* - `'sm'` (default) renders the compact list row with a small avatar, regular-weight name, and a trailing role label.
@@ -123,7 +123,7 @@ const ChannelMemberItemInner = ({
123123
<Pressable
124124
accessibilityLabel={accessibilityLabel}
125125
accessibilityRole='button'
126-
onPress={onPress}
126+
onPress={() => onPress(member)}
127127
style={({ pressed }) => [
128128
isLarge ? styles.containerLarge : styles.container,
129129
pressed ? { backgroundColor: semantics.backgroundUtilityPressed } : null,

package/src/components/ChannelDetailsScreen/components/members/ChannelMemberList.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export const ChannelMemberList = ({ additionalFlatListProps }: ChannelMemberList
4646

4747
const renderItem = useCallback(
4848
({ item }: { item: ChannelMemberResponse }) => (
49-
<ChannelMemberItem member={item} onPress={() => handleMemberPress(item)} />
49+
<ChannelMemberItem member={item} onPress={handleMemberPress} />
5050
),
5151
[ChannelMemberItem, handleMemberPress],
5252
);

0 commit comments

Comments
 (0)