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

Web - Key Navigation -The search query is highlighted instead of the first result in the list #58057

Open
1 of 8 tasks
lanitochka17 opened this issue Mar 8, 2025 · 3 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Mar 8, 2025

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.1.10-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5651244&group_by=cases:section_id&group_order=asc&group_id=229066&group_by=cases:section_id&group_order=asc&group_id=229066
Email or phone of affected tester (no customers): arkam2504+30@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

Preconditions: A personal account is created and the user is logged in

  1. Navigate to Staging.new.expensify.com
  2. Open any conversation
  3. Click on the compose box to set the focus
  4. Press on CTRL + K to open the search modal
  5. Enter a username that contains numbers in the search text box

Expected Result:

The first result in the list should be highlighted

Actual Result:

The entered search query is highlighted instead of the first result in the list

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Bug6762738_1741255211764.bandicam_2025-03-06_11-57-58-488.mp4

Image

View all open jobs on GitHub

@lanitochka17 lanitochka17 added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Mar 8, 2025
Copy link

melvin-bot bot commented Mar 8, 2025

Triggered auto assignment to @abekkala (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.

@Shahidullah-Muffakir
Copy link
Contributor

Shahidullah-Muffakir commented Mar 8, 2025

Proposal

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

When user search something in the searchRouter, the searchQuery gets highlighted, but the expected result is that the first item from the list should be highlighted.

What is the root cause of that problem?

In the SearchRouter component, when text is entered in the search box, the code calls updateAndScrollToFocusedIndex(0). This is setting focus to index 0, which corresponds to the search query item itself, not the first actual search result. The search results list includes both the search query (as a "find item") and the actual results, with the search query appearing first in the list.

if (updatedUserQuery || textInputValue.length > 0) {
listRef.current?.updateAndScrollToFocusedIndex(0);
} else {
listRef.current?.updateAndScrollToFocusedIndex(-1);
}

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

We should update the onSearchQueryChange function in the SearchRouter component to focus on index 1 instead of index 0 when there's text in the input field. This will skip the search query item and directly focus on the first actual search result.

if (updatedUserQuery || textInputValue.length > 0) {
    listRef.current?.updateAndScrollToFocusedIndex(1);
} else {
    listRef.current?.updateAndScrollToFocusedIndex(-1);
}

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

Test that when the search modal is opened and no text is entered, no item is highlighted
Test that when text is entered in the search field, the first actual search result (not the search query) is highlighted

What alternative solutions did you explore? (Optional)

In case we want to highlight the searchQuery if there are not results in the suggested autocomplete list, then we can expose the flattenedSections.allOptions from the BaseSelectionList, and then in the SearchRouter if the flattenedSections.allOptions.length ===1 , then we can highlight the searchQuery, and if it is more then 1, then highlight the first result from the list.

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

@huult
Copy link
Contributor

huult commented Mar 10, 2025

Proposal

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

The search query is highlighted instead of the first result in the list

What is the root cause of that problem?

When we input a search value, regardless of whether the sessions have data or not, we still set the index to 0

listRef.current?.updateAndScrollToFocusedIndex(0);

Due to updateAndScrollToFocusedIndex always being set to 0, the first result in the list is never highlighted, causing this issue.

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

To resolve this issue, we simply compare autocompleteQueryValue with the session item title. If they match, we should update updateAndScrollToFocusedIndex is 1. The implementation is as follows:

  1. create new prop for SearchAutocompleteList component to listen onHighlightFirstItem event to trigger updateAndScrollToFocusedIndex to 1

update to

    <SearchAutocompleteList
          autocompleteQueryValue={autocompleteQueryValue || textInputValue}
          searchQueryItem={searchQueryItem}
          getAdditionalSections={getAdditionalSections}
          onListItemPress={onListItemPress}
          setTextQuery={setTextAndUpdateSelection}
          updateAutocompleteSubstitutions={updateAutocompleteSubstitutions}
          onHighlightFirstItem={() => listRef.current?.updateAndScrollToFocusedIndex(1)} // add this
          ref={listRef}
      />

shouldSubscribeToArrowKeyEvents,

add:

function SearchAutocompleteList(
    {
        autocompleteQueryValue,
        searchQueryItem,
        getAdditionalSections,
        onListItemPress,
        setTextQuery,
        updateAutocompleteSubstitutions,
        shouldSubscribeToArrowKeyEvents,
        onHighlightFirstItem, // add this
    }: SearchAutocompleteListProps,
    ref: ForwardedRef<SelectionListHandle>,
) {

2 add function to compare `autocompleteQueryValue`` with the session item title. we able write it to utils.

  function checkTextMatch(sections, targetText = '') {
        // Extract reference text safely and normalize case
        const referenceText = sections?.[1]?.data?.[0]?.text?.toLowerCase() || '';
        const normalizedTargetText = targetText.toLowerCase().trim();

        // Reject single-letter targetText (avoids "t" matching "t's")
        if (normalizedTargetText.length < 2) return false;

        // Case 2: Ensure targetText is a valid word or valid full prefix (not a partial cut-off)
        const regex = new RegExp(`\\b${normalizedTargetText}\\b`, 'i');

        // Ensure targetText is not ending in a non-word character (e.g., ' @ #)
        if (/[^a-zA-Z0-9]$/.test(normalizedTargetText)) return false;

        return regex.test(referenceText);
    }

    useEffect(() => {
        const isHighlightFirstItem = checkTextMatch(sections, autocompleteQueryValue);
        if (isHighlightFirstItem) {
            // ref.current?.updateAndScrollToFocusedIndex(1);
            onHighlightFirstItem();
        }
    }, [autocompleteQueryValue, onHighlightFirstItem, ref, sections]);

Test branch

Screen.Recording.2025-03-10.at.23.19.00.mp4

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

we create unit test for checkTextMatch function:

Expected Matching Behavior

Target Text Reference Text Expected Output
"t" "t's expenses" ❌ false
"t'" "t's expenses" ❌ false
"t's" "t's expenses" ✅ true
"t@s" "t's expenses" ❌ false
"t#s" "t's expenses" ❌ false
"expenses" "t's expenses" ✅ true
"expense" "t's expenses" ❌ false
"your team" "Invite your team" ✅ true
"vite" "Invite your team" ❌ false

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
None yet
Development

No branches or pull requests

4 participants