[$250] Android/iOS- Highlighted search result go to search page rather that the highlighted detail page #53401

IuliiaHerets · 2024-12-02T20:15:50Z

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:

Screenshots/Videos

Bug6682275_1733167237785.Screen_Recording_20241202_221823_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021866990715543185912
Upwork Job ID: 1866990715543185912
Last Price Increase: 2024-12-25

Issue Owner

Current Issue Owner: @ikevin127

melvin-bot · 2024-12-02T20:15:58Z

Triggered auto assignment to @luacmartins (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

melvin-bot · 2024-12-02T20:16:00Z

💬 A slack conversation has been started in #expensify-open-source

melvin-bot · 2024-12-02T20:16:00Z

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.

github-actions · 2024-12-02T20:16:06Z

👋 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.

luacmartins · 2024-12-02T20:27:12Z

Demoting to NAB since it's just highlighting the wrong item. This is likely the offending PR @nyomanjyotisa @Pujan92

melvin-bot · 2024-12-06T09:37:02Z

@luacmartins, @MitchExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

Pujan92 · 2024-12-06T14:32:40Z

@luacmartins This doesn't seem to be from #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.

MitchExpensify · 2024-12-07T16:37:55Z

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

luacmartins · 2024-12-09T22:11:58Z

Correct, it should open the Admin room since that's what's highlighted

nyomanjyotisa · 2024-12-10T00:18:24Z

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

luacmartins · 2024-12-10T19:25:27Z

I think the behavior should be consistent on all platforms, so we should highlight the first item again once the search is altered

melvin-bot · 2024-12-11T23:37:30Z

Job added to Upwork: https://www.upwork.com/jobs/~021866990715543185912

melvin-bot · 2024-12-11T23:37:34Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ikevin127 (External)

ikevin127 · 2024-12-14T00:54:21Z

@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 ?

melvin-bot · 2024-12-16T09:03:16Z

@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!

MitchExpensify · 2024-12-16T13:41:49Z

Reminder on this @luacmartins

luacmartins · 2024-12-16T21:18:05Z

As pointed out here, I don't think this issue is a regression, so we're open to proposals

melvin-bot · 2024-12-17T09:24:01Z

@luacmartins, @MitchExpensify, @ikevin127 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2024-12-18T16:00:22Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

luacmartins · 2024-12-18T16:56:40Z

Still looking for proposals

ikevin127 · 2024-12-18T19:00:36Z

Still looking for proposals

nyomanjyotisa · 2024-12-20T08:25:07Z

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

App/src/components/Search/SearchRouter/SearchRouter.tsx

Lines 188 to 203 in 207622d

    
           const submitSearch = useCallback( 
        
               (queryString: SearchQueryString) => { 
        
                   const queryWithSubstitutions = getQueryWithSubstitutions(queryString, autocompleteSubstitutions); 
        
                   const updatedQuery = SearchQueryUtils.getQueryWithUpdatedValues(queryWithSubstitutions, activeWorkspaceID); 
        
                   if (!updatedQuery) { 
        
                       return; 
        
                   } 
        
                   onRouterClose(); 
        
                   Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: updatedQuery})); 
        
                   setTextInputValue(''); 
        
                   setAutocompleteQueryValue(''); 
        
               }, 
        
               [autocompleteSubstitutions, onRouterClose, setTextInputValue, activeWorkspaceID], 
        
           );

And then will be called on this onSubmit

App/src/components/Search/SearchRouter/SearchRouter.tsx

Lines 281 to 287 in 207622d

    
           <SearchRouterInput 
        
               value={textInputValue} 
        
               isFullWidth={shouldUseNarrowLayout} 
        
               onSearchQueryChange={onSearchQueryChange} 
        
               onSubmit={() => { 
        
                   submitSearch(textInputValue); 
        
               }}

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}
                        ...

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

iOS-Native.mp4

huult · 2024-12-21T14:00:42Z

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).

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 746 to 752 in 4cd759c

    
           useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, selectFocusedOption, { 
        
               captureOnInputs: true, 
        
               shouldBubble: !flattenedSections.allOptions.at(focusedIndex) || focusedIndex === -1, 
        
               shouldStopPropagation, 
        
               shouldPreventDefault, 
        
               isActive: !disableKeyboardShortcuts && !disableEnterShortcut && isFocused, 
        
           });

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 399 to 407 in 4cd759c

    
           const selectFocusedOption = () => { 
        
               const focusedOption = focusedIndex !== -1 ? flattenedSections.allOptions.at(focusedIndex) : undefined; 
        
               if (!focusedOption || (focusedOption.isDisabled && !focusedOption.isSelected)) { 
        
                   return; 
        
               } 
        
               selectRow(focusedOption); 
        
           };

And the selected item, when Enter is pressed, will be handled for search or navigation.

