Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f5bf126
WIP-WorkspacePageWithSections migrated to tsx
jayeshmangwani Jan 11, 2024
1b330c9
Merge branch 'main' of https://github.com/Expensify/App into ts-migra…
jayeshmangwani Jan 11, 2024
8071b7a
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 12, 2024
537f734
Merge branch 'main' of https://github.com/Expensify/App into ts-migra…
jayeshmangwani Jan 12, 2024
99ba295
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 13, 2024
7bedf19
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 15, 2024
be72cb5
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 16, 2024
a1409c1
feat: extract WithPolicyAndFullscreenLoadingProps and intersectioned …
jayeshmangwani Jan 17, 2024
c975738
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 17, 2024
016f612
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 18, 2024
5efe176
fix: route prop type updated
jayeshmangwani Jan 18, 2024
3b74a2a
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 22, 2024
08b57c0
fix: used shared type from types/onyx
jayeshmangwani Jan 22, 2024
93f450a
Merge branch 'main' into ts-migration/WorkspacePageWithSections
jayeshmangwani Jan 22, 2024
93656c3
feat: added default values and extracted USER_DEFAULT to CONST.ts
jayeshmangwani Jan 22, 2024
bd9298d
feat: memoized shouldShow
jayeshmangwani Jan 22, 2024
e878e0f
fix: removed optional children condition
jayeshmangwani Jan 22, 2024
b5602b2
fix: replaced PolicyRoute usage with RouteProp policyID
jayeshmangwani Jan 22, 2024
866ed82
resolved conflicts
jayeshmangwani Jan 23, 2024
a292d26
revert USER_DEFAULT from CONST
jayeshmangwani Jan 23, 2024
88f0d0d
fix: revert user value set to null
jayeshmangwani Jan 23, 2024
ee29a01
removed unnecessary import
jayeshmangwani Jan 23, 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
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3131,6 +3131,7 @@ const CONST = {
},

MINI_CONTEXT_MENU_MAX_ITEMS: 4,

REPORT_FIELD_TITLE_FIELD_ID: 'text_title',
} as const;

Expand Down
10 changes: 5 additions & 5 deletions src/components/ScrollViewWithContext.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type {ForwardedRef} from 'react';
import React, {useMemo, useRef, useState} from 'react';
import type {NativeScrollEvent, NativeSyntheticEvent} from 'react-native';
import type {NativeScrollEvent, NativeSyntheticEvent, ScrollViewProps} from 'react-native';
import {ScrollView} from 'react-native';

const MIN_SMOOTH_SCROLL_EVENT_THROTTLE = 16;
Expand All @@ -16,10 +16,10 @@ const ScrollContext = React.createContext<ScrollContextValue>({
});

type ScrollViewWithContextProps = {
onScroll: (event: NativeSyntheticEvent<NativeScrollEvent>) => void;
onScroll?: (event: NativeSyntheticEvent<NativeScrollEvent>) => void;
children?: React.ReactNode;
scrollEventThrottle: number;
} & Partial<ScrollView>;
scrollEventThrottle?: number;
} & Partial<ScrollViewProps>;

/*
* <ScrollViewWithContext /> is a wrapper around <ScrollView /> that provides a ref to the <ScrollView />.
Expand Down Expand Up @@ -54,7 +54,7 @@ function ScrollViewWithContextWithRef({onScroll, scrollEventThrottle, children,
{...restProps}
ref={scrollViewRef}
onScroll={setContextScrollPosition}
scrollEventThrottle={scrollEventThrottle || MIN_SMOOTH_SCROLL_EVENT_THROTTLE}
scrollEventThrottle={scrollEventThrottle ?? MIN_SMOOTH_SCROLL_EVENT_THROTTLE}
>
<ScrollContext.Provider value={contextValue}>{children}</ScrollContext.Provider>
</ScrollView>
Expand Down
4 changes: 1 addition & 3 deletions src/components/TestToolMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import * as Session from '@userActions/Session';
import * as User from '@userActions/User';
import CONFIG from '@src/CONFIG';
import ONYXKEYS from '@src/ONYXKEYS';
import type NetworkOnyx from '@src/types/onyx/Network';
import type UserOnyx from '@src/types/onyx/User';
import type {Network as NetworkOnyx, User as UserOnyx} from '@src/types/onyx';
import Button from './Button';
import {withNetwork} from './OnyxProvider';
import Switch from './Switch';
Expand All @@ -26,7 +25,6 @@ type TestToolMenuProps = TestToolMenuOnyxProps & {
/** Network object in Onyx */
network: OnyxEntry<NetworkOnyx>;
};

