Skip to content

Commit dfbfae2

Browse files
authored
Merge pull request Expensify#89791 from software-mansion-labs/korytko/perf/reopen/fix-search-dismiss
Re-land smooth post-submit Spend tab navigation with Android skeleton fix
2 parents 39d1d17 + 427e5c6 commit dfbfae2

24 files changed

Lines changed: 964 additions & 213 deletions

src/CONST/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7582,6 +7582,7 @@ const CONST = {
75827582

75837583
DOWNLOADS_PATH: '/Downloads',
75847584
DOWNLOADS_TIMEOUT: 5000,
7585+
75857586
NEW_EXPENSIFY_PATH: '/New Expensify',
75867587
RECEIPTS_UPLOAD_PATH: '/Receipts-Upload',
75877588

src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
2626
import useWindowDimensions from '@hooks/useWindowDimensions';
2727
import {isConsecutiveChronosAutomaticTimerAction} from '@libs/ChronosUtils';
2828
import DateUtils from '@libs/DateUtils';
29+
import {hasDeferredWriteForReport} from '@libs/deferredLayoutWrite';
2930
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
3031
import {getAllNonDeletedTransactions, isActionVisibleOnMoneyRequestReport} from '@libs/MoneyRequestReportUtils';
3132
import Navigation from '@libs/Navigation/Navigation';
@@ -662,6 +663,16 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
662663
return numToRender || undefined;
663664
}, [styles.chatItem.paddingBottom, styles.chatItem.paddingTop, windowHeight, linkedReportActionID]);
664665

