Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace global variables #47076

Merged
merged 14 commits into from
Sep 11, 2024
Merged

Replace global variables #47076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5a175d9
fix undefined role route
BrtqKr Aug 8, 2024
0d3cc6e
move search state to onyx wip
BrtqKr Aug 8, 2024
75654cf
apply onyx search persist to the members flow
BrtqKr Aug 8, 2024
f06810d
cleanup
BrtqKr Aug 8, 2024
d6df1ba
remove persist from workspace invite page
BrtqKr Aug 8, 2024
30ff249
cleanup
BrtqKr Aug 8, 2024
a311867
cleanup
BrtqKr Aug 8, 2024
ba8b960
review fixes
BrtqKr Aug 8, 2024
07a3dce
Merge remote-tracking branch 'origin/main' into brtqkr/repalce-global…
BrtqKr Aug 26, 2024
aff91e1
Merge remote-tracking branch 'origin/main' into brtqkr/repalce-global…
BrtqKr Aug 28, 2024
86db79c
cleanup
BrtqKr Aug 28, 2024
902e65c
Merge remote-tracking branch 'origin/main' into brtqkr/repalce-global…
BrtqKr Sep 5, 2024
043056c
Merge remote-tracking branch 'origin/main' into brtqkr/repalce-global…
BrtqKr Sep 10, 2024
dd11582
replace search input manager for the updated screens
BrtqKr Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/ONYXKEYS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,8 @@ const ONYXKEYS = {
/** Stores the information about currently edited advanced approval workflow */
APPROVAL_WORKFLOW: 'approvalWorkflow',

/** Stores the user search value for persistance across the screens */
ROOM_MEMBERS_USER_SEARCH_PHRASE: 'roomMembersUserSearchPhrase',
/** Stores information about recently uploaded spreadsheet file */
IMPORTED_SPREADSHEET: 'importedSpreadsheet',

Expand Down Expand Up @@ -950,6 +952,7 @@ type OnyxValuesMapping = {
[ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END]: number;
[ONYXKEYS.NVP_WORKSPACE_TOOLTIP]: OnyxTypes.WorkspaceTooltip;
[ONYXKEYS.NVP_PRIVATE_CANCELLATION_DETAILS]: OnyxTypes.CancellationDetails[];
[ONYXKEYS.ROOM_MEMBERS_USER_SEARCH_PHRASE]: string;
[ONYXKEYS.APPROVAL_WORKFLOW]: OnyxTypes.ApprovalWorkflowOnyx;
[ONYXKEYS.IMPORTED_SPREADSHEET]: OnyxTypes.ImportedSpreadsheet;
[ONYXKEYS.LAST_ROUTE]: string;
Expand Down
5 changes: 4 additions & 1 deletion src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,10 @@ const ROUTES = {
},
ROOM_INVITE: {
route: 'r/:reportID/invite/:role?',
getRoute: (reportID: string, role?: string) => `r/${reportID}/invite/${role ?? ''}` as const,
getRoute: (reportID: string, role?: string) => {
const route = role ? (`r/${reportID}/invite/${role}` as const) : (`r/${reportID}/invite` as const);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a shorter version?

Suggested change
const route = role ? (`r/${reportID}/invite/${role}` as const) : (`r/${reportID}/invite` as const);
getRoute: (reportID: string, role?: string) => `r/${reportID}/invite${role ? `/${role}`: ''}` as const,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BrtqKr Could you please help to detail which problem here?

Fixed role undefined case on room members invite

Copy link
Contributor Author

@BrtqKr BrtqKr Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when someone didn't have a role inside of a report we ended up with /undefined inside of the url. It just seems like someone forgot to put a proper fallback in there

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BrtqKr With previous code, we added a fallback value is empty string. Why shouldn't use ?? operator?

Copy link
Contributor Author

@BrtqKr BrtqKr Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you meant exactly, but two explanations:
1.
getRoute: (reportID: string, role?: string) =>r/${reportID}/invite/${role} as const,

In the previous version, skipping the role meant getting undefined, which was getting stringified, which resulted in for example
r/151422415264937/invite/undefined in the url.

?? operator works only as a fallback for null and undefined and in this case we want to include empty string in the same group to remove slash for this scenario

return route;
},
},
MONEY_REQUEST_HOLD_REASON: {
route: ':type/edit/reason/:transactionID?',
Expand Down
3 changes: 0 additions & 3 deletions src/libs/Navigation/AppNavigator/AuthScreens.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import * as SessionUtils from '@libs/SessionUtils';
import ConnectionCompletePage from '@pages/ConnectionCompletePage';
import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
import DesktopSignInRedirectPage from '@pages/signin/DesktopSignInRedirectPage';
import SearchInputManager from '@pages/workspace/SearchInputManager';
import * as App from '@userActions/App';
import * as Download from '@userActions/Download';
import * as Modal from '@userActions/Modal';
Expand Down Expand Up @@ -198,8 +197,6 @@ const modalScreenListeners = {
Modal.setModalVisibility(false);
},
beforeRemove: () => {
// Clear search input (WorkspaceInvitePage) when modal is closed
SearchInputManager.searchInput = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we clear it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because we've removed the usage of persist on that page. Apparently navigation patterns have changed and this is no longer necessary.

Modal.setModalVisibility(false);
Modal.willAlertModalBecomeVisible(false);
},
Expand Down
15 changes: 15 additions & 0 deletions src/libs/actions/RoomMembersUserSearchPhrase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import Onyx from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';

function clearUserSearchPhrase() {
Onyx.merge(ONYXKEYS.ROOM_MEMBERS_USER_SEARCH_PHRASE, '');
}

/**
* Persists user search phrase from the serch input across the screens.
*/
function updateUserSearchPhrase(value: string) {
Onyx.merge(ONYXKEYS.ROOM_MEMBERS_USER_SEARCH_PHRASE, value);
}

export {clearUserSearchPhrase, updateUserSearchPhrase};
37 changes: 18 additions & 19 deletions src/pages/InviteReportParticipantsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import type {SectionListData} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import {useOnyx, withOnyx} from 'react-native-onyx';
import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import {useOptionsList} from '@components/OptionListContextProvider';
Expand All @@ -14,6 +14,7 @@ import type {WithNavigationTransitionEndProps} from '@components/withNavigationT
import useDebouncedState from '@hooks/useDebouncedState';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import * as UserSearchPhraseActions from '@libs/actions/RoomMembersUserSearchPhrase';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import * as LoginUtils from '@libs/LoginUtils';
import Navigation from '@libs/Navigation/Navigation';
Expand All @@ -28,7 +29,6 @@ import ROUTES from '@src/ROUTES';
import type {InvitedEmailsToAccountIDs, PersonalDetailsList} from '@src/types/onyx';
import type {WithReportOrNotFoundProps} from './home/report/withReportOrNotFound';
import withReportOrNotFound from './home/report/withReportOrNotFound';
import SearchInputManager from './workspace/SearchInputManager';

type InviteReportParticipantsPageOnyxProps = {
/** All of the personal details for everyone */
Expand All @@ -46,13 +46,13 @@ function InviteReportParticipantsPage({betas, personalDetails, report, didScreen

const styles = useThemeStyles();
const {translate} = useLocalize();
const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState('');
const [userSearchPhrase] = useOnyx(ONYXKEYS.ROOM_MEMBERS_USER_SEARCH_PHRASE);
const [searchValue, debouncedSearchTerm, setSearchValue] = useDebouncedState(userSearchPhrase ?? '');
const [selectedOptions, setSelectedOptions] = useState<ReportUtils.OptionData[]>([]);

useEffect(() => {
setSearchTerm(SearchInputManager.searchInput);
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, []);
UserSearchPhraseActions.updateUserSearchPhrase(debouncedSearchTerm);
}, [debouncedSearchTerm]);

// Any existing participants and Expensify emails should not be eligible for invitation
const excludedUsers = useMemo(
Expand Down Expand Up @@ -104,8 +104,8 @@ function InviteReportParticipantsPage({betas, personalDetails, report, didScreen
filterSelectedOptions = selectedOptions.filter((option) => {
const accountID = option?.accountID;
const isOptionInPersonalDetails = inviteOptions.personalDetails.some((personalDetail) => accountID && personalDetail?.accountID === accountID);
const searchValue = OptionsListUtils.getSearchValueForPhoneOrEmail(debouncedSearchTerm);
const isPartOfSearchTerm = !!option.text?.toLowerCase().includes(searchValue) || !!option.login?.toLowerCase().includes(searchValue);
const processedSearchValue = OptionsListUtils.getSearchValueForPhoneOrEmail(debouncedSearchTerm);
const isPartOfSearchTerm = !!option.text?.toLowerCase().includes(processedSearchValue) || !!option.login?.toLowerCase().includes(processedSearchValue);
return isPartOfSearchTerm || isOptionInPersonalDetails;
});
}
Expand Down Expand Up @@ -183,22 +183,22 @@ function InviteReportParticipantsPage({betas, personalDetails, report, didScreen
}, [selectedOptions, backRoute, reportID, validate]);

const headerMessage = useMemo(() => {
const searchValue = debouncedSearchTerm.trim().toLowerCase();
const processedLogin = debouncedSearchTerm.trim().toLowerCase();
const expensifyEmails = CONST.EXPENSIFY_EMAILS as string[];
if (!inviteOptions.userToInvite && expensifyEmails.includes(searchValue)) {
if (!inviteOptions.userToInvite && expensifyEmails.includes(processedLogin)) {
return translate('messages.errorMessageInvalidEmail');
}
if (
!inviteOptions.userToInvite &&
excludedUsers.includes(
PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(searchValue)).possible
? PhoneNumber.addSMSDomainIfPhoneNumber(LoginUtils.appendCountryCode(searchValue))
: searchValue,
PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(processedLogin)).possible
? PhoneNumber.addSMSDomainIfPhoneNumber(LoginUtils.appendCountryCode(processedLogin))
: processedLogin,
)
) {
return translate('messages.userIsAlreadyMember', {login: searchValue, name: reportName ?? ''});
return translate('messages.userIsAlreadyMember', {login: processedLogin, name: reportName ?? ''});
}
return OptionsListUtils.getHeaderMessage(inviteOptions.recentReports.length + inviteOptions.personalDetails.length !== 0, !!inviteOptions.userToInvite, searchValue);
return OptionsListUtils.getHeaderMessage(inviteOptions.recentReports.length + inviteOptions.personalDetails.length !== 0, !!inviteOptions.userToInvite, processedLogin);
}, [debouncedSearchTerm, inviteOptions.userToInvite, inviteOptions.recentReports.length, inviteOptions.personalDetails.length, excludedUsers, translate, reportName]);

const footerContent = useMemo(
Expand All @@ -207,7 +207,7 @@ function InviteReportParticipantsPage({betas, personalDetails, report, didScreen
isDisabled={!selectedOptions.length}
buttonText={translate('common.invite')}
onSubmit={() => {
SearchInputManager.searchInput = '';
UserSearchPhraseActions.clearUserSearchPhrase();
inviteUsers();
}}
containerStyles={[styles.flexReset, styles.flexGrow0, styles.flexShrink0, styles.flexBasisAuto]}
Expand Down Expand Up @@ -236,10 +236,9 @@ function InviteReportParticipantsPage({betas, personalDetails, report, didScreen
sections={sections}
ListItem={InviteMemberListItem}
textInputLabel={translate('selectionList.nameEmailOrPhoneNumber')}
textInputValue={searchTerm}
textInputValue={searchValue}
onChangeText={(value) => {
SearchInputManager.searchInput = value;
setSearchTerm(value);
setSearchValue(value);
}}
headerMessage={headerMessage}
onSelectRow={toggleOption}
Expand Down
22 changes: 13 additions & 9 deletions src/pages/ReportParticipantsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem, SelectionListHandle} from '@components/SelectionList/types';
import SelectionListWithModal from '@components/SelectionListWithModal';
import Text from '@components/Text';
import useDebouncedState from '@hooks/useDebouncedState';
import useLocalize from '@hooks/useLocalize';
import useMobileSelectionMode from '@hooks/useMobileSelectionMode';
import useNetwork from '@hooks/useNetwork';
Expand All @@ -25,6 +26,7 @@ import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import {turnOffMobileSelectionMode} from '@libs/actions/MobileSelectionMode';
import * as Report from '@libs/actions/Report';
import * as UserSearchPhraseActions from '@libs/actions/RoomMembersUserSearchPhrase';
import Navigation from '@libs/Navigation/Navigation';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
Expand All @@ -34,7 +36,6 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {WithReportOrNotFoundProps} from './home/report/withReportOrNotFound';
import withReportOrNotFound from './home/report/withReportOrNotFound';
import SearchInputManager from './workspace/SearchInputManager';

type MemberOption = Omit<ListItem, 'accountID'> & {accountID: number};

Expand All @@ -47,6 +48,7 @@ function ReportParticipantsPage({report}: WithReportOrNotFoundProps) {
const {shouldUseNarrowLayout, isSmallScreenWidth} = useResponsiveLayout();
const selectionListRef = useRef<SelectionListHandle>(null);
const textInputRef = useRef<TextInput>(null);
const [userSearchPhrase] = useOnyx(ONYXKEYS.ROOM_MEMBERS_USER_SEARCH_PHRASE);
const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`);
const {selectionMode} = useMobileSelectionMode();
const [session] = useOnyx(ONYXKEYS.SESSION);
Expand All @@ -57,15 +59,19 @@ function ReportParticipantsPage({report}: WithReportOrNotFoundProps) {
const isFocused = useIsFocused();
const {isOffline} = useNetwork();
const canSelectMultiple = isGroupChat && isCurrentUserAdmin && (isSmallScreenWidth ? selectionMode?.isEnabled : true);
const [searchValue, setSearchValue] = useState('');
const [searchValue, debouncedSearchValue, setSearchValue] = useDebouncedState('');

useEffect(
() => () => {
SearchInputManager.searchInput = '';
UserSearchPhraseActions.clearUserSearchPhrase();
},
[],
);

useEffect(() => {
UserSearchPhraseActions.updateUserSearchPhrase(debouncedSearchValue);
}, [debouncedSearchValue]);

useEffect(() => {
if (isFocused) {
return;
Expand Down Expand Up @@ -96,12 +102,12 @@ function ReportParticipantsPage({report}: WithReportOrNotFoundProps) {
return;
}
if (shouldShowTextInput) {
setSearchValue(SearchInputManager.searchInput);
setSearchValue(userSearchPhrase ?? '');
} else {
SearchInputManager.searchInput = '';
UserSearchPhraseActions.clearUserSearchPhrase();
setSearchValue('');
}
}, [isFocused, shouldShowTextInput]);
}, [isFocused, setSearchValue, shouldShowTextInput, userSearchPhrase]);

const getParticipants = () => {
let result: MemberOption[] = [];
Expand Down Expand Up @@ -186,7 +192,6 @@ function ReportParticipantsPage({report}: WithReportOrNotFoundProps) {
* Open the modal to invite a user
*/
const inviteUser = useCallback(() => {
setSearchValue('');
Navigation.navigate(ROUTES.REPORT_PARTICIPANTS_INVITE.getRoute(report.reportID));
}, [report]);

Expand All @@ -199,7 +204,7 @@ function ReportParticipantsPage({report}: WithReportOrNotFoundProps) {
const accountIDsToRemove = selectedMembers.filter((id) => id !== currentUserAccountID);
Report.removeFromGroupChat(report.reportID, accountIDsToRemove);
setSearchValue('');
SearchInputManager.searchInput = '';
UserSearchPhraseActions.clearUserSearchPhrase();
setSelectedMembers([]);
setRemoveMembersConfirmModalVisible(false);
};
Expand Down Expand Up @@ -404,7 +409,6 @@ function ReportParticipantsPage({report}: WithReportOrNotFoundProps) {
textInputLabel={translate('selectionList.findMember')}
textInputValue={searchValue}
onChangeText={(value) => {
SearchInputManager.searchInput = value;
setSearchValue(value);
}}
headerMessage={headerMessage}
Expand Down
15 changes: 6 additions & 9 deletions src/pages/RoomInvitePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import useDebouncedState from '@hooks/useDebouncedState';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ReportActions from '@libs/actions/Report';
import * as UserSearchPhraseActions from '@libs/actions/RoomMembersUserSearchPhrase';
import {READ_COMMANDS} from '@libs/API/types';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import HttpUtils from '@libs/HttpUtils';
Expand All @@ -38,7 +39,6 @@ import type {Policy} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import type {WithReportOrNotFoundProps} from './home/report/withReportOrNotFound';
import withReportOrNotFound from './home/report/withReportOrNotFound';
import SearchInputManager from './workspace/SearchInputManager';

type RoomInvitePageProps = WithReportOrNotFoundProps & WithNavigationTransitionEndProps & StackScreenProps<RoomMembersNavigatorParamList, typeof SCREENS.ROOM_MEMBERS.INVITE>;

Expand All @@ -53,15 +53,12 @@ function RoomInvitePage({
}: RoomInvitePageProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState('');
const [userSearchPhrase] = useOnyx(ONYXKEYS.ROOM_MEMBERS_USER_SEARCH_PHRASE);
const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState(userSearchPhrase ?? '');
const [selectedOptions, setSelectedOptions] = useState<ReportUtils.OptionData[]>([]);
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});
const {options, areOptionsInitialized} = useOptionsList();

useEffect(() => {
setSearchTerm(SearchInputManager.searchInput);
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, []);
const {options, areOptionsInitialized} = useOptionsList();

// Any existing participants and Expensify emails should not be eligible for invitation
const excludedUsers = useMemo(() => {
Expand Down Expand Up @@ -207,7 +204,7 @@ function RoomInvitePage({
if (reportID) {
Report.inviteToRoom(reportID, invitedEmailsToAccountIDs);
}
SearchInputManager.searchInput = '';
UserSearchPhraseActions.clearUserSearchPhrase();
Navigation.navigate(backRoute);
}, [selectedOptions, backRoute, reportID, validate]);

Expand Down Expand Up @@ -239,6 +236,7 @@ function RoomInvitePage({
}, [debouncedSearchTerm, inviteOptions.userToInvite, inviteOptions.personalDetails, excludedUsers, translate, reportName]);

useEffect(() => {
UserSearchPhraseActions.updateUserSearchPhrase(debouncedSearchTerm);
ReportActions.searchInServer(debouncedSearchTerm);
}, [debouncedSearchTerm]);

Expand All @@ -265,7 +263,6 @@ function RoomInvitePage({
textInputLabel={translate('selectionList.nameEmailOrPhoneNumber')}
textInputValue={searchTerm}
onChangeText={(value) => {
SearchInputManager.searchInput = value;
setSearchTerm(value);
}}
headerMessage={headerMessage}
Expand Down
Loading
Loading