From 1c0e201edf3d164843e7de42902c5ecf45c33eec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 8 Apr 2025 10:14:01 +0200 Subject: [PATCH] Refactor chips (#8740) * Refactor chips * Fix tsc * Rename showXButton --- .../copy_channel_link_box/index.ts | 3 - app/components/chips/base_chip.test.tsx | 111 ++++++++++++++++++ .../index.tsx => chips/base_chip.tsx} | 94 +++++++++------ app/components/chips/constants.ts | 5 + app/components/chips/selected_chip.test.tsx | 42 +++++++ app/components/chips/selected_chip.tsx | 34 ++++++ .../chips/selected_user_chip.test.tsx | 40 +++++++ app/components/chips/selected_user_chip.tsx | 33 ++++++ app/components/chips/user_chip.test.tsx | 57 +++++++++ app/components/chips/user_chip.tsx | 57 +++++++++ .../floating_text_chips_input/index.tsx | 8 +- app/components/selected_users/index.tsx | 21 ++-- .../selected_users/selected_user.tsx | 66 ----------- app/components/user_avatars_stack/index.tsx | 11 +- .../user_avatars_stack/users_list/index.tsx | 4 +- app/screens/invite/selection.tsx | 8 +- 16 files changed, 461 insertions(+), 133 deletions(-) create mode 100644 app/components/chips/base_chip.test.tsx rename app/components/{selected_chip/index.tsx => chips/base_chip.tsx} (53%) create mode 100644 app/components/chips/constants.ts create mode 100644 app/components/chips/selected_chip.test.tsx create mode 100644 app/components/chips/selected_chip.tsx create mode 100644 app/components/chips/selected_user_chip.test.tsx create mode 100644 app/components/chips/selected_user_chip.tsx create mode 100644 app/components/chips/user_chip.test.tsx create mode 100644 app/components/chips/user_chip.tsx delete mode 100644 app/components/selected_users/selected_user.tsx diff --git a/app/components/channel_actions/copy_channel_link_box/index.ts b/app/components/channel_actions/copy_channel_link_box/index.ts index d14ec47de..61c51c878 100644 --- a/app/components/channel_actions/copy_channel_link_box/index.ts +++ b/app/components/channel_actions/copy_channel_link_box/index.ts @@ -1,9 +1,6 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - import {withDatabase, withObservables} from '@nozbe/watermelondb/react'; import {of as of$} from 'rxjs'; import {switchMap} from 'rxjs/operators'; diff --git a/app/components/chips/base_chip.test.tsx b/app/components/chips/base_chip.test.tsx new file mode 100644 index 000000000..d10deb547 --- /dev/null +++ b/app/components/chips/base_chip.test.tsx @@ -0,0 +1,111 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {fireEvent} from '@testing-library/react-native'; +import React from 'react'; + +import {renderWithIntlAndTheme} from '@test/intl-test-helper'; + +import BaseChip from './base_chip'; + +describe('BaseChip', () => { + const onPressMock = jest.fn(); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should render correctly with default props', () => { + const {getByText} = renderWithIntlAndTheme( + , + ); + + expect(getByText('Test Label')).toBeTruthy(); + }); + + it('should render with the X button when showRemoveOption is true', () => { + const {getByTestId} = renderWithIntlAndTheme( + , + ); + + expect(getByTestId('base_chip.remove.button')).toBeTruthy(); + }); + + it('should not render the X button when showRemoveOption is false', () => { + const {queryByTestId} = renderWithIntlAndTheme( + , + ); + + expect(queryByTestId('base_chip.remove.button')).toBeNull(); + }); + + it('should call onPress when the X button is pressed', () => { + const {getByTestId} = renderWithIntlAndTheme( + , + ); + + fireEvent.press(getByTestId('base_chip.remove.button')); + expect(onPressMock).toHaveBeenCalledTimes(1); + }); + + it('should call onPress when the chip is pressed without X button', () => { + const {getByTestId} = renderWithIntlAndTheme( + , + ); + + fireEvent.press(getByTestId('base_chip.chip_button')); + expect(onPressMock).toHaveBeenCalledTimes(1); + }); + + it('should handle animations when showAnimation is true', () => { + const {getByTestId} = renderWithIntlAndTheme( + , + ); + + expect(getByTestId('base_chip').props.entering).toBeTruthy(); + expect(getByTestId('base_chip').props.exiting).toBeTruthy(); + }); + + it('should not handle animations when showAnimation is false', () => { + const {getByTestId} = renderWithIntlAndTheme( + , + ); + + expect(getByTestId('base_chip').props.entering).toBeUndefined(); + expect(getByTestId('base_chip').props.exiting).toBeUndefined(); + }); +}); diff --git a/app/components/selected_chip/index.tsx b/app/components/chips/base_chip.tsx similarity index 53% rename from app/components/selected_chip/index.tsx rename to app/components/chips/base_chip.tsx index f961d5259..a52fb869d 100644 --- a/app/components/selected_chip/index.tsx +++ b/app/components/chips/base_chip.tsx @@ -1,8 +1,8 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useCallback} from 'react'; -import {Text, TouchableOpacity, useWindowDimensions, type StyleProp, type ViewStyle} from 'react-native'; +import React from 'react'; +import {Text, TouchableOpacity, useWindowDimensions} from 'react-native'; import Animated, {FadeIn, FadeOut} from 'react-native-reanimated'; import CompassIcon from '@components/compass_icon'; @@ -11,17 +11,17 @@ import {nonBreakingString} from '@utils/strings'; import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; import {typography} from '@utils/typography'; +import {CHIP_BOTTOM_MARGIN, CHIP_HEIGHT} from './constants'; + type SelectedChipProps = { - id: string; - text: string; - extra?: React.ReactNode; - onRemove: (id: string) => void; + onPress: () => void; testID?: string; - containerStyle?: StyleProp; + showRemoveOption?: boolean; + showAnimation?: boolean; + label: string; + prefix?: JSX.Element; } -export const USER_CHIP_HEIGHT = 32; -export const USER_CHIP_BOTTOM_MARGIN = 8; const FADE_DURATION = 100; const getStyleFromTheme = makeStyleSheetFromTheme((theme) => { @@ -31,9 +31,9 @@ const getStyleFromTheme = makeStyleSheetFromTheme((theme) => { justifyContent: 'center', flexDirection: 'row', borderRadius: 16, - height: USER_CHIP_HEIGHT, + height: CHIP_HEIGHT, backgroundColor: changeOpacity(theme.centerChannelColor, 0.08), - marginBottom: USER_CHIP_BOTTOM_MARGIN, + marginBottom: CHIP_BOTTOM_MARGIN, marginRight: 8, paddingHorizontal: 7, }, @@ -49,50 +49,68 @@ const getStyleFromTheme = makeStyleSheetFromTheme((theme) => { }; }); -export default function SelectedChip({ - id, - text, - extra, - onRemove, +export default function BaseChip({ testID, - containerStyle, + onPress, + showRemoveOption, + showAnimation, + label, + prefix, }: SelectedChipProps) { const theme = useTheme(); const style = getStyleFromTheme(theme); const dimensions = useWindowDimensions(); - const containerStyles = [style.container, containerStyle]; - - const onPress = useCallback(() => { - onRemove(id); - }, [onRemove, id]); - - return ( - - {extra} + const chipContent = ( + <> + {prefix} - {nonBreakingString(text)} + {nonBreakingString(label)} + + ); + + let content; + if (showRemoveOption) { + content = ( + <> + {chipContent} + + + + + ); + } else { + content = ( - + {chipContent} + ); + } + return ( + + {content} ); } diff --git a/app/components/chips/constants.ts b/app/components/chips/constants.ts new file mode 100644 index 000000000..a8bdf3dc4 --- /dev/null +++ b/app/components/chips/constants.ts @@ -0,0 +1,5 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +export const CHIP_HEIGHT = 32; +export const CHIP_BOTTOM_MARGIN = 8; diff --git a/app/components/chips/selected_chip.test.tsx b/app/components/chips/selected_chip.test.tsx new file mode 100644 index 000000000..6a622e4f3 --- /dev/null +++ b/app/components/chips/selected_chip.test.tsx @@ -0,0 +1,42 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {render} from '@testing-library/react-native'; +import React from 'react'; + +import BaseChip from './base_chip'; +import SelectedChip from './selected_chip'; + +jest.mock('./base_chip', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(BaseChip).mockImplementation((props) => React.createElement('BaseChip', {...props})); + +describe('SelectedChip', () => { + const mockOnRemove = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should render with the correct props', () => { + const {getByTestId} = render( + , + ); + + const baseChip = getByTestId('selected-chip'); + expect(baseChip.props.label).toBe('Test Chip'); + expect(baseChip.props.showRemoveOption).toBe(true); + expect(baseChip.props.showAnimation).toBe(true); + expect(baseChip.props.prefix).toBeUndefined(); + baseChip.props.onPress(); + expect(mockOnRemove).toHaveBeenCalledTimes(1); + expect(mockOnRemove).toHaveBeenCalledWith('test-id'); + }); +}); diff --git a/app/components/chips/selected_chip.tsx b/app/components/chips/selected_chip.tsx new file mode 100644 index 000000000..efb0b17e8 --- /dev/null +++ b/app/components/chips/selected_chip.tsx @@ -0,0 +1,34 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React, {useCallback} from 'react'; + +import BaseChip from './base_chip'; + +type SelectedChipProps = { + id: string; + text: string; + onRemove: (id: string) => void; + testID?: string; +} + +export default function SelectedChip({ + id, + text, + onRemove, + testID, +}: SelectedChipProps) { + const onPress = useCallback(() => { + onRemove(id); + }, [onRemove, id]); + + return ( + + ); +} diff --git a/app/components/chips/selected_user_chip.test.tsx b/app/components/chips/selected_user_chip.test.tsx new file mode 100644 index 000000000..6ef3ab2b2 --- /dev/null +++ b/app/components/chips/selected_user_chip.test.tsx @@ -0,0 +1,40 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {render} from '@testing-library/react-native'; +import React from 'react'; + +import TestHelper from '@test/test_helper'; + +import SelectedUserChip from './selected_user_chip'; +import UserChip from './user_chip'; + +jest.mock('./user_chip', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(UserChip).mockImplementation((props) => React.createElement('UserChip', {...props})); + +describe('SelectedUserChip', () => { + const mockOnPress = jest.fn(); + const mockUser = TestHelper.fakeUser({id: 'user-id', username: 'test-user'}); + + it('should render with the correct props', () => { + const {getByTestId} = render( + , + ); + + const userChip = getByTestId('selected-user-chip'); + expect(userChip.props.user).toBe(mockUser); + expect(userChip.props.teammateNameDisplay).toBe('username'); + expect(userChip.props.showRemoveOption).toBe(true); + expect(userChip.props.showAnimation).toBe(true); + userChip.props.onPress(); + expect(mockOnPress).toHaveBeenCalledTimes(1); + }); +}); diff --git a/app/components/chips/selected_user_chip.tsx b/app/components/chips/selected_user_chip.tsx new file mode 100644 index 000000000..efb098e80 --- /dev/null +++ b/app/components/chips/selected_user_chip.tsx @@ -0,0 +1,33 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import UserChip from './user_chip'; + +import type UserModel from '@typings/database/models/servers/user'; + +type SelectedChipProps = { + user: UserModel | UserProfile; + onPress: (id: string) => void; + testID?: string; + teammateNameDisplay: string; +} + +export default function SelectedUserChip({ + testID, + user, + teammateNameDisplay, + onPress, +}: SelectedChipProps) { + return ( + + ); +} diff --git a/app/components/chips/user_chip.test.tsx b/app/components/chips/user_chip.test.tsx new file mode 100644 index 000000000..f6d4c69d5 --- /dev/null +++ b/app/components/chips/user_chip.test.tsx @@ -0,0 +1,57 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import ProfilePicture from '@components/profile_picture'; +import {renderWithIntlAndTheme} from '@test/intl-test-helper'; +import TestHelper from '@test/test_helper'; + +import BaseChip from './base_chip'; +import UserChip from './user_chip'; + +jest.mock('./base_chip', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(BaseChip).mockImplementation((props) => React.createElement('BaseChip', {...props})); + +jest.mock('@components/profile_picture', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(ProfilePicture).mockImplementation((props) => React.createElement('ProfilePicture', {...props})); + +describe('UserChip', () => { + const mockOnPress = jest.fn(); + const mockUser = TestHelper.fakeUser({id: 'user-id', username: 'test-user'}); + + it('should render with the correct props', () => { + const {getByTestId} = renderWithIntlAndTheme( + , + ); + + const baseChip = getByTestId('user-chip'); + expect(baseChip.props.label).toBe('test-user'); + expect(baseChip.props.showRemoveOption).toBe(true); + expect(baseChip.props.showAnimation).toBe(true); + + expect(baseChip.props.prefix).toBeDefined(); + const profilePicture = baseChip.props.prefix; + expect(profilePicture.props.author).toBe(mockUser); + expect(profilePicture.props.size).toBe(20); + expect(profilePicture.props.iconSize).toBe(20); + expect(profilePicture.props.testID).toBe('user-chip.profile_picture'); + + baseChip.props.onPress(); + expect(mockOnPress).toHaveBeenCalledTimes(1); + expect(mockOnPress).toHaveBeenCalledWith('user-id'); + }); +}); diff --git a/app/components/chips/user_chip.tsx b/app/components/chips/user_chip.tsx new file mode 100644 index 000000000..651a30456 --- /dev/null +++ b/app/components/chips/user_chip.tsx @@ -0,0 +1,57 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React, {useCallback, useMemo} from 'react'; +import {useIntl} from 'react-intl'; + +import ProfilePicture from '@components/profile_picture'; +import {displayUsername} from '@utils/user'; + +import BaseChip from './base_chip'; + +import type UserModel from '@typings/database/models/servers/user'; + +type SelectedChipProps = { + user: UserModel | UserProfile; + onPress: (id: string) => void; + testID?: string; + teammateNameDisplay: string; + showRemoveOption?: boolean; + showAnimation?: boolean; +} + +export default function UserChip({ + testID, + user, + teammateNameDisplay, + onPress: receivedOnPress, + showRemoveOption, + showAnimation, +}: SelectedChipProps) { + const intl = useIntl(); + + const onPress = useCallback(() => { + receivedOnPress(user.id); + }, [receivedOnPress, user.id]); + + const name = displayUsername(user, intl.locale, teammateNameDisplay); + const picture = useMemo(() => ( + + ), [testID, user]); + + return ( + + ); +} diff --git a/app/components/floating_text_chips_input/index.tsx b/app/components/floating_text_chips_input/index.tsx index d2beb5e30..d977a48b5 100644 --- a/app/components/floating_text_chips_input/index.tsx +++ b/app/components/floating_text_chips_input/index.tsx @@ -31,8 +31,9 @@ import Animated, { Easing, } from 'react-native-reanimated'; +import {CHIP_HEIGHT} from '@components/chips/constants'; +import SelectedChip from '@components/chips/selected_chip'; import CompassIcon from '@components/compass_icon'; -import SelectedChip, {USER_CHIP_HEIGHT} from '@components/selected_chip'; import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; import {typography} from '@utils/typography'; @@ -67,7 +68,7 @@ const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => ({ paddingHorizontal: 0, paddingTop: 0, paddingBottom: 0, - height: USER_CHIP_HEIGHT, + height: CHIP_HEIGHT, flexGrow: 1, flexShrink: 0, flexBasis: 'auto', @@ -222,7 +223,7 @@ const FloatingTextChipsInput = forwardRef(({ } res.push({ borderWidth: focusedLabel ? BORDER_FOCUSED_WIDTH : BORDER_DEFAULT_WIDTH, - minHeight: (USER_CHIP_HEIGHT * 2.5) + ((focusedLabel ? BORDER_FOCUSED_WIDTH : BORDER_DEFAULT_WIDTH) * 2), + minHeight: (CHIP_HEIGHT * 2.5) + ((focusedLabel ? BORDER_FOCUSED_WIDTH : BORDER_DEFAULT_WIDTH) * 2), }); if (focused) { @@ -282,7 +283,6 @@ const FloatingTextChipsInput = forwardRef(({ text={chipValue} testID={`${testID}.${chipValue}`} onRemove={onChipRemove} - containerStyle={styles.chipContainer} /> ))} { const u = []; - for (const [id, user] of Object.entries(selectedIds)) { + for (const user of Object.values(selectedIds)) { if (!user) { continue; } + const userItemTestID = `${testID}.${user.id}`; + u.push( - , ); } diff --git a/app/components/selected_users/selected_user.tsx b/app/components/selected_users/selected_user.tsx deleted file mode 100644 index 4eadf2bcc..000000000 --- a/app/components/selected_users/selected_user.tsx +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import React, {useCallback} from 'react'; -import {useIntl} from 'react-intl'; - -import ProfilePicture from '@components/profile_picture'; -import SelectedChip from '@components/selected_chip'; -import {displayUsername} from '@utils/user'; - -import type UserModel from '@typings/database/models/servers/user'; - -type Props = { - - /* - * How to display the names of users. - */ - teammateNameDisplay: string; - - /* - * The user that this component represents. - */ - user: UserProfile|UserModel; - - /* - * A handler function that will deselect a user when clicked on. - */ - onRemove: (id: string) => void; - - /* - * The test ID. - */ - testID?: string; -} - -export default function SelectedUser({ - teammateNameDisplay, - user, - onRemove, - testID, -}: Props) { - const intl = useIntl(); - - const onPress = useCallback((id: string) => { - onRemove(id); - }, [onRemove]); - - const userItemTestID = `${testID}.${user.id}`; - - return ( - - )} - onRemove={onPress} - testID={userItemTestID} - /> - ); -} diff --git a/app/components/user_avatars_stack/index.tsx b/app/components/user_avatars_stack/index.tsx index 0ef8f8bb4..9c0e52ee9 100644 --- a/app/components/user_avatars_stack/index.tsx +++ b/app/components/user_avatars_stack/index.tsx @@ -8,11 +8,11 @@ import {Platform, type StyleProp, Text, type TextStyle, TouchableOpacity, View, import FormattedText from '@components/formatted_text'; import {useTheme} from '@context/theme'; import {useIsTablet} from '@hooks/device'; +import {usePreventDoubleTap} from '@hooks/utils'; import {BOTTOM_SHEET_ANDROID_OFFSET} from '@screens/bottom_sheet'; import {TITLE_HEIGHT} from '@screens/bottom_sheet/content'; import {bottomSheet} from '@screens/navigation'; import {bottomSheetSnapPoint} from '@utils/helpers'; -import {preventDoubleTap} from '@utils/tap'; import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; import {typography} from '@utils/typography'; @@ -25,7 +25,7 @@ const OVERFLOW_DISPLAY_LIMIT = 99; const USER_ROW_HEIGHT = 40; type Props = { - channelId: string; + channelId?: string; location: string; users: UserModel[]; breakAt?: number; @@ -100,10 +100,11 @@ const UserAvatarsStack = ({ overflowTextStyle, }: Props) => { const theme = useTheme(); + const style = getStyleSheet(theme); const intl = useIntl(); const isTablet = useIsTablet(); - const showParticipantsList = useCallback(preventDoubleTap(() => { + const showParticipantsList = usePreventDoubleTap(useCallback(() => { const renderContent = () => ( <> {!isTablet && ( @@ -141,13 +142,11 @@ const UserAvatarsStack = ({ title: intl.formatMessage({id: 'mobile.participants.header', defaultMessage: 'Thread Participants'}), theme, }); - }), [isTablet, theme, users, channelId, location]); + }, [users, intl, theme, isTablet, style.listHeader, style.listHeaderText, channelId, location])); const displayUsers = users.slice(0, breakAt); const overflowUsersCount = Math.min(users.length - displayUsers.length, OVERFLOW_DISPLAY_LIMIT); - const style = getStyleSheet(theme); - return ( ) : ( - ));