const USER_DEFAULT: UserOnyx = {shouldUseStagingServer: undefined, isSubscribedToNewsletter: false, validated: false, isFromPublicDomain: false, isUsingExpensifyCard: false};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we need this change. Onyx props can be undefined so don't have to define a default value for them. Also, we should not put this value in CONST file.

Let's revert this change and not define any default value for user prop.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure, we will revert this change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have reverted the changes that we have added in this PR for USER_DEFAULT. For src/components/TestToolMenu.tsx file, we are not touching the USER_DEFAULT added by some other PR, please let me know if we need to update the USER_DEFAULT for this page to null


function TestToolMenu({user = USER_DEFAULT, network}: TestToolMenuProps) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,86 +1,70 @@
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useEffect, useRef} from 'react';
import type {RouteProp} from '@react-navigation/native';
import React, {useEffect, useMemo, useRef} from 'react';
import type {ReactNode} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import type {OnyxEntry} from 'react-native-onyx/lib/types';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
import ScrollViewWithContext from '@components/ScrollViewWithContext';
import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import BankAccount from '@libs/models/BankAccount';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ReimbursementAccountProps from '@pages/ReimbursementAccount/reimbursementAccountPropTypes';
import userPropTypes from '@pages/settings/userPropTypes';
import * as BankAccounts from '@userActions/BankAccounts';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Route} from '@src/ROUTES';
import type {Policy, ReimbursementAccount, User} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import type {WithPolicyAndFullscreenLoadingProps} from './withPolicyAndFullscreenLoading';
import withPolicyAndFullscreenLoading from './withPolicyAndFullscreenLoading';

const propTypes = {
shouldSkipVBBACall: PropTypes.bool,

/** The text to display in the header */
headerText: PropTypes.string.isRequired,

/** The route object passed to this page from the navigator */
route: PropTypes.shape({
/** Each parameter passed via the URL */
params: PropTypes.shape({
/** The policyID that is being configured */
policyID: PropTypes.string.isRequired,
}).isRequired,
}).isRequired,

type WorkspacePageWithSectionsOnyxProps = {
/** From Onyx */
/** Bank account attached to free plan */
reimbursementAccount: ReimbursementAccountProps.reimbursementAccountPropTypes,
reimbursementAccount: OnyxEntry<ReimbursementAccount>;

/** User Data from Onyx */
user: userPropTypes,
user: OnyxEntry<User>;
};

/** Main content of the page */
children: PropTypes.func,
type WorkspacePageWithSectionsProps = WithPolicyAndFullscreenLoadingProps &
WorkspacePageWithSectionsOnyxProps & {
shouldSkipVBBACall?: boolean;

/** Content to be added as fixed footer */
footer: PropTypes.element,
/** The text to display in the header */
headerText: string;

/** The guides call task ID to associate with the workspace page being shown */
guidesCallTaskID: PropTypes.string,
/** The route object passed to this page from the navigator */
route: RouteProp<{params: {policyID: string}}>;

/** The route where we navigate when the user press the back button */
backButtonRoute: PropTypes.string,
/** Main content of the page */
children: (hasVBA?: boolean, policyID?: string, isUsingECard?: boolean) => ReactNode;

/** Policy values needed in the component */
policy: PropTypes.shape({
name: PropTypes.string,
}).isRequired,
/** Content to be added as fixed footer */
footer?: ReactNode;

/** Option to use the default scroll view */
shouldUseScrollView: PropTypes.bool,
/** The guides call task ID to associate with the workspace page being shown */
guidesCallTaskID: string;

/** Option to show the loading page while the API is calling */
shouldShowLoading: PropTypes.bool,
};
/** The route where we navigate when the user press the back button */
backButtonRoute?: Route;

const defaultProps = {
children: () => {},
user: {},
reimbursementAccount: ReimbursementAccountProps.reimbursementAccountDefaultProps,
footer: null,
guidesCallTaskID: '',
shouldUseScrollView: false,
shouldSkipVBBACall: false,
backButtonRoute: '',
shouldShowLoading: true,
};
/** Option to use the default scroll view */
shouldUseScrollView?: boolean;

/** Option to show the loading page while the API is calling */
shouldShowLoading?: boolean;

/** Policy values needed in the component */
policy: OnyxEntry<Policy>;
};