666+
const isReportEmpty = isEmpty(visibleReportActions) && isEmpty(transactions) && !showReportActionsLoadingState;
667+
// hasDeferredWriteForReport is non-reactive (reads a module-level Map, not tracked by React).
668+
// This is intentional: we only check on the initial render after the RHP dismisses.
669+
// Once the deferred write flushes and createTransaction runs, Onyx updates make
670+
// transactions non-empty, which drives the transition away from the skeleton.
671+
// Scoping to `report.reportID` ensures an unrelated submit flow's pending dismiss doesn't keep
672+
// *this* report stuck on the skeleton.
673+
const isAwaitingDeferredTransaction = isReportEmpty && hasDeferredWriteForReport(CONST.DEFERRED_LAYOUT_WRITE_KEYS.DISMISS_MODAL, report?.reportID);
674+
const showEmptyState = isReportEmpty && !isAwaitingDeferredTransaction;
675+
665676
if (!report) {
666677
return null;
667678
}
@@ -682,7 +693,12 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
682693
isActive={isFloatingMessageCounterVisible}
683694
onClick={scrollToBottomAndMarkReportAsRead}
684695
/>
685-
{isEmpty(visibleReportActions) && isEmpty(transactions) && !showReportActionsLoadingState ? (
696+
{/* Exactly one of these three branches is active at a time:
697+
1. isAwaitingDeferredTransaction — skeleton while dismiss-first creates the transaction
698+
2. showEmptyState — genuinely empty report
699+
3. !isReportEmpty — report has data, render the FlatList */}
700+
{isAwaitingDeferredTransaction && <ReportActionsListLoadingSkeleton reasonAttributes={skeletonReasonAttributes} />}
701+
{showEmptyState && (
686702
<ScrollView contentContainerStyle={styles.flexGrow1}>
687703
<MoneyRequestViewReportFields
688704
report={report}
@@ -694,7 +710,8 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
694710
policy={policy}
695711
/>
696712
</ScrollView>
697-
) : (
713+
)}
714+
{!isReportEmpty && (
698715
<FlatListWithScrollKey
699716
initialNumToRender={initialNumToRender}
700717
accessibilityLabel={translate('sidebarScreen.listOfChatMessages')}

src/components/ReportActionItem/MoneyReportView.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ type MoneyReportViewProps = {
7070

7171
/** Whether we should display the animated banner above the component */
7272
shouldShowAnimatedBackground?: boolean;
73+
74+
/**
75+
* When true, the Total amount is rendered as a loading indicator regardless of `isOffline`.
76+
* Use this when the caller knows the underlying total is being recomputed and a
77+
* network-independent update is expected, so falling back to the (stale) amount while offline
78+
* would be misleading.
79+
*/
80+
isTotalPending?: boolean;
7381
};
7482

7583
function MoneyReportView({
@@ -80,6 +88,7 @@ function MoneyReportView({
8088
shouldHideThreadDividerLine,
8189
pendingAction,
8290
shouldShowAnimatedBackground = true,
91+
isTotalPending = false,
8392
}: MoneyReportViewProps) {
8493
const theme = useTheme();
8594
const styles = useThemeStyles();
@@ -89,7 +98,7 @@ function MoneyReportView({
8998
const {convertToDisplayString} = useCurrencyListActions();
9099
const {isOffline} = useNetwork();
91100
const isSettled = isSettledReportUtils(report?.reportID);
92-
const isTotalUpdated = hasUpdatedTotal(report, policy);
101+
const isTotalUpdated = hasUpdatedTotal(report, policy) && !isTotalPending;
93102

94103
const {totalDisplaySpend, nonReimbursableSpend, reimbursableSpend} = getMoneyRequestSpendBreakdown(report);
95104
const transactions = useReportTransactions(report?.reportID);
@@ -107,6 +116,7 @@ function MoneyReportView({
107116
context: 'MoneyReportView.Total',
108117
isTotalUpdated,
109118
isOffline,
119+
isTotalPending,
110120
};
111121

112122
const subAmountTextStyles: StyleProp<TextStyle> = [
@@ -228,7 +238,7 @@ function MoneyReportView({
228238
/>
229239
</View>
230240
)}
231-
{!isTotalUpdated && !isOffline ? (
241+
{!isTotalUpdated && (!isOffline || isTotalPending) ? (
232242
<ActivityIndicator
233243
style={[styles.moneyRequestLoadingHeight]}
234244
color={theme.textSupporting}

src/components/Search/SearchPageHeader/useSearchFiltersBar.tsx

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,11 @@ import {useCurrencyListActions} from '@hooks/useCurrencyList';
1212
import useLocalize from '@hooks/useLocalize';
1313
import useNetwork from '@hooks/useNetwork';
1414
import useOnyx from '@hooks/useOnyx';
15-
import {FILTER_LABEL_MAP, isAmountFilterKey, isDateFilterKey, mapFiltersFormToLabelValueList} from '@libs/SearchUIUtils';
15+
import {FILTER_LABEL_MAP, isAmountFilterKey, isDateFilterKey, mapFiltersFormToLabelValueList, SKIPPED_SEARCH_FILTERS} from '@libs/SearchUIUtils';
1616
import type {SearchFilter} from '@libs/SearchUIUtils';
1717
import CONST from '@src/CONST';
1818
import ONYXKEYS from '@src/ONYXKEYS';
1919
import type {SearchAdvancedFiltersForm} from '@src/types/form';
20-
import FILTER_KEYS from '@src/types/form/SearchAdvancedFiltersForm';
2120
import type {SearchAdvancedFiltersKey} from '@src/types/form/SearchAdvancedFiltersForm';
2221
import {getEmptyObject} from '@src/types/utils/EmptyObject';
2322
import type WithSentryLabel from '@src/types/utils/SentryLabel';
@@ -42,18 +41,6 @@ type FilterPopupProps = {
4241
setPopoverWidth: PopoverComponentProps['setPopoverWidth'];
4342
};
4443

45-
const SKIPPED_FILTERS = new Set<SearchAdvancedFiltersKey>([
46-
FILTER_KEYS.GROUP_BY,
47-
FILTER_KEYS.GROUP_CURRENCY,
48-
FILTER_KEYS.LIMIT,
49-
FILTER_KEYS.TYPE,
50-
FILTER_KEYS.VIEW,
51-
FILTER_KEYS.PAYER,
52-
FILTER_KEYS.ACTION,
53-
FILTER_KEYS.COLUMNS,
54-
FILTER_KEYS.KEYWORD,
55-
]);
56-
5744
function getFilterSentryLabel(filterKey: SearchAdvancedFiltersKey | SearchFilterKey | ReportFieldKey) {
5845
return `Search-Filter-${filterKey}`;
5946
}
@@ -132,7 +119,7 @@ function useSearchFiltersBar(queryJSON: SearchQueryJSON): UseSearchFiltersBarRes
132119
const filters = mapFiltersFormToLabelValueList<FilterItem>(
133120
searchAdvancedFiltersForm,
134121
queryJSON.policyID,
135-
SKIPPED_FILTERS,
122+
SKIPPED_SEARCH_FILTERS,
136123
translate,
137124
localeCompare,
138125
convertToDisplayStringWithoutCurrency,
@@ -192,4 +179,3 @@ function useSearchFiltersBar(queryJSON: SearchQueryJSON): UseSearchFiltersBarRes
192179

193180
export default useSearchFiltersBar;
194181
export type {FilterItem};
195-
export {SKIPPED_FILTERS};

src/components/Search/SearchStaticList.tsx

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import {hasDeferredWrite} from '@libs/deferredLayoutWrite';
3131
import Navigation from '@libs/Navigation/Navigation';
3232
import {getReportStatusColorStyle, getReportStatusTranslation, isOneTransactionReport} from '@libs/ReportUtils';
3333
import {createAndOpenSearchTransactionThread, getSections, getSortedSections, getValidGroupBy} from '@libs/SearchUIUtils';
34-
import {getPendingSubmitFollowUpAction} from '@libs/telemetry/submitFollowUpAction';
3534
import CONST from '@src/CONST';
3635
import ROUTES from '@src/ROUTES';
3736
import type {SearchResults} from '@src/types/onyx';
@@ -110,12 +109,10 @@ function SearchStaticList({
110109
// the destination is visible (focus signal for the dual-gate span ending).
111110
useFocusEffect(
112111
useCallback(() => {
113-
const hasPendingAction = getPendingSubmitFollowUpAction()?.followUpAction === CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.NAVIGATE_TO_SEARCH;
114-
if (!showPendingExpensePlaceholder && hasPendingAction) {
112+
const hasPendingWrite = hasDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH);
113+
if (!showPendingExpensePlaceholder && hasPendingWrite) {
115114
setShowPendingExpensePlaceholder(true);
116-
} else if (showPendingExpensePlaceholder && !hasPendingAction && sortedData.length > 0) {
117-
// Only clear the placeholder once real data is available to avoid
118-
// a blank flash when the stale snapshot has been filtered empty.
115+
} else if (showPendingExpensePlaceholder && !hasPendingWrite && sortedData.length > 0) {
119116
setShowPendingExpensePlaceholder(false);
120117
}
121118

@@ -370,7 +367,7 @@ function SearchStaticList({
370367
<SearchRowSkeleton
371368
shouldAnimate
372369
fixedNumItems={1}
373-
isLoadMore={!shouldUseNarrowLayout}
370+
isLoadMore
374371
reasonAttributes={pendingExpenseReasonAttributes}
375372
/>
376373
) : undefined

src/components/Search/index.tsx

Lines changed: 72 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import {
5757
isTransactionGroupListItemType,
5858
isTransactionListItemType,
5959
isTransactionReportGroupListItemType,
60+
isTransactionSearchType,
6061
shouldShowEmptyState,
6162
shouldShowYear as shouldShowYearUtil,
6263
} from '@libs/SearchUIUtils';
@@ -84,7 +85,6 @@ import {useSearchActionsContext, useSearchStateContext} from './SearchContext';
8485
import SearchList from './SearchList';
8586
import type {ReportActionListItemType, SearchListItem, TransactionGroupListItemType, TransactionListItemType, TransactionReportGroupListItemType} from './SearchList/ListItem/types';
8687
import {SearchScopeProvider} from './SearchScopeProvider';
87-
import SearchStaticList from './SearchStaticList';
8888
import SearchTableHeader from './SearchTableHeader';
8989
import type {SearchColumnType, SearchParams, SearchQueryJSON, SelectedTransactionInfo, SelectedTransactions, SortOrder} from './types';
9090

@@ -282,9 +282,14 @@ function Search({
282282
// Flush (not cancel) on unmount so the API.write() still executes if the
283283
// user navigates away before onLayout fires. This also clears the channel,
284284
// preventing a stale hasDeferredWrite() on the next mount.
285+
// Skip when navigate_to_search is pending: the old Search instance is being
286+
// replaced by a new one (route swap), and the new instance will handle the
287+
// flush via its own layout/focus callbacks.
285288
useEffect(
286289
() => () => {
287-
flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH);
290+
if (getPendingSubmitFollowUpAction()?.followUpAction !== CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.NAVIGATE_TO_SEARCH) {
291+
flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH);
292+
}
288293
if (rollbackTimeoutRef.current) {
289294
clearTimeout(rollbackTimeoutRef.current);
290295
}
@@ -1476,14 +1481,23 @@ function Search({
14761481
searchResults?.data,
14771482
]);
14781483

1479-
const onLayout = useCallback(() => {
1484+
const onLayoutBase = useCallback(() => {
14801485
hasHadFirstLayout.current = true;
14811486
onDestinationVisible?.(isSearchResultsEmptyRef.current, 'layout');
14821487
endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, {[CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: true});
1488+
TransitionTracker.runAfterTransitions({
1489+
callback: () => flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH),
1490+
});
1491+
}, [onDestinationVisible]);
1492+
1493+
// Deferred layout only needs the base work (no scroll handling, no content-ready signal).
1494+
const onDeferredLayout = onLayoutBase;
1495+
1496+
const onLayout = useCallback(() => {
1497+
onLayoutBase();
14831498
handleSelectionListScroll(stableSortedData, searchListRef.current);
1484-
flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH);
14851499
onContentReady?.();
1486-
}, [handleSelectionListScroll, stableSortedData, onContentReady, onDestinationVisible]);
1500+
}, [onLayoutBase, handleSelectionListScroll, stableSortedData, onContentReady]);
14871501

14881502
// Must be a ref, not state: cancelNavigationSpans is called during render
14891503
// (inside conditional returns), so using setState would trigger infinite re-renders.
@@ -1516,22 +1530,65 @@ function Search({
15161530
endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, {[CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: true});
15171531
}, []);
15181532

1533+
// Tracks whether the pending-expense tracking was re-armed on re-focus
1534+
// (subsequent expense creation while Search stays mounted). Used by the
1535+
// effect below to dismiss the overlay once the deferred write completes,
1536+
// since onLayout won't re-fire for already-mounted content.
1537+
const wasRearmedRef = useRef(false);
1538+
15191539
// On re-visits, react-freeze serves the cached layout — onLayout/onLayoutSkeleton never fire.
15201540
// useFocusEffect fires on unfreeze, which is when the screen becomes visible.
15211541
useFocusEffect(
15221542
useCallback(() => {
15231543
if (!hasHadFirstLayout.current) {
15241544
return;
15251545
}
1546+
1547+
// Re-arm pending expense skeleton for subsequent creations while Search
1548+
// stays mounted (the original hasPendingWriteOnMountRef only covers the first).
1549+
if (hasDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH) && !showPendingExpensePlaceholder) {
1550+
wasRearmedRef.current = true;
1551+
1552+
// Let the optimistic tracking effect know it should watch for the new item.
1553+
hasPendingWriteOnMountRef.current = true;
1554+
optimisticTrackingCleanedUpRef.current = false;
1555+
setIsOptimisticTrackingCleared(false);
1556+
1557+
setShowPendingExpensePlaceholder(true);
1558+
// Prevent the full-page SearchRowSkeleton from rendering (shouldShowRowSkeleton),
1559+
// which would cause a blink as SelectionList unmounts/remounts.
1560+
setSkeletonWasDisplayed(true);
1561+
1562+
// Clear stale cached item so the new optimistic row is picked up fresh.
1563+
cachedOptimisticItemRef.current = null;
1564+
const latestKey = getOptimisticWatchKey(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH);
1565+
if (latestKey) {
1566+
optimisticWatchKeyRef.current = latestKey;
1567+
}
1568+
}
1569+
15261570
onDestinationVisible?.(isSearchResultsEmptyRef.current, 'focus');
15271571
endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, {
15281572
[CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: !shouldShowLoadingState,
15291573
});
15301574
// On re-focus (e.g. DISMISS_MODAL_ONLY) onLayout won't re-fire — flush here.
15311575
flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH);
1532-
}, [shouldShowLoadingState, onDestinationVisible]),
1576+
}, [shouldShowLoadingState, onDestinationVisible, showPendingExpensePlaceholder]),
15331577
);
15341578

1579+
// Dismiss the overlay after a re-armed deferred write completes. On re-focus,
1580+
// onLayout doesn't re-fire (content already mounted), so onContentReady is
1581+
// never called via the normal path. This effect detects when the deferred
1582+
// write channel is gone (write executed) and sortedData has updated, then
1583+
// signals overlay readiness.
1584+
useEffect(() => {
1585+
if (!wasRearmedRef.current || hasDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH)) {
1586+
return;
1587+
}
1588+
wasRearmedRef.current = false;
1589+
onContentReady?.();
1590+
}, [sortedData, onContentReady]);
1591+
15351592
// Reset before conditional returns. Only cancelNavigationSpans (error/empty paths)
15361593
// sets it to true. Must happen during render since it coordinates with the
15371594
// dep-free useEffect above — see comment on didBailToFallbackState.
@@ -1579,25 +1636,14 @@ function Search({
15791636
);
15801637

15811638
// When heavy work is deferred (e.g. during the RHP dismiss animation after
1582-
// submitting an expense), show a lightweight static list instead of the skeleton.
1583-
// This gives the user real-looking content during the animation while avoiding
1584-
// the expensive hooks and renders of the full Search component.
1585-
// Restricted to transaction-based search types (expense/invoice) because
1586-
// SearchStaticList only renders rows with a transactionID - non-transaction
1587-
// types (chat, task, report) would render empty/blank during the deferral.
1588-
const isTransactionSearchType = type === CONST.SEARCH.DATA_TYPES.EXPENSE || type === CONST.SEARCH.DATA_TYPES.INVOICE;
1589-
if (isDeferringHeavyWork && searchResults?.data && isTransactionSearchType) {
1590-
return (
1591-
<SearchStaticList
1592-
searchResults={searchResults}
1593-
queryJSON={queryJSON}
1594-
shouldUseNarrowLayout={shouldUseNarrowLayout}
1595-
canSelectMultiple={canSelectMultiple}
1596-
columns={currentColumns}
1597-
contentContainerStyle={shouldUseNarrowLayout ? styles.searchListContentContainerStyles(!!hasFilterBars) : undefined}
1598-
onLayout={onLayout}
1599-
/>
1600-
);
1639+
// submitting an expense), skip the expensive render below. The ancestor
1640+
// SearchPage (via SearchPageNarrow / SearchPageWide) renders a SearchStaticList
1641+
// overlay that covers this component, so the user sees real-looking content.
1642+
// The minimal View fires onLayout to flush the deferred API write and set
1643+
// hasHadFirstLayout.
1644+
if (isDeferringHeavyWork && searchResults?.data && isTransactionSearchType(type)) {
1645+
// Zero-sized View - onLayout still fires on RN, which is all we need here.
1646+
return <View onLayout={onDeferredLayout} />;
16011647
}
16021648

16031649
// This is a performance optimization for the submit-expense->search path only.
@@ -1768,7 +1814,7 @@ function Search({
17681814
shouldAnimate
17691815
fixedNumItems={shouldShowLoadingMoreItems ? 5 : 1}
17701816
reasonAttributes={showPendingExpensePlaceholder ? pendingExpenseReasonAttributes : loadMoreSkeletonReasonAttributes}
1771-
isLoadMore={shouldShowLoadingMoreItems}
1817+
isLoadMore
17721818
/>
17731819
) : undefined
17741820
}

0 commit comments

Comments
 (0)