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.

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

perf: make switch between chat list and workspaces smoother #36420

Merged
merged 68 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
3088672
perf: use context for shared reports access
hurali97 Feb 13, 2024
f33930e
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Feb 15, 2024
6921051
perf: improve renderItem and use FlatList
hurali97 Feb 15, 2024
4d267b4
Merge branch 'main' into hur/perf/issue-35704
jbroma Feb 26, 2024
dc88950
refactor: revert changes to withCurrentReportID
jbroma Feb 26, 2024
9ae8701
refactor: remove useReports context & hooks
jbroma Feb 26, 2024
b613e97
refactor: use reports from onyx in useOrderedReportIDs
jbroma Feb 28, 2024
7f15fbc
refactor: move creating orderedReport objects from getOrderedReportId…
jbroma Feb 28, 2024
7fa44fc
Revert "refactor: revert changes to withCurrentReportID"
jbroma Feb 28, 2024
ae12b00
refactor: remove comment in SidebarLinksData
jbroma Feb 28, 2024
8087e59
Merge branch 'main' into hur/perf/issue-35704
jbroma Feb 28, 2024
c87f579
refactor: rename useOrderedReportIDs to useOrderedReportListItems
jbroma Feb 28, 2024
df1069c
perf: use memo for extraData in LHNOptionList
jbroma Feb 28, 2024
1d294bc
fix: typescript fixes
jbroma Feb 28, 2024
7dfba43
fix: update reportActionsSelector to match the one from SidebarLinksData
jbroma Feb 28, 2024
696c5ee
fix: typescript issues
hurali97 Feb 29, 2024
6739b58
test: fix reassure failing test
hurali97 Feb 29, 2024
92fae0c
revert: move option item data calculation to the renderItem component
hurali97 Feb 29, 2024
596fd63
fix: linting
hurali97 Feb 29, 2024
e735658
test: fix failing test
hurali97 Mar 1, 2024
94fcc48
fix: prettier
hurali97 Mar 1, 2024
31f5ed4
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 5, 2024
e0673e0
refactor: rename to useReportIDs
hurali97 Mar 5, 2024
30ca303
refactor: don't set currentReportID if it's on workspaces screen
hurali97 Mar 5, 2024
121b537
refactor: remove dead code
hurali97 Mar 5, 2024
9e41603
fix: linting
hurali97 Mar 5, 2024
4a32841
refactor: remove irrelevant changes
hurali97 Mar 5, 2024
581d09d
fix: comments
hurali97 Mar 11, 2024
fffe41a
fix: safely check the nested properties
hurali97 Mar 11, 2024
6b41bf7
feat: add comments for extraData prop
hurali97 Mar 11, 2024
5f832f9
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 11, 2024
49e32fd
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 13, 2024
6412711
fix: reassure tests
hurali97 Mar 13, 2024
e3477b2
fix: apply prettier
hurali97 Mar 13, 2024
769f83a
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 14, 2024
30b319f
perf: check for the settings tab existence in screen params and early…
hurali97 Mar 14, 2024
71978c2
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 15, 2024
8dba89b
feat: add selector from SidebarLinksData
hurali97 Mar 15, 2024
c39f358
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 15, 2024
f333232
fix: reassure tests
hurali97 Mar 15, 2024
3557027
fix: resolve merge conflicts
hurali97 Mar 15, 2024
335f1fa
Merge branch 'main' into hur/perf/issue-35704
jbroma Mar 20, 2024
61ef588
refactor: use currentReportID from useReportIDs
hurali97 Mar 21, 2024
84aa5ac
refactor: remove unnecessary ref
hurali97 Mar 21, 2024
2d8f88a
Merge branch 'hur/perf/issue-35704' of https://github.com/callstack-i…
hurali97 Mar 21, 2024
8532c08
update comment is updateCurrentReportID function
hurali97 Mar 21, 2024
ac465f3
update comment in useReportIDs hook
hurali97 Mar 21, 2024
886622e
refactor: make comment less verbose
hurali97 Mar 21, 2024
6d17116
refactor: remove irrelevant comment
hurali97 Mar 21, 2024
91ed2e6
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 21, 2024
4e1fbd2
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 22, 2024
2ef5c7f
refactor: reduce memo usage
hurali97 Mar 22, 2024
3bcc037
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 27, 2024
ab5bce3
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Apr 16, 2024
9e4f464
fix: typecheck
hurali97 Apr 16, 2024
20d798e
refactor: remove unnecessary diff
hurali97 Apr 16, 2024
6f20396
refactor: leverage useOnyx hook instead of withOnyx HOC
hurali97 Apr 16, 2024
33fe888
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Apr 18, 2024
c791be0
fix: max depth exceeded error by not mutating allReports object
hurali97 Apr 18, 2024
4aa6b99
fix: use inline options and relevant initialValue
hurali97 Apr 18, 2024
459e86c
fix: remove not needed prop
hurali97 Apr 18, 2024
61fc9ce
fix: use correct prop in memo check
hurali97 Apr 18, 2024
489c652
fix: use policies instead of policyMembers
hurali97 Apr 18, 2024
10c0c04
Apply workaround for initialValue
fabioh8010 Apr 18, 2024
e075aee
fix: failing SidebarOrderTest
hurali97 Apr 19, 2024
0d4637e
fix: typecheck
hurali97 Apr 19, 2024
42068cc
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Apr 25, 2024
821e4e0
refactor: remove initialValues
hurali97 Apr 25, 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
4 changes: 4 additions & 0 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import {KeyboardStateProvider} from './components/withKeyboardState';
import {WindowDimensionsProvider} from './components/withWindowDimensions';
import Expensify from './Expensify';
import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop';
import {OrderedReportIDsContextProvider} from './hooks/useOrderedReportIDs';
import {ReportsContextProvider} from './hooks/useReports';
import OnyxUpdateManager from './libs/actions/OnyxUpdateManager';
import * as Session from './libs/actions/Session';
import * as Environment from './libs/Environment/Environment';
Expand Down Expand Up @@ -81,6 +83,8 @@ function App({url}: AppProps) {
CustomStatusBarAndBackgroundContextProvider,
ActiveElementRoleProvider,
ActiveWorkspaceContextProvider,
ReportsContextProvider,
OrderedReportIDsContextProvider,
]}
>
<CustomStatusBarAndBackground />
Expand Down
116 changes: 18 additions & 98 deletions src/components/LHNOptionsList/LHNOptionsList.tsx
Original file line number Diff line number Diff line change
@@ -1,40 +1,27 @@
import {FlashList} from '@shopify/flash-list';
import type {ReactElement} from 'react';
import React, {memo, useCallback} from 'react';
import {StyleSheet, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import withCurrentReportID from '@components/withCurrentReportID';
import usePermissions from '@hooks/usePermissions';
import React, {useCallback, memo} from 'react';
import {FlatList, StyleSheet, View} from 'react-native';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import OptionRowLHNData from './OptionRowLHNData';

Check failure on line 5 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

`./OptionRowLHNData` import should occur after import of `@shopify/flash-list`
import type {LHNOptionsListOnyxProps, LHNOptionsListProps, RenderItemProps} from './types';
import type {LHNOptionsListProps, RenderItemProps} from './types';

Check failure on line 6 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

`./types` type import should occur after import of `@shopify/flash-list`
import { OrderedReports } from '@libs/SidebarUtils';

Check failure on line 7 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

All imports in the declaration are only used as types. Use `import type`
import CONST from '@src/CONST';

Check failure on line 8 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'CONST' is defined but never used
import variables from '@styles/variables';

Check failure on line 9 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'variables' is defined but never used
import { FlashList } from '@shopify/flash-list';

Check failure on line 10 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'FlashList' is defined but never used

const keyExtractor = (item: string) => `report_${item}`;
const keyExtractor = (item: OrderedReports) => `report_${item?.reportID}`;

function LHNOptionsList({
style,
contentContainerStyles,
data,
onSelectRow,
optionMode,
shouldDisableFocusOptions = false,
reports = {},
reportActions = {},
policy = {},
preferredLocale = CONST.LOCALES.DEFAULT,
personalDetails = {},
transactions = {},
currentReportID = '',
draftComments = {},
transactionViolations = {},
optionMode,

Check failure on line 21 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

'optionMode' is defined but never used
hurali97 marked this conversation as resolved.
Show resolved Hide resolved
onFirstItemRendered = () => {},
}: LHNOptionsListProps) {
const styles = useThemeStyles();
const {canUseViolations} = usePermissions();

// When the first item renders we want to call the onFirstItemRendered callback.
// At this point in time we know that the list is actually displaying items.
Expand All @@ -52,78 +39,38 @@
* Function which renders a row in the list
*/
const renderItem = useCallback(
({item: reportID}: RenderItemProps): ReactElement => {
const itemFullReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? null;
const itemReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? null;
const itemParentReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null;
const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null;
const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null;
const transactionID = itemParentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? itemParentReportAction.originalMessage.IOUTransactionID ?? '' : '';
const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? null;
const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? '';
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions);
const lastReportAction = sortedReportActions[0];

// Get the transaction for the last report action
let lastReportActionTransactionID = '';

if (lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU) {
lastReportActionTransactionID = lastReportAction.originalMessage?.IOUTransactionID ?? '';
}
const lastReportActionTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${lastReportActionTransactionID}`] ?? {};
({item}: RenderItemProps): ReactElement => {

Check failure on line 42 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected block statement surrounding arrow body; move the returned value immediately after the `=>`

return (
<OptionRowLHNData
reportID={reportID}
fullReport={itemFullReport}
reportActions={itemReportActions}
parentReportAction={itemParentReportAction}
policy={itemPolicy}
personalDetails={personalDetails ?? {}}
transaction={itemTransaction}
lastReportActionTransaction={lastReportActionTransaction}
receiptTransactions={transactions}
viewMode={optionMode}
isFocused={!shouldDisableFocusOptions && reportID === currentReportID}
reportID={item?.reportID}
isFocused={!shouldDisableFocusOptions && item?.reportID === currentReportID}
onSelectRow={onSelectRow}
preferredLocale={preferredLocale}
comment={itemComment}
transactionViolations={transactionViolations}
canUseViolations={canUseViolations}
comment={item?.comment}
optionItem={item?.optionItem}

Check failure on line 50 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Type '{ reportID: string; isFocused: boolean; onSelectRow: ((optionItem: OptionData, popoverAnchor: RefObject<View>) => void) | undefined; comment: string; optionItem: OptionData | undefined; onLayout: () => void; }' is not assignable to type 'IntrinsicAttributes & OptionRowLHNDataProps'.
onLayout={onLayoutItem}
/>
);
},
[

Check warning on line 55 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / lint

React Hook useCallback has a missing dependency: 'shouldDisableFocusOptions'. Either include it or remove the dependency array
currentReportID,
draftComments,
onSelectRow,
optionMode,
personalDetails,
policy,
preferredLocale,
reportActions,
reports,
shouldDisableFocusOptions,
transactions,
transactionViolations,
canUseViolations,
onLayoutItem,
],
);

return (
<View style={style ?? styles.flex1}>
<FlashList
<FlatList
indicatorStyle="white"
keyboardShouldPersistTaps="always"
contentContainerStyle={StyleSheet.flatten(contentContainerStyles)}
data={data}

Check failure on line 68 in src/components/LHNOptionsList/LHNOptionsList.tsx

View workflow job for this annotation

GitHub Actions / typecheck

No overload matches this call.
testID="lhn-options-list"
keyExtractor={keyExtractor}
renderItem={renderItem}
estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight}
extraData={[currentReportID]}
// estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight}
// extraData={[currentReportID]}
showsVerticalScrollIndicator={false}
/>
</View>
Expand All @@ -132,33 +79,6 @@

LHNOptionsList.displayName = 'LHNOptionsList';

export default withCurrentReportID(
withOnyx<LHNOptionsListProps, LHNOptionsListOnyxProps>({
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
},
reportActions: {
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
},
policy: {
key: ONYXKEYS.COLLECTION.POLICY,
},
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
},
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
transactions: {
key: ONYXKEYS.COLLECTION.TRANSACTION,
},
draftComments: {
key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT,
},
transactionViolations: {
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
},
})(memo(LHNOptionsList)),
);
export default memo(LHNOptionsList);

export type {LHNOptionsListProps};
51 changes: 1 addition & 50 deletions src/components/LHNOptionsList/OptionRowLHNData.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {deepEqual} from 'fast-equals';

Check failure on line 1 in src/components/LHNOptionsList/OptionRowLHNData.tsx

View workflow job for this annotation

GitHub Actions / lint

'deepEqual' is defined but never used
import React, {useEffect, useMemo, useRef} from 'react';

Check failure on line 2 in src/components/LHNOptionsList/OptionRowLHNData.tsx

View workflow job for this annotation

GitHub Actions / lint

'useMemo' is defined but never used
import * as ReportUtils from '@libs/ReportUtils';
import SidebarUtils from '@libs/SidebarUtils';
import * as Report from '@userActions/Report';
Expand All @@ -16,61 +16,12 @@
*/
function OptionRowLHNData({
isFocused = false,
fullReport,
reportActions,
personalDetails = {},
preferredLocale = CONST.LOCALES.DEFAULT,
comment,
policy,
receiptTransactions,
parentReportAction,
transaction,
lastReportActionTransaction = {},
transactionViolations,
canUseViolations,
optionItem,

Check failure on line 20 in src/components/LHNOptionsList/OptionRowLHNData.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Property 'optionItem' does not exist on type 'OptionRowLHNDataProps'.
...propsToForward
}: OptionRowLHNDataProps) {
const reportID = propsToForward.reportID;

const optionItemRef = useRef<OptionData>();

const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(fullReport, transactionViolations, parentReportAction ?? null);

const optionItem = useMemo(() => {
// Note: ideally we'd have this as a dependent selector in onyx!
const item = SidebarUtils.getOptionData({
report: fullReport,
reportActions,
personalDetails,
preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT,
policy,
parentReportAction,
hasViolations: !!hasViolations,
});
if (deepEqual(item, optionItemRef.current)) {
return optionItemRef.current;
}

optionItemRef.current = item;

return item;
// Listen parentReportAction to update title of thread report when parentReportAction changed
// Listen to transaction to update title of transaction report when transaction changed
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
fullReport,
lastReportActionTransaction,
reportActions,
personalDetails,
preferredLocale,
policy,
parentReportAction,
transaction,
transactionViolations,
canUseViolations,
receiptTransactions,
]);

useEffect(() => {
if (!optionItem || !!optionItem.hasDraftComment || !comment || comment.length <= 0 || isFocused) {
return;
Expand Down
6 changes: 2 additions & 4 deletions src/components/LHNOptionsList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@ import type CONST from '@src/CONST';
import type {OptionData} from '@src/libs/ReportUtils';
import type {Locale, PersonalDetailsList, Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx';
import type {EmptyObject} from '@src/types/utils/EmptyObject';
import { OrderedReports } from '@libs/SidebarUtils';

type OptionMode = ValueOf<typeof CONST.OPTION_MODE>;

type LHNOptionsListOnyxProps = {
/** The policy which the user has access to and which the report could be tied to */
policy: OnyxCollection<Policy>;

/** All reports shared with the user */
reports: OnyxCollection<Report>;

/** Array of report actions for this report */
reportActions: OnyxCollection<ReportActions>;

Expand Down Expand Up @@ -137,6 +135,6 @@ type OptionRowLHNProps = {
onLayout?: (event: LayoutChangeEvent) => void;
};

type RenderItemProps = {item: string};
type RenderItemProps = {item: OrderedReports};

export type {LHNOptionsListProps, OptionRowLHNDataProps, OptionRowLHNProps, LHNOptionsListOnyxProps, RenderItemProps};
10 changes: 9 additions & 1 deletion src/components/withCurrentReportID.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,15 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro
*/
const updateCurrentReportID = useCallback(
(state: NavigationState) => {
setCurrentReportID(Navigation.getTopmostReportId(state) ?? '');
const reportID = Navigation.getTopmostReportId(state) ?? '';
/**
* This is to make sure we don't set the undefined as reportID when
hurali97 marked this conversation as resolved.
Show resolved Hide resolved
* switching between chat list and settings->workspaces tab.
* and doing so avoid unnecessary re-render of `useOrderedReportIDs`.
*/
if (reportID) {
setCurrentReportID(reportID);
}
},
[setCurrentReportID],
);
Expand Down
Loading
Loading