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

Search - Highlight newly added expense #49192

Merged
merged 20 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4668b8a
Search - Highlight newly added expense
ikevin127 Sep 13, 2024
14818c8
fixed type issues
ikevin127 Sep 13, 2024
43a49eb
Merge branch 'main' of https://github.com/Expensify/App into ikevin12…
ikevin127 Sep 13, 2024
b38ba48
adjusted logic for new account (empty Search transaction list)
ikevin127 Sep 16, 2024
db55ff0
Merge branch 'main' of https://github.com/Expensify/App into ikevin12…
ikevin127 Sep 16, 2024
3da3f78
Merge branch 'main' of https://github.com/Expensify/App into ikevin12…
ikevin127 Sep 18, 2024
4595e26
added useScreenWrapperTransitionStatus mock to fix failing tests
ikevin127 Sep 19, 2024
24c9e8f
added BootSplash mock and adjusted previously added mock
ikevin127 Sep 19, 2024
aa9317b
targeted useScreenWrapperTransitionStatus mock to failing test files
ikevin127 Sep 19, 2024
babb58b
mocked useScreenWrapperTransitionStatus in failing perf test
ikevin127 Sep 20, 2024
72fd612
lint fix
ikevin127 Sep 20, 2024
9e43cbc
scroll to newly added expense functionality
ikevin127 Sep 24, 2024
05786c9
Merge branch 'main' of https://github.com/Expensify/App into ikevin12…
ikevin127 Sep 24, 2024
0f1bb60
hooks refactoring, fixed highlight animation issues
ikevin127 Sep 25, 2024
34c133f
Merge branch 'main' of https://github.com/Expensify/App into ikevin12…
ikevin127 Sep 25, 2024
cc892d8
fixed lint and prettier
ikevin127 Sep 26, 2024
90ddc4d
refactoring and bugfixing
ikevin127 Sep 27, 2024
265916d
Merge branch 'main' of https://github.com/Expensify/App into ikevin12…
ikevin127 Sep 27, 2024
c66f148
prettier
ikevin127 Sep 27, 2024
52c515a
Merge branch 'main' of https://github.com/Expensify/App into ikevin12…
ikevin127 Sep 30, 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
73 changes: 65 additions & 8 deletions src/components/Search/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {useNavigation} from '@react-navigation/native';
import type {StackNavigationProp} from '@react-navigation/stack';
import React, {useCallback, useEffect, useRef, useState} from 'react';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {InteractionManager} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
Expand Down Expand Up @@ -51,19 +52,26 @@ function mapTransactionItemToSelectedEntry(item: TransactionListItemType): [stri
return [item.keyForList, {isSelected: true, canDelete: item.canDelete, canHold: item.canHold, canUnhold: item.canUnhold, action: item.action}];
}