App/src/components/Search/SearchRouter/SearchRouter.tsx

Line 301 in eb896c6

onListItemPress={onListItemPress}

App/src/components/Search/SearchRouter/SearchRouter.tsx

Lines 239 to 243 in eb896c6

    
           if (item?.reportID) { 
        
               Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(item?.reportID)); 
        
           } else if ('login' in item) { 
        
               ReportUserActions.navigateToAndOpenReport(item.login ? [item.login] : [], false); 
        
           }

In this case, we encountered an issue (mweb) where, if no item in the list is highlighted, pressing Enter does not trigger any action.

Screen.Recording.2024-12-21.at.16.25.56.mov

For native apps (Android/iOS), we detect the Enter key press using onSubmitEditing.

App/src/components/Search/SearchRouter/SearchRouterInput.tsx

Line 109 in 9675b9c

onSubmitEditing={onSubmit}

App/src/components/Search/SearchRouter/SearchRouter.tsx

Lines 285 to 287 in eb896c6

    
           onSubmit={() => { 
        
               submitSearch(textInputValue); 
        
           }}

When we press Enter, it calls submitSearch without handling the case for navigation to the highlighted item

App/src/components/Search/SearchRouter/SearchRouter.tsx

Lines 188 to 203 in eb896c6

    
           const submitSearch = useCallback( 
        
               (queryString: SearchQueryString) => { 
        
                   const queryWithSubstitutions = getQueryWithSubstitutions(queryString, autocompleteSubstitutions); 
        
                   const updatedQuery = SearchQueryUtils.getQueryWithUpdatedValues(queryWithSubstitutions, activeWorkspaceID); 
        
                   if (!updatedQuery) { 
        
                       return; 
        
                   } 
        
                   onRouterClose(); 
        
                   Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: updatedQuery})); 
        
                   setTextInputValue(''); 
        
                   setAutocompleteQueryValue(''); 
        
               }, 
        
               [autocompleteSubstitutions, onRouterClose, setTextInputValue, activeWorkspaceID], 
        
           );

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

App/src/components/SelectionList/BaseSelectionList.tsx

Line 398 in 4cd759c

  const getFocusedItem = useCallback(() => {
      const focusedOption = focusedIndex !== -1 ? flattenedSections.allOptions.at(focusedIndex) : undefined;

      if (!focusedOption || (focusedOption.isDisabled && !focusedOption.isSelected)) {
          return;
      }

      return focusedOption;
  }, [focusedIndex, flattenedSections.allOptions]);

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 737 to 743 in 4cd759c

    
           useImperativeHandle(ref, () => ({scrollAndHighlightItem, clearInputAfterSelect, updateAndScrollToFocusedIndex, updateExternalTextInputFocus, scrollToIndex}), [ 
        
               scrollAndHighlightItem, 
        
               clearInputAfterSelect, 
        
               updateAndScrollToFocusedIndex, 
        
               updateExternalTextInputFocus, 
        
               scrollToIndex, 
        
           ]);

Update to

    useImperativeHandle(ref, () => ({scrollAndHighlightItem, clearInputAfterSelect, updateAndScrollToFocusedIndex, updateExternalTextInputFocus, scrollToIndex, getFocusedItem}), [
        scrollAndHighlightItem,
        clearInputAfterSelect,
        updateAndScrollToFocusedIndex,
        updateExternalTextInputFocus,
        scrollToIndex,
        getFocusedItem,
    ]);

We will update onSubmit

App/src/components/Search/SearchRouter/SearchRouter.tsx

Lines 285 to 287 in eb896c6

    
           onSubmit={() => { 
        
               submitSearch(textInputValue); 
        
           }}

  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:

App/src/components/Search/SearchRouter/SearchRouterInput.tsx

Line 93 in 9675b9c

<TextInput

    <TextInput
        ...
        onKeyPress={(event) => {
          const isWeb = getPlatform() === CONST.PLATFORM.WEB;
          if (!isWeb) {
              return;
          }
          if (event.nativeEvent.key !== 'Enter') {
              return;
          }
          onSubmit();
        }}
    />

POC

Screen.Recording.2024-12-21.at.20.59.15.mp4

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.

MitchExpensify · 2024-12-23T13:51:57Z

@ikevin127 Proposals are ready for your review

ikevin127 · 2024-12-23T19:35:44Z

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
ios-noHighlight.mp4	ios-mweb-noHighlight.mp4

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!

shawnborton · 2024-12-24T15:37:49Z

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.

melvin-bot · 2024-12-25T16:00:34Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot · 2024-12-27T09:15:49Z

@luacmartins, @MitchExpensify, @ikevin127 Whoops! This issue is 2 days overdue. Let's get this updated quick!

ikevin127 · 2024-12-27T23:14:19Z