function fetchData(skipVBBACal) {
function fetchData(skipVBBACal?: boolean) {
if (skipVBBACal) {
return;
}
Expand All @@ -89,28 +73,28 @@ function fetchData(skipVBBACal) {
}

function WorkspacePageWithSections({
backButtonRoute,
children,
footer,
guidesCallTaskID,
backButtonRoute = '',
children = () => null,
footer = null,
guidesCallTaskID = '',
headerText,
policy,
reimbursementAccount,
reimbursementAccount = {},
route,
shouldUseScrollView,
shouldSkipVBBACall,
shouldUseScrollView = false,
shouldSkipVBBACall = false,
user,
shouldShowLoading,
}) {
shouldShowLoading = true,
}: WorkspacePageWithSectionsProps) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add defaults to props.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated!

we have a same user prop on WorkspacePageWithSections page and TestToolMenu,On TestToolMenu component we're using default value defined on the page, so I have extracted USER_DEFAULT to CONST file and now used same default value at both places.

const USER_DEFAULT: UserOnyx = {shouldUseStagingServer: undefined, isSubscribedToNewsletter: false, validated: false, isFromPublicDomain: false, isUsingExpensifyCard: false};
function TestToolMenu({user = USER_DEFAULT, network}: TestToolMenuProps) {

const styles = useThemeStyles();
useNetwork({onReconnect: () => fetchData(shouldSkipVBBACall)});

const isLoading = lodashGet(reimbursementAccount, 'isLoading', true);
const achState = lodashGet(reimbursementAccount, 'achData.state', '');
const isLoading = reimbursementAccount?.isLoading ?? true;
const achState = reimbursementAccount?.achData?.state ?? '';
const isUsingECard = user?.isUsingExpensifyCard ?? false;
const policyID = route.params.policyID;
const policyName = policy?.name;
const hasVBA = achState === BankAccount.STATE.OPEN;
const isUsingECard = lodashGet(user, 'isUsingExpensifyCard', false);
const policyID = lodashGet(route, 'params.policyID');
const policyName = lodashGet(policy, 'name');
const content = children(hasVBA, policyID, isUsingECard);
const firstRender = useRef(true);

Expand All @@ -123,6 +107,14 @@ function WorkspacePageWithSections({
fetchData(shouldSkipVBBACall);
}, [shouldSkipVBBACall]);

const shouldShow = useMemo(() => {
if (isEmptyObject(policy)) {
return true;
}

return !PolicyUtils.isPolicyAdmin(policy) || PolicyUtils.isPendingDeletePolicy(policy);
}, [policy]);
Comment on lines +110 to +116

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NAB: IMO the code here don't justify a useMemo as it is very lean. Just a couple of !== checks and Object.keys(obj ?? {}).length === 0. I think useMemo makes the code less readable and maybe even slower here :P


return (
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
Expand All @@ -132,15 +124,15 @@ function WorkspacePageWithSections({
>
<FullPageNotFoundView
onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_WORKSPACES)}
shouldShow={_.isEmpty(policy) || !PolicyUtils.isPolicyAdmin(policy) || PolicyUtils.isPendingDeletePolicy(policy)}
subtitleKey={_.isEmpty(policy) ? undefined : 'workspace.common.notAuthorized'}
shouldShow={shouldShow}
subtitleKey={isEmptyObject(policy) ? undefined : 'workspace.common.notAuthorized'}
>
<HeaderWithBackButton
title={headerText}
subtitle={policyName}
shouldShowGetAssistanceButton
guidesCallTaskID={guidesCallTaskID}
onBackButtonPress={() => Navigation.goBack(backButtonRoute || ROUTES.WORKSPACE_INITIAL.getRoute(policyID))}
onBackButtonPress={() => Navigation.goBack(backButtonRoute ?? ROUTES.WORKSPACE_INITIAL.getRoute(policyID))}
/>
{(isLoading || firstRender.current) && shouldShowLoading ? (
<FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />
Expand All @@ -164,18 +156,15 @@ function WorkspacePageWithSections({
);
}

WorkspacePageWithSections.propTypes = propTypes;
WorkspacePageWithSections.defaultProps = defaultProps;
WorkspacePageWithSections.displayName = 'WorkspacePageWithSections';

export default compose(
withOnyx({
export default withPolicyAndFullscreenLoading(
withOnyx<WorkspacePageWithSectionsProps, WorkspacePageWithSectionsOnyxProps>({
user: {
key: ONYXKEYS.USER,
},
reimbursementAccount: {
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
},
}),
withPolicyAndFullscreenLoading,
)(WorkspacePageWithSections);
})(WorkspacePageWithSections),
);
17 changes: 9 additions & 8 deletions src/pages/workspace/invoices/WorkspaceInvoicesPage.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import type {StackScreenProps} from '@react-navigation/stack';
import type {RouteProp} from '@react-navigation/native';
import React from 'react';
import useLocalize from '@hooks/useLocalize';
import type {SettingsNavigatorParamList} from '@libs/Navigation/types';
import WorkspacePageWithSections from '@pages/workspace/WorkspacePageWithSections';
import CONST from '@src/CONST';
import type SCREENS from '@src/SCREENS';
import WorkspaceInvoicesNoVBAView from './WorkspaceInvoicesNoVBAView';
import WorkspaceInvoicesVBAView from './WorkspaceInvoicesVBAView';

type WorkspaceInvoicesPageProps = StackScreenProps<SettingsNavigatorParamList, typeof SCREENS.WORKSPACE.INVOICES>;
/** Defined route object that contains the policyID param, WorkspacePageWithSections is a common component for Workspaces and expect the route prop that includes the policyID */
type WorkspaceInvoicesPageProps = {
route: RouteProp<{params: {policyID: string}}>;
};

function WorkspaceInvoicesPage({route}: WorkspaceInvoicesPageProps) {
const {translate} = useLocalize();
Expand All @@ -17,13 +18,13 @@ function WorkspaceInvoicesPage({route}: WorkspaceInvoicesPageProps) {
<WorkspacePageWithSections
shouldUseScrollView
headerText={translate('workspace.common.invoices')}
route={route}
guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_INVOICES}
route={route}
>
{(hasVBA: boolean, policyID: string) => (
{(hasVBA?: boolean, policyID?: string) => (
<>
{!hasVBA && <WorkspaceInvoicesNoVBAView policyID={policyID} />}
{hasVBA && <WorkspaceInvoicesVBAView policyID={policyID} />}
{!hasVBA && policyID && <WorkspaceInvoicesNoVBAView policyID={policyID} />}
{hasVBA && policyID && <WorkspaceInvoicesVBAView policyID={policyID} />}
</>
)}
</WorkspacePageWithSections>
Expand Down