function mapToTransactionItemWithSelectionInfo(item: TransactionListItemType, selectedTransactions: SelectedTransactions, canSelectMultiple: boolean) {
return {...item, isSelected: selectedTransactions[item.keyForList]?.isSelected && canSelectMultiple};
function mapToTransactionItemWithSelectionInfo(item: TransactionListItemType, selectedTransactions: SelectedTransactions, canSelectMultiple: boolean, shouldAnimateInHighlight: boolean) {
return {...item, shouldAnimateInHighlight, isSelected: selectedTransactions[item.keyForList]?.isSelected && canSelectMultiple};
}

function mapToItemWithSelectionInfo(item: TransactionListItemType | ReportListItemType | ReportActionListItemType, selectedTransactions: SelectedTransactions, canSelectMultiple: boolean) {
function mapToItemWithSelectionInfo(
item: TransactionListItemType | ReportListItemType | ReportActionListItemType,
selectedTransactions: SelectedTransactions,
canSelectMultiple: boolean,
shouldAnimateInHighlight: boolean,
) {
if (SearchUtils.isReportActionListItemType(item)) {
return item;
}

return SearchUtils.isTransactionListItemType(item)
? mapToTransactionItemWithSelectionInfo(item, selectedTransactions, canSelectMultiple)
? mapToTransactionItemWithSelectionInfo(item, selectedTransactions, canSelectMultiple, shouldAnimateInHighlight)
: {
...item,
transactions: item.transactions?.map((transaction) => mapToTransactionItemWithSelectionInfo(transaction, selectedTransactions, canSelectMultiple)),
shouldAnimateInHighlight,
transactions: item.transactions?.map((transaction) => mapToTransactionItemWithSelectionInfo(transaction, selectedTransactions, canSelectMultiple, shouldAnimateInHighlight)),
isSelected: item.transactions.every((transaction) => selectedTransactions[transaction.keyForList]?.isSelected && canSelectMultiple),
};
}
Expand Down Expand Up @@ -96,6 +104,8 @@ function Search({queryJSON}: SearchProps) {
const {type, status, sortBy, sortOrder, hash} = queryJSON;

const [currentSearchResults] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`);
const [transactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION);
const previousTransactions = usePrevious(transactions);

const canSelectMultiple = isSmallScreenWidth ? !!selectionMode?.isEnabled : true;

Expand Down Expand Up @@ -123,9 +133,41 @@ function Search({queryJSON}: SearchProps) {
}

SearchActions.search({queryJSON, offset});
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [isOffline, offset, queryJSON]);

const searchTriggeredRef = useRef(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Request: can we move this animation/scrolling logic into a custom hook so we can keep the logic in the Search component small?


useEffect(() => {
const previousTransactionsLength = previousTransactions && Object.keys(previousTransactions).length;
const transactionsLength = transactions && Object.keys(transactions).length;

if (!previousTransactionsLength || !transactionsLength || previousTransactionsLength === transactionsLength) {
return;
}

// Check if the search block was already triggered
if (!searchTriggeredRef.current && transactionsLength > previousTransactionsLength) {
// A transaction was added, trigger the action again
SearchActions.search({queryJSON, offset});
searchTriggeredRef.current = true; // Set the flag to true to prevent further triggers
}
}, [transactions, previousTransactions, queryJSON, offset]);

// Reset the flag using InteractionManager after the search action
useEffect(() => {
if (!searchTriggeredRef.current) {
return;
}

// Schedule a task to reset the flag after all current interactions have completed
const interactionHandle = InteractionManager.runAfterInteractions(() => {
searchTriggeredRef.current = false; // Reset the flag after interactions
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@luacmartins Ideally, we would update the Search snapshot in BE and pop the newly added transaction in the search list via pusher and we wouldn't have to check transactions / previous transactions in this way - but it's better than performing a full page reload from a UX point of view, given that we want to pop the row in with a nice animation.

// Cleanup the interaction handle if the component unmounts or effect reruns
return () => interactionHandle.cancel();
}, [transactions]);

const handleOnCancelConfirmModal = () => {
setDeleteExpensesConfirmModalVisible(false);
};
Expand Down Expand Up @@ -182,6 +224,18 @@ function Search({queryJSON}: SearchProps) {
}

const searchResults = currentSearchResults?.data ? currentSearchResults : lastSearchResultsRef.current;
const previousSearchResults = usePrevious(searchResults?.data);

const newSearchResultKey = useMemo(() => {
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
if (!previousSearchResults || !searchResults?.data) {
return null;
}

const previousKeys = Object.keys(previousSearchResults);
const currentKeys = Object.keys(searchResults.data);

return currentKeys.find((key) => !previousKeys.includes(key));
}, [searchResults, previousSearchResults]);

// There's a race condition in Onyx which makes it return data from the previous Search, so in addition to checking that the data is loaded
// we also need to check that the searchResults matches the type and status of the current search
Expand Down Expand Up @@ -229,7 +283,10 @@ function Search({queryJSON}: SearchProps) {
const ListItem = SearchUtils.getListItem(type, status);
const data = SearchUtils.getSections(type, status, searchResults.data, searchResults.search);
const sortedData = SearchUtils.getSortedSections(type, status, data, sortBy, sortOrder);
const sortedSelectedData = sortedData.map((item) => mapToItemWithSelectionInfo(item, selectedTransactions, canSelectMultiple));
const sortedSelectedData = sortedData.map((item) => {
const shouldAnimateInHighlight = `${ONYXKEYS.COLLECTION.TRANSACTION}${(item as TransactionListItemType).transactionID}` === newSearchResultKey;
return mapToItemWithSelectionInfo(item, selectedTransactions, canSelectMultiple, shouldAnimateInHighlight);
});

const shouldShowEmptyState = !isDataLoaded || data.length === 0;

Expand Down
11 changes: 11 additions & 0 deletions src/components/SelectionList/BaseListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import PressableWithFeedback from '@components/Pressable/PressableWithFeedback';
import useAnimatedHighlightStyle from '@hooks/useAnimatedHighlightStyle';
import useHover from '@hooks/useHover';
import {useMouseContext} from '@hooks/useMouseContext';
import useSyncFocus from '@hooks/useSyncFocus';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import type {BaseListItemProps, ListItem} from './types';

Expand All @@ -33,6 +35,7 @@ function BaseListItem<TItem extends ListItem>({
onFocus = () => {},
hoverStyle,
onLongPressRow,
shouldAnimateInHighlight = false,
}: BaseListItemProps<TItem>) {
const theme = useTheme();
const styles = useThemeStyles();
Expand Down Expand Up @@ -60,6 +63,13 @@ function BaseListItem<TItem extends ListItem>({
return rightHandSideComponent;
};

const animatedHighlightStyle = useAnimatedHighlightStyle({
borderRadius: variables.componentBorderRadius,
shouldHighlight: item?.shouldAnimateInHighlight ?? false,
highlightColor: theme.messageHighlightBG,
backgroundColor: theme.highlightBG,
});

return (
<OfflineWithFeedback
onClose={() => onDismissError(item)}
Expand Down Expand Up @@ -98,6 +108,7 @@ function BaseListItem<TItem extends ListItem>({
onFocus={onFocus}
onMouseLeave={handleMouseLeave}
tabIndex={item.tabIndex}
wrapperStyle={shouldAnimateInHighlight ? [styles.mh5, animatedHighlightStyle] : []}
>
<View style={wrapperStyle}>
{typeof children === 'function' ? children(hovered) : children}
Expand Down
12 changes: 11 additions & 1 deletion src/components/SelectionList/Search/TransactionListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,16 @@ function TransactionListItem<TItem extends ListItem>({

const {isLargeScreenWidth} = useResponsiveLayout();

const listItemPressableStyle = [styles.selectionListPressableItemWrapper, styles.pv3, styles.ph3, item.isSelected && styles.activeComponentBG, isFocused && styles.sidebarLinkActive];
const listItemPressableStyle = [
styles.selectionListPressableItemWrapper,
styles.pv3,
styles.ph3,
item.isSelected && styles.activeComponentBG,
isFocused && styles.sidebarLinkActive,
// Removing some of the styles because they are added to the parent OpacityView via animatedHighlightStyle
{backgroundColor: 'unset'},
styles.mh0,
];

const listItemWrapperStyle = [
styles.flex1,
Expand All @@ -50,6 +59,7 @@ function TransactionListItem<TItem extends ListItem>({
onLongPressRow={onLongPressRow}
shouldSyncFocus={shouldSyncFocus}
hoverStyle={item.isSelected && styles.activeComponentBG}
shouldAnimateInHighlight
>
<TransactionListItemRow
item={transactionItem}
Expand Down
4 changes: 4 additions & 0 deletions src/components/SelectionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ type ListItem = {

/** The style to override the cursor appearance */
cursorStyle?: CursorStyles[keyof CursorStyles];

/** Determines whether the newly added item should animate in / highlight */
shouldAnimateInHighlight?: boolean;
};

type TransactionListItemType = ListItem &
Expand Down Expand Up @@ -273,6 +276,7 @@ type BaseListItemProps<TItem extends ListItem> = CommonListItemProps<TItem> & {
children?: ReactElement<ListItemProps<TItem>> | ((hovered: boolean) => ReactElement<ListItemProps<TItem>>);
shouldSyncFocus?: boolean;
hoverStyle?: StyleProp<ViewStyle>;
shouldAnimateInHighlight: boolean;
};

type UserListItemProps<TItem extends ListItem> = ListItemProps<TItem> & {
Expand Down
17 changes: 14 additions & 3 deletions src/hooks/useAnimatedHighlightStyle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type Props = {
borderRadius: number;

/** Height of the item that is to be faded */
height: number;
height?: number;

/** Delay before the highlighted item enters */
itemEnterDelay?: number;
Expand All @@ -32,6 +32,15 @@ type Props = {

/** Whether the item should be highlighted */
shouldHighlight: boolean;

/** The base backgroundColor used for the highlight animation, defaults to theme.appBG
* @default theme.appBG
*/
backgroundColor?: string;
/** The base highlightColor used for the highlight animation, defaults to theme.border
* @default theme.border
*/
highlightColor?: string;
};

/**
Expand All @@ -47,6 +56,8 @@ export default function useAnimatedHighlightStyle({
highlightEndDelay = CONST.ANIMATED_HIGHLIGHT_END_DELAY,
highlightEndDuration = CONST.ANIMATED_HIGHLIGHT_END_DURATION,
height,
highlightColor,
backgroundColor,
}: Props) {
const [startHighlight, setStartHighlight] = useState(false);
const repeatableProgress = useSharedValue(0);
Expand All @@ -55,8 +66,8 @@ export default function useAnimatedHighlightStyle({
const theme = useTheme();

const highlightBackgroundStyle = useAnimatedStyle(() => ({
backgroundColor: interpolateColor(repeatableProgress.value, [0, 1], [theme.appBG, theme.border]),
height: interpolate(nonRepeatableProgress.value, [0, 1], [0, height]),
backgroundColor: interpolateColor(repeatableProgress.value, [0, 1], [backgroundColor ?? theme.appBG, highlightColor ?? theme.border]),
height: height ? interpolate(nonRepeatableProgress.value, [0, 1], [0, height]) : 'auto',
opacity: interpolate(nonRepeatableProgress.value, [0, 1], [0, 1]),
borderRadius,
}));
Expand Down
Loading