From df2eb9eb100f0da589c60b10f4bfbbfe5da06f71 Mon Sep 17 00:00:00 2001 From: dilshodmackbook-sketch <279628751+dilshodmackbook-sketch@users.noreply.github.com> Date: Mon, 13 Jul 2026 15:58:09 +0500 Subject: [PATCH 1/5] perf: paint checkbox optimistically and run the selection update as a transition --- .../components/ListSelectionButton.tsx | 21 +++- src/pages/NewChatPage/index.tsx | 35 +++++-- tests/ui/NewChatPageTest.tsx | 45 +++++++++ tests/unit/ListSelectionButtonTest.tsx | 97 +++++++++++++++++++ 4 files changed, 188 insertions(+), 10 deletions(-) create mode 100644 tests/unit/ListSelectionButtonTest.tsx diff --git a/src/components/SelectionList/components/ListSelectionButton.tsx b/src/components/SelectionList/components/ListSelectionButton.tsx index 16a2c9eaed4a..57d7120c43f1 100644 --- a/src/components/SelectionList/components/ListSelectionButton.tsx +++ b/src/components/SelectionList/components/ListSelectionButton.tsx @@ -5,7 +5,7 @@ import CONST from '@src/CONST'; import type {StyleProp, ViewStyle} from 'react-native'; -import React from 'react'; +import React, {useState} from 'react'; type ListSelectionButtonProps = { /** The item to render the selection button for */ @@ -50,13 +50,28 @@ function ListSelectionButton({ }: ListSelectionButtonProps & {role: typeof CONST.ROLE.CHECKBOX | typeof CONST.ROLE.RADIO}) { const label = accessibilityLabel ?? item.text ?? ''; + // Paint the checkmark immediately on press, even when the parent defers the (expensive) selection-state + // update in a transition. The optimistic value is dropped as soon as the item prop catches up + // (state-adjustment-during-render, see https://react.dev/reference/react/useState#storing-information-from-previous-renders). + const isCheckedProp = item.isSelected ?? false; + const [prevCheckedProp, setPrevCheckedProp] = useState(isCheckedProp); + const [optimisticChecked, setOptimisticChecked] = useState(null); + if (prevCheckedProp !== isCheckedProp) { + setPrevCheckedProp(isCheckedProp); + setOptimisticChecked(null); + } + const isChecked = optimisticChecked ?? isCheckedProp; + return ( onSelectRow(item)} + isChecked={isChecked} + onPress={() => { + setOptimisticChecked(!isChecked); + onSelectRow(item); + }} disabled={disabled} style={style} containerStyle={containerStyle} diff --git a/src/pages/NewChatPage/index.tsx b/src/pages/NewChatPage/index.tsx index 39cbd47bb5a8..ff75d1a34f7f 100755 --- a/src/pages/NewChatPage/index.tsx +++ b/src/pages/NewChatPage/index.tsx @@ -49,7 +49,7 @@ import {useFocusEffect} from '@react-navigation/native'; import {guidedSetupAndTourStatusSelector} from '@selectors/Onboarding'; import passthroughPolicyTagListSelector from '@selectors/PolicyTagList'; import reject from 'lodash/reject'; -import React, {useEffect, useImperativeHandle, useRef, useState} from 'react'; +import React, {startTransition, useEffect, useImperativeHandle, useRef, useState} from 'react'; import {Keyboard} from 'react-native'; import type SelectedOption from './types'; @@ -268,6 +268,13 @@ function NewChatPage({ref}: NewChatPageProps) { areOptionsInitialized, } = useOptions(reportAttributesDerived); + // Latest committed selection, kept in a ref so back-to-back toggles compose off the newest list instead of a + // stale render snapshot while the deferred (startTransition) selection update below is still catching up. + const latestSelectedOptionsRef = useRef(selectedOptions); + useEffect(() => { + latestSelectedOptionsRef.current = selectedOptions; + }, [selectedOptions]); + // Selected rows are marked in place by getValidOptions (isSelected), so the checkmark stays with the row instead of jumping to the top. // In group selection mode the self DM stays visible (so the list doesn't shift and jump the scroll position) but is made non-selectable. const recentReportsData = selectedOptions.length ? recentReports.map((option) => (option.isSelfDM ? {...option, isDisabled: true} : option)) : recentReports; @@ -316,23 +323,37 @@ function NewChatPage({ref}: NewChatPageProps) { * Removes a selected option from list if already selected. If not already selected add this option to the list. */ const toggleOption = (option: ListItem & Partial) => { - const isOptionInList = !!option.isSelected; + // Read membership from the latest selection (not the item or a render snapshot) so rapid toggles stay + // correct while the deferred list update below is still catching up to a previous press. + const currentSelectedOptions = latestSelectedOptionsRef.current; + const isOptionInList = currentSelectedOptions.some((selectedOption) => selectedOption.login === option.login); let newSelectedOptions: SelectedOption[]; if (isOptionInList) { - newSelectedOptions = reject(selectedOptions, (selectedOption) => selectedOption.login === option.login); + newSelectedOptions = reject(currentSelectedOptions, (selectedOption) => selectedOption.login === option.login); } else { - newSelectedOptions = [...selectedOptions, {...option, isSelected: true, reportID: option.reportID, keyForList: `${option.keyForList ?? option.reportID}`}]; + newSelectedOptions = [...currentSelectedOptions, {...option, isSelected: true, reportID: option.reportID, keyForList: `${option.keyForList ?? option.reportID}`}]; } + // Advance the ref immediately so a second tap landing before the transition commits composes off this + // result instead of dropping it. External updates re-sync the ref via the effect above. + latestSelectedOptionsRef.current = newSelectedOptions; + selectionListRef.current?.clearInputAfterSelect(); if (!canUseTouchScreen()) { selectionListRef.current?.focusTextInput(); } - setSelectedOptions(newSelectedOptions); - if (personalData?.login && personalData?.accountID) { + // The selection update fans out into the whole options pipeline (getValidOptions -> filterAndOrderOptions -> + // sections -> useFlattenedSections -> every visible row re-render). Run it as a transition so the tapped + // checkbox (which shows optimistic feedback) can paint first and the heavy re-render doesn't block the frame. + startTransition(() => { + setSelectedOptions(newSelectedOptions); + + if (!personalData?.login || !personalData?.accountID) { + return; + } const participants: SelectedParticipant[] = [ ...newSelectedOptions.map((selectedOption) => ({ login: selectedOption.login, @@ -344,7 +365,7 @@ function NewChatPage({ref}: NewChatPageProps) { }, ]; setGroupDraft({participants}); - } + }); }; /** diff --git a/tests/ui/NewChatPageTest.tsx b/tests/ui/NewChatPageTest.tsx index e3e790602d7a..d11d15094aae 100644 --- a/tests/ui/NewChatPageTest.tsx +++ b/tests/ui/NewChatPageTest.tsx @@ -199,6 +199,51 @@ describe('NewChatPage', () => { expect(scrollToSpy).not.toHaveBeenCalled(); }); + it('should toggle selection correctly via the row checkbox with the deferred selection update', async () => { + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, fakePersonalDetails); + }); + render(, {wrapper}); + await waitForBatchedUpdatesWithAct(); + act(() => { + triggerTransitionEnd(); + }); + + // Enter group-selection mode by selecting one user. + const addButton = await waitFor(() => { + const button = screen.getAllByText(translateLocal('newChatPage.addToGroup')).at(0); + expect(button).toBeTruthy(); + return button; + }); + if (!addButton) { + return; + } + fireEvent.press(addButton); + await waitForBatchedUpdatesWithAct(); + expect(screen.getByText(translateLocal('common.next'))).toBeVisible(); + + // Toggle the selected row's checkbox (it is only rendered on selected rows). The selection update now runs + // as a transition, so this covers that deferring it does not change the toggle semantics: the user is + // unselected and group-selection mode is exited. + const checkedCheckbox = screen.getAllByTestId(new RegExp(`^${CONST.SELECTION_BUTTON_TEST_ID}`)).at(0); + expect(checkedCheckbox).toBeTruthy(); + if (!checkedCheckbox) { + return; + } + fireEvent.press(checkedCheckbox); + await waitForBatchedUpdatesWithAct(); + expect(screen.queryByText(translateLocal('common.next'))).toBeNull(); + expect(screen.queryAllByTestId(new RegExp(`^${CONST.SELECTION_BUTTON_TEST_ID}`))).toHaveLength(0); + + // And selecting again from the empty state still works. + const buttonAfter = screen.getAllByText(translateLocal('newChatPage.addToGroup')).at(0); + if (buttonAfter) { + fireEvent.press(buttonAfter); + } + await waitForBatchedUpdatesWithAct(); + expect(screen.getByText(translateLocal('common.next'))).toBeVisible(); + }); + describe('should not display "Add to group" button on expensify emails', () => { const excludedGroupEmails = CONST.EXPENSIFY_EMAILS.filter((value) => value !== CONST.EMAIL.CONCIERGE && value !== CONST.EMAIL.NOTIFICATIONS).map((email) => [email]); diff --git a/tests/unit/ListSelectionButtonTest.tsx b/tests/unit/ListSelectionButtonTest.tsx new file mode 100644 index 000000000000..1d54d2ac3f83 --- /dev/null +++ b/tests/unit/ListSelectionButtonTest.tsx @@ -0,0 +1,97 @@ +import {fireEvent, render, screen} from '@testing-library/react-native'; + +import ListCheckbox from '@components/SelectionList/components/ListCheckbox'; +import type {ListItem} from '@components/SelectionList/ListItem/types'; + +import CONST from '@src/CONST'; + +import React from 'react'; + +const TEST_ID = `${CONST.SELECTION_BUTTON_TEST_ID}Test User`; + +const buildItem = (isSelected: boolean): ListItem => ({ + text: 'Test User', + keyForList: 'test-user', + isSelected, +}); + +// The pressable renders with accessible={false}, so role-based queries and toBeChecked() can't reach it - +// read the checked flag from the accessibility state it exposes instead. +const getCheckedState = (): unknown => { + const props: unknown = screen.getByTestId(TEST_ID).props; + if (typeof props !== 'object' || props === null || !('accessibilityState' in props)) { + return undefined; + } + const state = props.accessibilityState; + if (typeof state !== 'object' || state === null || !('checked' in state)) { + return undefined; + } + return state.checked; +}; + +describe('ListSelectionButton', () => { + it('paints the checkmark optimistically on press, before the parent commits the selection', () => { + const onSelectRow = jest.fn(); + // The parent defers its selection update (e.g. in a transition), so item.isSelected does not change on press. + render( + , + ); + + expect(getCheckedState()).toBe(false); + + fireEvent.press(screen.getByTestId(TEST_ID)); + + expect(onSelectRow).toHaveBeenCalledTimes(1); + // The checkmark flips immediately even though the item prop has not caught up yet. + expect(getCheckedState()).toBe(true); + }); + + it('drops the optimistic value once the item prop catches up', () => { + const {rerender} = render( + , + ); + + fireEvent.press(screen.getByTestId(TEST_ID)); + expect(getCheckedState()).toBe(true); + + // The parent's deferred update lands and confirms the selection. + rerender( + , + ); + expect(getCheckedState()).toBe(true); + + // A later external update unselects the item - the stale optimistic value must not mask it. + rerender( + , + ); + expect(getCheckedState()).toBe(false); + }); + + it('toggles the checkmark back on a second press before the parent commits', () => { + render( + , + ); + + fireEvent.press(screen.getByTestId(TEST_ID)); + expect(getCheckedState()).toBe(true); + + // A second rapid press (still no prop change) reverts the optimistic checkmark. + fireEvent.press(screen.getByTestId(TEST_ID)); + expect(getCheckedState()).toBe(false); + }); +}); From 0bead3c22f6312751916e8621c2abaa3ecfeaad9 Mon Sep 17 00:00:00 2001 From: dilshodmackbook-sketch <279628751+dilshodmackbook-sketch@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:09:53 +0500 Subject: [PATCH 2/5] fix: keep selected radio checked on re-press in ListSelectionButton The optimistic checkmark also backs radio rows. Re-pressing an already- selected radio inverted it to unchecked, and since its isSelected prop never changes the reset never fired, leaving it stuck unchecked. A radio press only ever selects, so paint it checked instead of toggling. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/ListSelectionButton.tsx | 5 ++++- tests/unit/ListSelectionButtonTest.tsx | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/components/SelectionList/components/ListSelectionButton.tsx b/src/components/SelectionList/components/ListSelectionButton.tsx index 57d7120c43f1..f42524a4092b 100644 --- a/src/components/SelectionList/components/ListSelectionButton.tsx +++ b/src/components/SelectionList/components/ListSelectionButton.tsx @@ -69,7 +69,10 @@ function ListSelectionButton({ accessibilityLabel={label} isChecked={isChecked} onPress={() => { - setOptimisticChecked(!isChecked); + // A checkbox toggles, so flip the current value. A radio press only ever selects, so paint it + // checked - inverting an already-checked radio would leave it stuck unchecked because its + // isSelected prop never changes to trigger the reset above. + setOptimisticChecked(role === CONST.ROLE.RADIO ? true : !isChecked); onSelectRow(item); }} disabled={disabled} diff --git a/tests/unit/ListSelectionButtonTest.tsx b/tests/unit/ListSelectionButtonTest.tsx index 1d54d2ac3f83..f14823ca2e84 100644 --- a/tests/unit/ListSelectionButtonTest.tsx +++ b/tests/unit/ListSelectionButtonTest.tsx @@ -1,6 +1,7 @@ import {fireEvent, render, screen} from '@testing-library/react-native'; import ListCheckbox from '@components/SelectionList/components/ListCheckbox'; +import ListRadioButton from '@components/SelectionList/components/ListRadioButton'; import type {ListItem} from '@components/SelectionList/ListItem/types'; import CONST from '@src/CONST'; @@ -94,4 +95,20 @@ describe('ListSelectionButton', () => { fireEvent.press(screen.getByTestId(TEST_ID)); expect(getCheckedState()).toBe(false); }); + + it('keeps a selected radio checked when it is pressed again', () => { + // A radio press only ever selects, so re-pressing an already-selected radio (whose isSelected prop + // never changes) must not optimistically flip it to unchecked. + render( + , + ); + + expect(getCheckedState()).toBe(true); + + fireEvent.press(screen.getByTestId(TEST_ID)); + expect(getCheckedState()).toBe(true); + }); }); From d136ff4b4c753638d61d516e9e45def126932971 Mon Sep 17 00:00:00 2001 From: dilshodmackbook-sketch <279628751+dilshodmackbook-sketch@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:16:39 +0500 Subject: [PATCH 3/5] chore: re-trigger CI (flaky unrelated SplitTest) Co-Authored-By: Claude Opus 4.8 (1M context) From 8d4940fd56cf80d25c56d6e28853657bcbd5b9fa Mon Sep 17 00:00:00 2001 From: dilshodmackbook-sketch <279628751+dilshodmackbook-sketch@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:32:56 +0500 Subject: [PATCH 4/5] fix: reset optimistic check on row recycle and build group draft from latest selection Address two Codex P2 findings: - ListSelectionButton: the optimistic-check reset only watched item.isSelected, so a FlashList-recycled cell could keep a stale optimisticChecked=true and show the wrong checkmark. Track item.keyForList identity in the reset too. - NewChatPage.createGroup: read from latestSelectedOptionsRef so tapping "Add to group" then quickly Next (while the deferred selection transition is still pending) doesn't rebuild the draft from a stale selectedOptions snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/ListSelectionButton.tsx | 9 ++++--- src/pages/NewChatPage/index.tsx | 5 +++- tests/unit/ListSelectionButtonTest.tsx | 26 +++++++++++++++++-- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/components/SelectionList/components/ListSelectionButton.tsx b/src/components/SelectionList/components/ListSelectionButton.tsx index f42524a4092b..abfb09b2897c 100644 --- a/src/components/SelectionList/components/ListSelectionButton.tsx +++ b/src/components/SelectionList/components/ListSelectionButton.tsx @@ -53,11 +53,14 @@ function ListSelectionButton({ // Paint the checkmark immediately on press, even when the parent defers the (expensive) selection-state // update in a transition. The optimistic value is dropped as soon as the item prop catches up // (state-adjustment-during-render, see https://react.dev/reference/react/useState#storing-information-from-previous-renders). + // Track the row identity (keyForList) alongside isSelected: SelectionList rows render through FlashList and are + // recycled, so the same component instance can receive a different item with the same isSelected value - resetting + // on identity change prevents a stale optimistic checkmark from leaking onto the recycled row. const isCheckedProp = item.isSelected ?? false; - const [prevCheckedProp, setPrevCheckedProp] = useState(isCheckedProp); + const [prevItem, setPrevItem] = useState({key: item.keyForList, checked: isCheckedProp}); const [optimisticChecked, setOptimisticChecked] = useState(null); - if (prevCheckedProp !== isCheckedProp) { - setPrevCheckedProp(isCheckedProp); + if (prevItem.key !== item.keyForList || prevItem.checked !== isCheckedProp) { + setPrevItem({key: item.keyForList, checked: isCheckedProp}); setOptimisticChecked(null); } const isChecked = optimisticChecked ?? isCheckedProp; diff --git a/src/pages/NewChatPage/index.tsx b/src/pages/NewChatPage/index.tsx index ff75d1a34f7f..708747518b83 100755 --- a/src/pages/NewChatPage/index.tsx +++ b/src/pages/NewChatPage/index.tsx @@ -466,7 +466,10 @@ function NewChatPage({ref}: NewChatPageProps) { if (!personalData?.login || !personalData.accountID) { return; } - const selectedParticipants: SelectedParticipant[] = selectedOptions.map((option) => ({ + // Read from the latest-selection ref (not the render snapshot): the Next button stays tappable while the + // deferred selection transition is still pending, so tapping "Add to group" then quickly Next must not rebuild + // the draft from a stale selectedOptions that drops the just-added participant. + const selectedParticipants: SelectedParticipant[] = latestSelectedOptionsRef.current.map((option) => ({ login: option?.login, accountID: option.accountID ?? CONST.DEFAULT_NUMBER_ID, })); diff --git a/tests/unit/ListSelectionButtonTest.tsx b/tests/unit/ListSelectionButtonTest.tsx index f14823ca2e84..332ad36782ec 100644 --- a/tests/unit/ListSelectionButtonTest.tsx +++ b/tests/unit/ListSelectionButtonTest.tsx @@ -10,9 +10,9 @@ import React from 'react'; const TEST_ID = `${CONST.SELECTION_BUTTON_TEST_ID}Test User`; -const buildItem = (isSelected: boolean): ListItem => ({ +const buildItem = (isSelected: boolean, keyForList = 'test-user'): ListItem => ({ text: 'Test User', - keyForList: 'test-user', + keyForList, isSelected, }); @@ -96,6 +96,28 @@ describe('ListSelectionButton', () => { expect(getCheckedState()).toBe(false); }); + it('drops the optimistic value when the row is recycled to a different item', () => { + const {rerender} = render( + , + ); + + fireEvent.press(screen.getByTestId(TEST_ID)); + expect(getCheckedState()).toBe(true); + + // FlashList recycles the cell to a different, still-unselected item (same isSelected, new keyForList). + // The optimistic checkmark from the previous row must not leak onto the recycled one. + rerender( + , + ); + expect(getCheckedState()).toBe(false); + }); + it('keeps a selected radio checked when it is pressed again', () => { // A radio press only ever selects, so re-pressing an already-selected radio (whose isSelected prop // never changes) must not optimistically flip it to unchecked. From 865a2b0bf349665b7fd3228a9375d87343358b92 Mon Sep 17 00:00:00 2001 From: dilshodmackbook-sketch <279628751+dilshodmackbook-sketch@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:29:29 +0500 Subject: [PATCH 5/5] fix: make stale-control presses safe during the pending selection window - A re-press on a still-visible Add to group button while the deferred selection update is pending now stays an idempotent add instead of silently cancelling the pending one; removal intent comes from the pressed row's rendered state. - createGroup returns early when the latest selection is empty, so a stale Next press right after removing the last member no longer builds a draft containing only the current user. - Membership matching falls back to accountID/reportID for login-less options so two distinct login-less DMs no longer collide on undefined === undefined. Co-Authored-By: Claude Fable 5 --- src/pages/NewChatPage/index.tsx | 41 +++++++++++-- tests/ui/NewChatPageTest.tsx | 103 ++++++++++++++++++++++++++++++-- 2 files changed, 133 insertions(+), 11 deletions(-) diff --git a/src/pages/NewChatPage/index.tsx b/src/pages/NewChatPage/index.tsx index 708747518b83..caec4c586df3 100755 --- a/src/pages/NewChatPage/index.tsx +++ b/src/pages/NewChatPage/index.tsx @@ -60,6 +60,22 @@ import useGroupChatDraftParticipantSync from './useGroupChatDraftParticipantSync const excludedGroupEmails = new Set(CONST.EXPENSIFY_EMAILS.filter((value) => value !== CONST.EMAIL.CONCIERGE)); const PAGINATION_SIZE = CONST.MAX_SELECTION_LIST_PAGE_LENGTH; +/** + * Whether a pressed option refers to the same person as an already-selected one. Login is the primary identity, + * but login-less options (e.g. DMs whose participant details haven't loaded yet) would all compare equal on login + * alone (undefined === undefined), so fall back to accountID, then reportID. Pairs with no shared identity are + * treated as distinct so they follow the add path. + */ +function isSameSelectedOption(selectedOption: SelectedOption, option: ListItem & Partial): boolean { + if (selectedOption.login || option.login) { + return selectedOption.login === option.login; + } + if (selectedOption.accountID || option.accountID) { + return selectedOption.accountID === option.accountID; + } + return !!selectedOption.reportID && selectedOption.reportID === option.reportID; +} + function useOptions(reportAttributesDerived: ReportAttributesDerivedValue['reports'] | undefined) { const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState(''); const [selectedOptions, setSelectedOptions] = useState([]); @@ -323,15 +339,22 @@ function NewChatPage({ref}: NewChatPageProps) { * Removes a selected option from list if already selected. If not already selected add this option to the list. */ const toggleOption = (option: ListItem & Partial) => { - // Read membership from the latest selection (not the item or a render snapshot) so rapid toggles stay - // correct while the deferred list update below is still catching up to a previous press. + // Compose off the latest selection (not a render snapshot) so back-to-back toggles don't drop each + // other's pending updates while the deferred list update below is still catching up. const currentSelectedOptions = latestSelectedOptionsRef.current; - const isOptionInList = currentSelectedOptions.some((selectedOption) => selectedOption.login === option.login); + const isOptionInList = currentSelectedOptions.some((selectedOption) => isSameSelectedOption(selectedOption, option)); + // Removal intent comes from the pressed row's rendered state, not from ref membership: while the + // transition is pending the row still shows its stale control, so a re-press on a still-visible + // "Add to group" button must stay an add (idempotent below) rather than silently cancel the pending one. + const shouldRemoveOption = !!option.isSelected && isOptionInList; let newSelectedOptions: SelectedOption[]; - if (isOptionInList) { - newSelectedOptions = reject(currentSelectedOptions, (selectedOption) => selectedOption.login === option.login); + if (shouldRemoveOption) { + newSelectedOptions = reject(currentSelectedOptions, (selectedOption) => isSameSelectedOption(selectedOption, option)); + } else if (isOptionInList) { + // Already added by a previous press of the same still-visible Add button; nothing to change. + newSelectedOptions = currentSelectedOptions; } else { newSelectedOptions = [...currentSelectedOptions, {...option, isSelected: true, reportID: option.reportID, keyForList: `${option.keyForList ?? option.reportID}`}]; } @@ -469,7 +492,13 @@ function NewChatPage({ref}: NewChatPageProps) { // Read from the latest-selection ref (not the render snapshot): the Next button stays tappable while the // deferred selection transition is still pending, so tapping "Add to group" then quickly Next must not rebuild // the draft from a stale selectedOptions that drops the just-added participant. - const selectedParticipants: SelectedParticipant[] = latestSelectedOptionsRef.current.map((option) => ({ + const latestSelectedOptions = latestSelectedOptionsRef.current; + // Conversely, the stale Next button can be tapped right after the last member was removed; don't build + // a group draft containing only the current user. + if (latestSelectedOptions.length === 0) { + return; + } + const selectedParticipants: SelectedParticipant[] = latestSelectedOptions.map((option) => ({ login: option?.login, accountID: option.accountID ?? CONST.DEFAULT_NUMBER_ID, })); diff --git a/tests/ui/NewChatPageTest.tsx b/tests/ui/NewChatPageTest.tsx index d11d15094aae..007c8f93f7b5 100644 --- a/tests/ui/NewChatPageTest.tsx +++ b/tests/ui/NewChatPageTest.tsx @@ -1,10 +1,13 @@ import {act, fireEvent, render, screen, waitFor, within} from '@testing-library/react-native'; +import {CurrentUserPersonalDetailsProvider} from '@components/CurrentUserPersonalDetailsProvider'; import HTMLEngineProvider from '@components/HTMLEngineProvider'; import {LocaleContextProvider} from '@components/LocaleContextProvider'; import OnyxListItemProvider from '@components/OnyxListItemProvider'; import ScreenWrapper from '@components/ScreenWrapper'; +import Navigation from '@libs/Navigation/Navigation'; + import NewChatPage from '@pages/NewChatPage'; import CONST from '@src/CONST'; @@ -24,6 +27,10 @@ import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct' jest.mock('@react-navigation/native'); jest.mock('@src/libs/Navigation/navigationRef'); +// Use the web implementation (a passthrough): Jest resolves the .native variant, whose in-flight guard would +// swallow the rapid re-presses that the pending-transition tests below simulate. On web, where the deferred +// selection update matters most, there is no such guard. +jest.mock('@hooks/useSingleExecution', (): unknown => jest.requireActual('@hooks/useSingleExecution/index.ts')); jest.mock('react-native-permissions', () => ({ __esModule: true, RESULTS: { @@ -49,11 +56,13 @@ const triggerTransitionEnd = () => (NativeNavigation as NativeNavigationMock).tr const wrapper = ({children}: {children: React.ReactNode}) => ( - - - {children} - - + + + + {children} + + + ); @@ -244,6 +253,90 @@ describe('NewChatPage', () => { expect(screen.getByText(translateLocal('common.next'))).toBeVisible(); }); + it('should keep the user selected when the still-visible "Add to group" button is pressed twice before the deferred update commits', async () => { + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, fakePersonalDetails); + }); + render(, {wrapper}); + await waitForBatchedUpdatesWithAct(); + act(() => { + triggerTransitionEnd(); + }); + + const addButton = await waitFor(() => { + const button = screen.getAllByText(translateLocal('newChatPage.addToGroup')).at(0); + expect(button).toBeTruthy(); + return button; + }); + if (!addButton) { + return; + } + + // Both presses run in one act() batch: nothing commits between them, so the second press lands on the + // same still-visible "Add to group" button — the retry/double-tap that arrives before the deferred + // selection update has caught up. + // eslint-disable-next-line testing-library/no-unnecessary-act -- the shared act batch is the point: it keeps the deferred update from committing between the two presses + act(() => { + fireEvent.press(addButton); + fireEvent.press(addButton); + }); + await waitForBatchedUpdatesWithAct(); + + // The retry press must stay an add (idempotent), not silently cancel the pending one. + expect(screen.getByText(translateLocal('common.next'))).toBeVisible(); + expect(screen.getAllByTestId(new RegExp(`^${CONST.SELECTION_BUTTON_TEST_ID}`))).toHaveLength(1); + }); + + it('should not open the group confirmation when the last member was removed just before pressing Next', async () => { + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, fakePersonalDetails); + await Onyx.merge(ONYXKEYS.SESSION, {accountID: 1, email: 'email1@test.com'}); + }); + render(, {wrapper}); + await waitForBatchedUpdatesWithAct(); + act(() => { + triggerTransitionEnd(); + }); + + // Enter group-selection mode by selecting one user. + const addButton = await waitFor(() => { + const button = screen.getAllByText(translateLocal('newChatPage.addToGroup')).at(0); + expect(button).toBeTruthy(); + return button; + }); + if (!addButton) { + return; + } + fireEvent.press(addButton); + await waitForBatchedUpdatesWithAct(); + const nextButton = screen.getByText(translateLocal('common.next')); + expect(nextButton).toBeVisible(); + + const navigateSpy = jest.spyOn(Navigation, 'navigate').mockImplementation(() => {}); + + const checkedCheckbox = screen.getAllByTestId(new RegExp(`^${CONST.SELECTION_BUTTON_TEST_ID}`)).at(0); + expect(checkedCheckbox).toBeTruthy(); + if (!checkedCheckbox) { + return; + } + + // Both presses run in one act() batch: nothing commits between them, so the Next button is still + // rendered when it is pressed even though the last member was just removed. Confirming with an empty + // latest selection must not navigate to the group confirmation page. + // eslint-disable-next-line testing-library/no-unnecessary-act -- the shared act batch is the point: it keeps the deferred update from committing between the two presses + act(() => { + fireEvent.press(checkedCheckbox); + fireEvent.press(nextButton); + }); + expect(navigateSpy).not.toHaveBeenCalled(); + + // Once the deferred update commits the removal, group-selection mode is exited. + await waitForBatchedUpdatesWithAct(); + expect(screen.queryByText(translateLocal('common.next'))).toBeNull(); + expect(navigateSpy).not.toHaveBeenCalled(); + navigateSpy.mockRestore(); + }); + describe('should not display "Add to group" button on expensify emails', () => { const excludedGroupEmails = CONST.EXPENSIFY_EMAILS.filter((value) => value !== CONST.EMAIL.CONCIERGE && value !== CONST.EMAIL.NOTIFICATIONS).map((email) => [email]);