Android/iOS- Highlighted search result go to search page rather that the highlighted detail page
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.0.69-4 Reproducible in staging?: Y Reproducible in production?: N/A - new feature, doesn't exist in prod If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team
Action Performed:
-
Navigate to https://staging.new.expensify.com/
-
Go to search router
-
Write anything eg. "admin" and clear the search again ( to get the highlight on the first result)
-
Click search icon on keyboard (enter)
Expected Result:
The highlighted item should open the detail page when clicking enter (Mweb behavior)
Actual Result:
The highlighted search result go to search page rather that the highlighted detail page
Workaround:
Unknown
Platforms:
- [x] Android: Standalone
- [x] Android: HybridApp
- [ ] Android: mWeb Chrome
- [x] iOS: Standalone
- [x] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/29478d14-d77b-4c4a-9829-98b9fb474c3b
Triggered auto assignment to @luacmartins (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
💬 A slack conversation has been started in #expensify-open-source
Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
- Identify the pull request that introduced this issue and revert it.
- Find someone who can quickly fix the issue.
- Fix the issue yourself.
Demoting to NAB since it's just highlighting the wrong item. This is likely the offending PR @nyomanjyotisa @Pujan92
@luacmartins, @MitchExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!
@luacmartins This doesn't seem to be from https://github.com/Expensify/App/pull/52298, as we have specific logic on search input submit. I think for smaller screens the first item should not be highlighted in the selection list when the sections data are altered to avoid this issue.
The expected behavior here is to open the "Admin" room or whatever search row is highlighted, not return to search right? Its not super clear in the OP so wanted to double check what we're aiming for here @luacmartins
Correct, it should open the Admin room since that's what's highlighted
I think for smaller screens the first item should not be highlighted in the selection list when the sections data are altered to avoid this issue
Should we implement this then? Or should we update the logic on search input submit to open Admin room in this case? Because in native platforms, it will always execute submitSearch on enter click, instead of opening the focused item
I think the behavior should be consistent on all platforms, so we should highlight the first item again once the search is altered
Job added to Upwork: https://www.upwork.com/jobs/~021866990715543185912
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ikevin127 (External)
@luacmartins Could you please confirm the nexts steps here ?
- Is this issue considered a regression from #52298 ?
- Should the issue be addressed by author / reviewer as a follow-up or we're open for proposals ?
@luacmartins @MitchExpensify @ikevin127 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
Reminder on this @luacmartins
As pointed out here, I don't think this issue is a regression, so we're open to proposals
@luacmartins, @MitchExpensify, @ikevin127 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Still looking for proposals
Still looking for proposals
Edited by proposal-police: This proposal was edited at 2025-01-02 07:53:16 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Android/iOS- Highlighted search result go to search page rather that the highlighted detail page
What is the root cause of that problem?
On smaller screens, the top-most result in Recent Chats is not highlighted when the search router is opened for the first time. However, it will highlight the first item (from Recent Chats or Recent Searches, if available) when the user types in the search bar and then clears it. On native platforms specifically, every time the user presses the search icon on the keyboard (Enter), it will always execute submitSearch
https://github.com/Expensify/App/blob/207622d1d5c1e3c229cd4294c3b449d60fc76767/src/components/Search/SearchRouter/SearchRouter.tsx#L188-L203
And then will be called on this onSubmit
https://github.com/Expensify/App/blob/207622d1d5c1e3c229cd4294c3b449d60fc76767/src/components/Search/SearchRouter/SearchRouter.tsx#L281-L287
What changes do you think we should make in order to solve the problem?
We can create a new function that will handle the onSubmit. The idea is to navigate to the highlightedItem using onListItemPress if the textInputValue is not empty and it is not the initial state (i.e., the user has already typed and cleared the search query).
// Track if the user has typed initially
const [isInitialType, setIsInitialType] = useState(true);
We set it to false on onSearchQueryChange
if (userQuery.length > 0) {
setIsInitialType(false);
}
We also need recentSearchesData to retrieve the recent searches (if available). Because if there is recent searches, the highlighted item is the last recentSearchesData
const sortedRecentSearches = useMemo(() => {
return Object.values(recentSearches ?? {}).sort((a, b) => b.timestamp.localeCompare(a.timestamp));
}, [recentSearches]);
const recentSearchesData = sortedRecentSearches?.slice(0, 5).map(({query, timestamp}) => {
const searchQueryJSON = SearchQueryUtils.buildSearchQueryJSON(query);
return {
text: searchQueryJSON ? SearchQueryUtils.buildUserReadableQueryString(searchQueryJSON, personalDetails, reports, taxRates) : query,
singleIcon: Expensicons.History,
searchQuery: query,
keyForList: timestamp,
searchItemType: CONST.SEARCH.SEARCH_ROUTER_ITEM_TYPE.SEARCH,
};
});
const onSubmit = useCallback(() => {
if (!textInputValue && !isInitialType) {
const highlightedItem = recentSearchesData[0] || searchOptions.recentReports[0];
if (highlightedItem) {
onListItemPress(highlightedItem);
}
} else {
submitSearch(textInputValue);
}
}, [textInputValue, recentSearchesData, searchOptions.recentReports, onListItemPress, submitSearch]);
<SearchRouterInput
...
onSubmit={onSubmit}
...
To achieve the same behavior on mWeb version when the textInputValue is empty and there is no highlighted item, we can add onKeyPress event to this TextInput
onKeyPress={(event) => {
if (Browser.isMobile() && event.nativeEvent.key === 'Enter') {
onSubmit();
}
}}
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
What alternative solutions did you explore? (Optional)
POC
https://github.com/user-attachments/assets/523b1945-53e1-4641-916f-71a2f6ffa659
Edited by proposal-police: This proposal was edited at 2024-12-21 14:07:20 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Highlighted search result go to search page rather that the highlighted detail page
What is the root cause of that problem?
First, I will explain why the highlighted item on the mobile web can open the detail page when pressing Enter.
We use useKeyboardShortcut to listen for the Enter key press. When Enter is pressed, it executes the selectFocusedOption function to select the highlighted row in the list. and this useKeyboardShortcut only works on the mobile web (mWeb).
https://github.com/Expensify/App/blob/4cd759c6a12f37a7fe818537204f1e39ed882647/src/components/SelectionList/BaseSelectionList.tsx#L746-L752
https://github.com/Expensify/App/blob/4cd759c6a12f37a7fe818537204f1e39ed882647/src/components/SelectionList/BaseSelectionList.tsx#L399-L407
And the selected item, when Enter is pressed, will be handled for search or navigation.
https://github.com/Expensify/App/blob/eb896c68d2d36fccb0fb7a5ecef5360f62bd000b/src/components/Search/SearchRouter/SearchRouter.tsx#L301
https://github.com/Expensify/App/blob/eb896c68d2d36fccb0fb7a5ecef5360f62bd000b/src/components/Search/SearchRouter/SearchRouter.tsx#L239-L243
In this case, we encountered an issue (mweb) where, if no item in the list is highlighted, pressing Enter does not trigger any action.
https://github.com/user-attachments/assets/b74831e8-b08d-4fc8-8324-83d26cc3b447
For native apps (Android/iOS), we detect the Enter key press using onSubmitEditing.
https://github.com/Expensify/App/blob/9675b9cb27b8fd4170a6461adbcf62c9afdb5844/src/components/Search/SearchRouter/SearchRouterInput.tsx#L109
https://github.com/Expensify/App/blob/eb896c68d2d36fccb0fb7a5ecef5360f62bd000b/src/components/Search/SearchRouter/SearchRouter.tsx#L285-L287
When we press Enter, it calls submitSearch without handling the case for navigation to the highlighted item
https://github.com/Expensify/App/blob/eb896c68d2d36fccb0fb7a5ecef5360f62bd000b/src/components/Search/SearchRouter/SearchRouter.tsx#L188-L203
What changes do you think we should make in order to solve the problem?
To resolve this issue in this ticket, on the native app, when Enter is pressed, if the text input value is empty, we need to check if a row is highlighted and then navigate to it.
- We need to get the focused item in the selection list.
Create getFocusedItem function
https://github.com/Expensify/App/blob/4cd759c6a12f37a7fe818537204f1e39ed882647/src/components/SelectionList/BaseSelectionList.tsx#L398
const getFocusedItem = useCallback(() => {
const focusedOption = focusedIndex !== -1 ? flattenedSections.allOptions.at(focusedIndex) : undefined;
if (!focusedOption || (focusedOption.isDisabled && !focusedOption.isSelected)) {
return;
}
return focusedOption;
}, [focusedIndex, flattenedSections.allOptions]);
https://github.com/Expensify/App/blob/4cd759c6a12f37a7fe818537204f1e39ed882647/src/components/SelectionList/BaseSelectionList.tsx#L737-L743
Update to
useImperativeHandle(ref, () => ({scrollAndHighlightItem, clearInputAfterSelect, updateAndScrollToFocusedIndex, updateExternalTextInputFocus, scrollToIndex, getFocusedItem}), [
scrollAndHighlightItem,
clearInputAfterSelect,
updateAndScrollToFocusedIndex,
updateExternalTextInputFocus,
scrollToIndex,
getFocusedItem,
]);
- We will update
onSubmit
https://github.com/Expensify/App/blob/eb896c68d2d36fccb0fb7a5ecef5360f62bd000b/src/components/Search/SearchRouter/SearchRouter.tsx#L285-L287
onSubmit={() => {
const item = listRef.current?.getFocusedItem();
if (!item) {
submitSearch(textInputValue);
return;
}
onListItemPress(item);
}}
or
onSubmit={() => {
const item = listRef.current?.getFocusedItem();
if (!textInputValue && !isSearchQueryItem(item)) {
// or if (!textInputValue && item && !isSearchQueryItem(item)) {
onRouterClose();
if (item?.reportID) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(item?.reportID));
} else if ('login' in item) {
ReportUserActions.navigateToAndOpenReport(item.login ? [item.login] : [], false);
}
return;
}
submitSearch(textInputValue);
}}
Regarding the mWeb issue, if we want to fix it in this ticket, we will update it as follows:
https://github.com/Expensify/App/blob/9675b9cb27b8fd4170a6461adbcf62c9afdb5844/src/components/Search/SearchRouter/SearchRouterInput.tsx#L93
<TextInput
...
onKeyPress={(event) => {
const isWeb = getPlatform() === CONST.PLATFORM.WEB;
if (!isWeb) {
return;
}
if (event.nativeEvent.key !== 'Enter') {
return;
}
onSubmit();
}}
/>
POC
https://github.com/user-attachments/assets/53c1d61a-9f88-4f63-9cb6-2a81d60941ee
Test branch
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
Test getFocusedItem :
- should return undefined when focusedIndex is -1
- should return undefined when focused item is undefined
- should return undefined when focused item is disabled and not selected
- should return the focused item when it is valid
Test onSubmit logic:
- calls submitSearch when no item is focused
- calls onListItemPress with the focused item
What alternative solutions did you explore? (Optional)
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
@ikevin127 Proposals are ready for your review
Will review today.
Both proposals are suggesting similar solutions when it comes to our main issue with slightly different implementations, both are fixing the issue, but the second proposal is considering another case that is related / in scope. Before I make a decision I want to check with @Expensify/design and / or @luacmartins on whether we should address the related case here.
The issue is that, on mWeb, if no item is selected and input is empty and we press Enter aka software keyboard search button, nothing happens. Compared to Native where the search input page is dismissed (w/ no specific search query):
| iOS: Native | iOS: mWeb Safari |
|---|---|
IMO pressing Enter aka software keyboard search button should do something like it happens on Native, should dismiss the search input page with no specific search query, as to me it feels like something's broken on mWeb when it doesn't do anything.
I think we should fix the mWeb case such that we would have the same behaviour on both mWeb and Native. Please let me know what do you think!
Interesting, I think I tend to agree with you. Basically hitting enter on a blank Search query essentially is like resetting the search query back to nothing, aka searching for nothing which would just bring you back to the default home state of Search.
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@luacmartins, @MitchExpensify, @ikevin127 Whoops! This issue is 2 days overdue. Let's get this updated quick!
We're still awaiting some more input on https://github.com/Expensify/App/issues/53401#issuecomment-2560208680 regarding final decision.