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

Fix - Web - Search - New messages are not updated automatically in chats section #53482

Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
24 changes: 18 additions & 6 deletions src/components/Search/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ function mapToItemWithSelectionInfo(
shouldAnimateInHighlight: boolean,
) {
if (SearchUIUtils.isReportActionListItemType(item)) {
return item;
return {
...item,
shouldAnimateInHighlight,
};
}

return SearchUIUtils.isTransactionListItemType(item)
Expand Down Expand Up @@ -134,6 +137,8 @@ function Search({queryJSON, onSearchListScroll, isSearchScreenFocused, contentCo
const [currentSearchResults] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`);
const [transactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION);
const previousTransactions = usePrevious(transactions);
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);
const previousReportActions = usePrevious(reportActions);

useEffect(() => {
if (!currentSearchResults?.search?.type) {
Expand Down Expand Up @@ -211,6 +216,8 @@ function Search({queryJSON, onSearchListScroll, isSearchScreenFocused, contentCo
previousTransactions,
queryJSON,
offset,
reportActions,
previousReportActions,
});

// 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
Expand Down Expand Up @@ -323,15 +330,20 @@ function Search({queryJSON, onSearchListScroll, isSearchScreenFocused, contentCo

const ListItem = SearchUIUtils.getListItem(type, status);
const sortedData = SearchUIUtils.getSortedSections(type, status, data, sortBy, sortOrder);
const isChat = type === CONST.SEARCH.DATA_TYPES.CHAT;
const sortedSelectedData = sortedData.map((item) => {
const baseKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${(item as TransactionListItemType).transactionID}`;
const baseKey = isChat
? `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${(item as ReportActionListItemType).reportActionID}`
: `${ONYXKEYS.COLLECTION.TRANSACTION}${(item as TransactionListItemType).transactionID}`;
// Check if the base key matches the newSearchResultKey (TransactionListItemType)
const isBaseKeyMatch = baseKey === newSearchResultKey;
// Check if any transaction within the transactions array (ReportListItemType) matches the newSearchResultKey
const isAnyTransactionMatch = (item as ReportListItemType)?.transactions?.some((transaction) => {
const transactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`;
return transactionKey === newSearchResultKey;
});
const isAnyTransactionMatch =
!isChat &&
(item as ReportListItemType)?.transactions?.some((transaction) => {
const transactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`;
return transactionKey === newSearchResultKey;
});
// Determine if either the base key or any transaction key matches
const shouldAnimateInHighlight = isBaseKeyMatch || isAnyTransactionMatch;

Expand Down
20 changes: 18 additions & 2 deletions src/components/SelectionList/ChatListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import MentionReportContext from '@components/HTMLEngineProvider/HTMLRenderers/M
import MultipleAvatars from '@components/MultipleAvatars';
import {ShowContextMenuContext} from '@components/ShowContextMenuContext';
import TextWithTooltip from '@components/TextWithTooltip';
import useAnimatedHighlightStyle from '@hooks/useAnimatedHighlightStyle';
import useStyleUtils from '@hooks/useStyleUtils';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import ReportActionItemDate from '@pages/home/report/ReportActionItemDate';
import ReportActionItemFragment from '@pages/home/report/ReportActionItemFragment';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import BaseListItem from './BaseListItem';
import type {ChatListItemProps, ListItem, ReportActionListItemType} from './types';
Expand Down Expand Up @@ -56,11 +58,24 @@ function ChatListItem<TItem extends ListItem>({
const hoveredBackgroundColor = styles.sidebarLinkHover?.backgroundColor ? styles.sidebarLinkHover.backgroundColor : theme.sidebar;

const mentionReportContextValue = useMemo(() => ({currentReportID: item?.reportID ?? '-1'}), [item.reportID]);

const animatedHighlightStyle = useAnimatedHighlightStyle({
borderRadius: variables.componentBorderRadius,
shouldHighlight: item?.shouldAnimateInHighlight ?? false,
highlightColor: theme.messageHighlightBG,
backgroundColor: theme.highlightBG,
});
const pressableStyle = [
styles.selectionListPressableItemWrapper,
styles.textAlignLeft,
// Removing background style because they are added to the parent OpacityView via animatedHighlightStyle
styles.bgTransparent,
item.isSelected && styles.activeComponentBG,
item.cursorStyle,
];
return (
<BaseListItem
item={item}
pressableStyle={[[styles.selectionListPressableItemWrapper, styles.textAlignLeft, item.isSelected && styles.activeComponentBG, item.cursorStyle]]}
pressableStyle={pressableStyle}
wrapperStyle={[styles.flexRow, styles.flex1, styles.justifyContentBetween, styles.userSelectNone]}
containerStyle={styles.mb2}
isFocused={isFocused}
Expand All @@ -75,6 +90,7 @@ function ChatListItem<TItem extends ListItem>({
keyForList={item.keyForList}
onFocus={onFocus}
shouldSyncFocus={shouldSyncFocus}
pressableWrapperStyle={[styles.mh5, animatedHighlightStyle]}
hoverStyle={item.isSelected && styles.activeComponentBG}
>
{(hovered) => (
Expand Down
132 changes: 87 additions & 45 deletions src/hooks/useSearchHighlightAndScroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,52 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import type {SearchQueryJSON} from '@components/Search/types';
import type {ReportActionListItemType, ReportListItemType, SelectionListHandle, TransactionListItemType} from '@components/SelectionList/types';
import * as SearchActions from '@libs/actions/Search';
import {isReportActionEntry} from '@libs/SearchUIUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {SearchResults, Transaction} from '@src/types/onyx';
import type {ReportActions, SearchResults, Transaction} from '@src/types/onyx';
import usePrevious from './usePrevious';

type UseSearchHighlightAndScroll = {
searchResults: OnyxEntry<SearchResults>;
transactions: OnyxCollection<Transaction>;
previousTransactions: OnyxCollection<Transaction>;
reportActions: OnyxCollection<ReportActions>;
previousReportActions: OnyxCollection<ReportActions>;
queryJSON: SearchQueryJSON;
offset: number;
};

/**
* Hook used to trigger a search when a new transaction is added and handle highlighting and scrolling.
* Hook used to trigger a search when a new transaction or report action is added and handle highlighting and scrolling.
*/
function useSearchHighlightAndScroll({searchResults, transactions, previousTransactions, queryJSON, offset}: UseSearchHighlightAndScroll) {
function useSearchHighlightAndScroll({searchResults, transactions, previousTransactions, reportActions, previousReportActions, queryJSON, offset}: UseSearchHighlightAndScroll) {
// Ref to track if the search was triggered by this hook
const triggeredByHookRef = useRef(false);
const searchTriggeredRef = useRef(false);
const previousSearchResults = usePrevious(searchResults?.data);
const [newSearchResultKey, setNewSearchResultKey] = useState<string | null>(null);
const highlightedTransactionIDs = useRef<Set<string>>(new Set());
const highlightedIDs = useRef<Set<string>>(new Set());
const initializedRef = useRef(false);
const type = queryJSON.type;
FitseTLT marked this conversation as resolved.
Show resolved Hide resolved
const isChat = type === CONST.SEARCH.DATA_TYPES.CHAT;
FitseTLT marked this conversation as resolved.
Show resolved Hide resolved

// Trigger search when a new transaction is added
// Trigger search when a new report action is added while on chat or when a new transaction is added for the other search types.
useEffect(() => {
const previousTransactionsLength = previousTransactions && Object.keys(previousTransactions).length;
const transactionsLength = transactions && Object.keys(transactions).length;

// Return early if search was already triggered or there's no change in transactions length
if (searchTriggeredRef.current || previousTransactionsLength === transactionsLength) {
const reportActionsLength = reportActions && Object.values(reportActions).reduce((sum, curr) => sum + Object.keys(curr ?? {}).length, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be changed to this, no?

Suggested change
const reportActionsLength = reportActions && Object.values(reportActions).reduce((sum, curr) => sum + Object.keys(curr ?? {}).length, 0);
const reportActionsLength = reportActions && Object.values(reportActions).length;

And also below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope @marcochavezf the structure of reportActions is

{
reportAction_reportID:{
reportActionID:{},
reportActionID2:{}
...
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh cool, thanks for the clarification!

const prevReportActionsLength = previousReportActions && Object.values(previousReportActions).reduce((sum, curr) => sum + Object.keys(curr ?? {}).length, 0);
// Return early if search was already triggered or there's no change in current and previous data length
if (searchTriggeredRef.current || (!isChat && previousTransactionsLength === transactionsLength) || (isChat && reportActionsLength === prevReportActionsLength)) {
return;
}
const newTransactionAdded = transactionsLength && typeof previousTransactionsLength === 'number' && transactionsLength > previousTransactionsLength;
FitseTLT marked this conversation as resolved.
Show resolved Hide resolved
const newReportActionAdded = reportActionsLength && typeof prevReportActionsLength === 'number' && reportActionsLength > prevReportActionsLength;

// Check if a new transaction was added
if (transactionsLength && typeof previousTransactionsLength === 'number' && transactionsLength > previousTransactionsLength) {
// Check if a new transaction or report action was added
if ((!isChat && !!newTransactionAdded) || (isChat && !!newReportActionAdded)) {
// Set the flag indicating the search is triggered by the hook
triggeredByHookRef.current = true;

Expand All @@ -50,45 +59,62 @@ function useSearchHighlightAndScroll({searchResults, transactions, previousTrans
searchTriggeredRef.current = true;
}

// Reset the ref when transactions are updated
// Reset the ref when transactions (or report actions in chat search type) are updated
FitseTLT marked this conversation as resolved.
Show resolved Hide resolved
return () => {
searchTriggeredRef.current = false;
};
}, [transactions, previousTransactions, queryJSON, offset]);
}, [transactions, previousTransactions, queryJSON, offset, reportActions, previousReportActions, isChat]);

// Initialize the set with existing transaction IDs only once
// Initialize the set with existing IDs only once
useEffect(() => {
if (initializedRef.current || !searchResults?.data) {
return;
}

const existingTransactionIDs = extractTransactionIDsFromSearchResults(searchResults.data);
highlightedTransactionIDs.current = new Set(existingTransactionIDs);
const existingIDs = isChat ? extractReportActionIDsFromSearchResults(searchResults.data) : extractTransactionIDsFromSearchResults(searchResults.data);
highlightedIDs.current = new Set(existingIDs);
initializedRef.current = true;
}, [searchResults?.data]);
}, [searchResults?.data, isChat]);

// Detect new transactions
// Detect new items (transactions or report actions)
useEffect(() => {
if (!previousSearchResults || !searchResults?.data) {
return;
}
if (isChat) {
const previousReportActionIDs = extractReportActionIDsFromSearchResults(previousSearchResults);
const currentReportActionIDs = extractReportActionIDsFromSearchResults(searchResults.data);

const previousTransactionIDs = extractTransactionIDsFromSearchResults(previousSearchResults);
const currentTransactionIDs = extractTransactionIDsFromSearchResults(searchResults.data);
// Find new report action IDs that are not in the previousReportActionIDs and not already highlighted
const newReportActionIDs = currentReportActionIDs.filter((id) => !previousReportActionIDs.includes(id) && !highlightedIDs.current.has(id));

// Find new transaction IDs that are not in the previousTransactionIDs and not already highlighted
const newTransactionIDs = currentTransactionIDs.filter((id) => !previousTransactionIDs.includes(id) && !highlightedTransactionIDs.current.has(id));
if (!triggeredByHookRef.current || newReportActionIDs.length === 0) {
return;
}

if (!triggeredByHookRef.current || newTransactionIDs.length === 0) {
return;
}
const newReportActionID = newReportActionIDs.at(0) ?? '';
const newReportActionKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newReportActionID}`;

const newTransactionID = newTransactionIDs.at(0) ?? '';
const newTransactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${newTransactionID}`;
setNewSearchResultKey(newReportActionKey);
highlightedIDs.current.add(newReportActionID);
} else {
const previousTransactionIDs = extractTransactionIDsFromSearchResults(previousSearchResults);
const currentTransactionIDs = extractTransactionIDsFromSearchResults(searchResults.data);

setNewSearchResultKey(newTransactionKey);
highlightedTransactionIDs.current.add(newTransactionID);
}, [searchResults, previousSearchResults]);
// Find new transaction IDs that are not in the previousTransactionIDs and not already highlighted
const newTransactionIDs = currentTransactionIDs.filter((id) => !previousTransactionIDs.includes(id) && !highlightedIDs.current.has(id));

if (!triggeredByHookRef.current || newTransactionIDs.length === 0) {
return;
}

const newTransactionID = newTransactionIDs.at(0) ?? '';
const newTransactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${newTransactionID}`;

setNewSearchResultKey(newTransactionKey);
highlightedIDs.current.add(newTransactionID);
}
}, [searchResults?.data, previousSearchResults, isChat]);

// Reset newSearchResultKey after it's been used
useEffect(() => {
Expand All @@ -114,35 +140,41 @@ function useSearchHighlightAndScroll({searchResults, transactions, previousTrans
return;
}

// Extract the transaction ID from the newSearchResultKey
const newTransactionID = newSearchResultKey.replace(ONYXKEYS.COLLECTION.TRANSACTION, '');

// Find the index of the new transaction in the data array
const indexOfNewTransaction = data.findIndex((item) => {
// Handle TransactionListItemType
if ('transactionID' in item && item.transactionID === newTransactionID) {
return true;
}

// Handle ReportListItemType with transactions array
if ('transactions' in item && Array.isArray(item.transactions)) {
return item.transactions.some((transaction) => transaction?.transactionID === newTransactionID);
// Extract the transaction/report action ID from the newSearchResultKey
const newID = newSearchResultKey.replace(isChat ? ONYXKEYS.COLLECTION.REPORT_ACTIONS : ONYXKEYS.COLLECTION.TRANSACTION, '');

// Find the index of the new transaction/report action in the data array
const indexOfNewItem = data.findIndex((item) => {
if (isChat) {
if ('reportActionID' in item && item.reportActionID === newID) {
return true;
}
} else {
// Handle TransactionListItemType
if ('transactionID' in item && item.transactionID === newID) {
return true;
}

// Handle ReportListItemType with transactions array
if ('transactions' in item && Array.isArray(item.transactions)) {
return item.transactions.some((transaction) => transaction?.transactionID === newID);
}
}

return false;
});

// Early return if the transaction is not found in the data array
if (indexOfNewTransaction <= 0) {
// Early return if the new item is not found in the data array
if (indexOfNewItem <= 0) {
return;
}

// Perform the scrolling action
ref.scrollToIndex(indexOfNewTransaction);
ref.scrollToIndex(indexOfNewItem);
// Reset the trigger flag to prevent unintended future scrolls and highlights
triggeredByHookRef.current = false;
},
[newSearchResultKey],
[newSearchResultKey, isChat],
);

return {newSearchResultKey, handleSelectionListScroll};
Expand Down Expand Up @@ -174,4 +206,14 @@ function extractTransactionIDsFromSearchResults(searchResultsData: Partial<Searc
return transactionIDs;
}

/**
* Helper function to extract report action IDs from search results data.
*/
function extractReportActionIDsFromSearchResults(searchResultsData: Partial<SearchResults['data']>): string[] {
return Object.keys(searchResultsData ?? {})
.filter(isReportActionEntry)
.map((key) => Object.keys(searchResultsData[key] ?? {}))
.flat();
}

export default useSearchHighlightAndScroll;
1 change: 1 addition & 0 deletions src/libs/SearchUIUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,4 +585,5 @@ export {
getExpenseTypeTranslationKey,
getOverflowMenu,
isCorrectSearchUserName,
isReportActionEntry,
};
Loading