We're still awaiting some more input on #53401 (comment) regarding final decision.

IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Dec 2, 2024

melvin-bot bot assigned luacmartins and MitchExpensify Dec 2, 2024

melvin-bot bot added the Daily KSv2 label Dec 2, 2024

github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Dec 2, 2024

github-actions bot mentioned this issue Dec 2, 2024

Deploy Checklist: New Expensify 2024-11-30 #53334

Closed

41 tasks

luacmartins added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Dec 2, 2024

melvin-bot bot mentioned this issue Dec 2, 2024

[HOLD for payment 2024-12-09] [$125] Make sure the first result in most recents is highlighted when user uses CMD+K #51894

Closed

melvin-bot bot added the Overdue label Dec 5, 2024

melvin-bot bot removed the Overdue label Dec 7, 2024

luacmartins added the External Added to denote the issue can be worked on by a contributor label Dec 11, 2024

melvin-bot bot changed the title ~~Android/iOS- Highlighted search result go to search page rather that the highlighted detail page~~ [$250] Android/iOS- Highlighted search result go to search page rather that the highlighted detail page Dec 11, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 11, 2024

melvin-bot bot assigned ikevin127 Dec 11, 2024

melvin-bot bot added the Overdue label Dec 16, 2024

melvin-bot bot removed the Overdue label Dec 18, 2024

melvin-bot bot added the Overdue label Dec 21, 2024

This comment was marked as outdated.

Sign in to view

melvin-bot bot removed the Overdue label Dec 23, 2024

melvin-bot bot added the Overdue label Dec 27, 2024

melvin-bot bot removed the Overdue label Dec 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$250] Android/iOS- Highlighted search result go to search page rather that the highlighted detail page #53401

[$250] Android/iOS- Highlighted search result go to search page rather that the highlighted detail page #53401

IuliiaHerets commented Dec 2, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

luacmartins commented Dec 2, 2024

melvin-bot bot commented Dec 6, 2024

Pujan92 commented Dec 6, 2024

MitchExpensify commented Dec 7, 2024

luacmartins commented Dec 9, 2024

nyomanjyotisa commented Dec 10, 2024

luacmartins commented Dec 10, 2024

melvin-bot bot commented Dec 11, 2024

melvin-bot bot commented Dec 11, 2024

ikevin127 commented Dec 14, 2024

melvin-bot bot commented Dec 16, 2024

MitchExpensify commented Dec 16, 2024

luacmartins commented Dec 16, 2024

melvin-bot bot commented Dec 17, 2024

melvin-bot bot commented Dec 18, 2024

luacmartins commented Dec 18, 2024

ikevin127 commented Dec 18, 2024

nyomanjyotisa commented Dec 20, 2024

huult commented Dec 21, 2024 •

edited by github-actions bot

Loading

MitchExpensify commented Dec 23, 2024

This comment was marked as outdated.

ikevin127 commented Dec 23, 2024

shawnborton commented Dec 24, 2024

melvin-bot bot commented Dec 25, 2024

melvin-bot bot commented Dec 27, 2024

ikevin127 commented Dec 27, 2024

[$250] Android/iOS- Highlighted search result go to search page rather that the highlighted detail page #53401

[$250] Android/iOS- Highlighted search result go to search page rather that the highlighted detail page #53401

Comments

IuliiaHerets commented Dec 2, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 2, 2024

melvin-bot bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

luacmartins commented Dec 2, 2024

melvin-bot bot commented Dec 6, 2024

Pujan92 commented Dec 6, 2024

MitchExpensify commented Dec 7, 2024

luacmartins commented Dec 9, 2024

nyomanjyotisa commented Dec 10, 2024

luacmartins commented Dec 10, 2024

melvin-bot bot commented Dec 11, 2024

melvin-bot bot commented Dec 11, 2024

ikevin127 commented Dec 14, 2024

melvin-bot bot commented Dec 16, 2024

MitchExpensify commented Dec 16, 2024

luacmartins commented Dec 16, 2024

melvin-bot bot commented Dec 17, 2024

melvin-bot bot commented Dec 18, 2024

luacmartins commented Dec 18, 2024

ikevin127 commented Dec 18, 2024

nyomanjyotisa commented Dec 20, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

huult commented Dec 21, 2024 • edited by github-actions bot Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

Test branch

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

MitchExpensify commented Dec 23, 2024

This comment was marked as outdated.

ikevin127 commented Dec 23, 2024

shawnborton commented Dec 24, 2024

melvin-bot bot commented Dec 25, 2024

melvin-bot bot commented Dec 27, 2024

ikevin127 commented Dec 27, 2024

IuliiaHerets commented Dec 2, 2024 •

edited by melvin-bot bot

Loading

huult commented Dec 21, 2024 •

edited by github-actions bot

Loading