[PAID] [HOLD for payment 2024-11-29] [$250] mWeb -Finder -User not scrolled to top of chats list when scrolling down and deleting search

[lanitochka17]

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: 9.0.47-1
Reproducible in staging?: Y
Reproducible in production?: 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/5068088&group_by=cases:section_id&group_order=asc&group_id=229066
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Open the staging.new.expensify.com website
2. Log in to an account with a large list of chats
3. Tap on the search icon on the top right corner
4. Write a few letters to trigger some results
5. Scroll down to the middle of the results
6. Select all the content in the search input and delete it
7. Verify the chats list is updated and the user is navigated to the top of it

Expected Result:

After deleting the content in search input, the chats list should be updated and the user should be navigated to the top of it

Actual Result:

When writing a few letters to trigger a search, scrolling down to the middle of the results, selecting all the content on search input and deleting it, the chats list updated but the user is not redirected to the top. The list stays in the middle

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6630312_1728534947618.Finder.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021846215092993792827
• Upwork Job ID: 1846215092993792827
• Last Price Increase: 2024-11-12
• Automatic offers:
• allgandalf | Reviewer | 104904927
• wildan-m | Contributor | 104904928

Issue Owner

Current Issue Owner: @CortneyOfstad

[melvin-bot]

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

[lanitochka17]

@CortneyOfstad FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[daledah]

Edited by proposal-police: This proposal was edited at 2024-10-30 14:39:15 UTC.

Proposal

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

When writing a few letters to trigger a search, scrolling down to the middle of the results, selecting all the content on search input and deleting it, the chats list updated but the user is not redirected to the top. The list stays in the middle

What is the root cause of that problem?

When the newUserQuery becomes empty, we have the logic to scroll to -1

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

Line 308 in de42f2e

listRef.current?.updateAndScrollToFocusedIndex(-1);

But we prevent to scroll if the index is -1

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 254 to 256 in de42f2e

	if (!listRef.current || !item || index === -1) {
	return;
	}

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

We want to prevent scrolling if index is -1 because at the beginning the search term is always -1 then we don't want to do any scroll (If not the list can be blink)

1. We should remove these redundant code here since we already handle it in BaseSelectionList

2. Add textInputValue that is missed here

3. Update scrollToIndex function to add new param isFirstRender

 const scrollToIndex = useCallback(
        (index: number, animated = true, isFirstRender = false) => {
            const item = flattenedSections.allOptions.at(index);

            if (!listRef.current || !item || (isFirstRender && index === -1)) {
                return;
            }

            const itemIndex = index === -1 ? 0 : item.index ?? -1;
            const sectionIndex = index === -1 ? 0 : item.sectionIndex ?? -1;

            listRef.current.scrollToLocation({sectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight});
        },

        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
        [flattenedSections.allOptions],
    );

Then update

App/src/components/SelectionList/BaseSelectionList.tsx

Line 507 in 4c90d62

scrollToIndex(focusedIndex, false);

to

scrollToIndex(focusedIndex, false, true);

What alternative solutions did you explore? (Optional)

We can update this logic to

listRef.current?.updateAndScrollToFocusedIndex(0);

so the first item will be focused after deleting search text

[CortneyOfstad]

Didn't realize this was Android — so having Stevie do some testing for me!

[CortneyOfstad]

Confirmed that this was reproducible so getting eyes on it! S/O to @slafortune for her help!

[melvin-bot]

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

[melvin-bot]

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

[huult]

Proposal

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

User not scrolled to top of chats list when scrolling down and deleting search

What is the root cause of that problem?

After deleting the search, scrolling to the top is not possible because newSelectedIndex returns -1 when textInputValue === '' (which occurs after the previous search is cleared).

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 568 to 574 in 7fca65b

	const newSelectedIndex =
	textInputValue === '' || (flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length) ? -1 : 0;
	// reseting the currrent page to 1 when the user types something
	setCurrentPage(1);
	updateAndScrollToFocusedIndex(newSelectedIndex);

The function updateAndScrollToFocusedIndex receives this -1 and passes it to scrollToIndex. Inside scrollToIndex, there's a condition that checks if the index is -1. If true, it returns without performing the scroll, causing the issue

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 252 to 254 in 7fca65b

	if (!listRef.current || !item || index === -1) {
	return;
	}

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

To resolve this issue, we need to check if textInputValue is empty after the search text is removed. If it is, we need to return to index === 0 to enable scrolling to the top.

// .src/components/SelectionList/BaseSelectionList.tsx#L568
- const newSelectedIndex =
-            textInputValue === '' || (flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length) ? -1 : 0;
+ const newSelectedIndex =
+            (textInputValue === '' && !prevTextInputValue) || (flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length) ? -1 : 0;

POC

Screen.Recording.2024-10-16.at.01.34.42.mov

I've tested the previous issue, and this condition still works well after the update

POC

Screen.Recording.2024-10-16.at.01.36.45.mp4

[melvin-bot]

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

[CortneyOfstad]

Hey @stephanieelliott! I am heading OoO shortly (until 10/23) so reassigning to keep an eye on this! Thanks!

[daledah]

@allgandalf What do you think about my proposal? Thank you

[melvin-bot]

@CortneyOfstad, @stephanieelliott, @allgandalf Huh... This is 4 days overdue. Who can take care of this?

[daledah]

@allgandalf Can you take a look at my proposal?

[Kicu]

FYI @allgandalf @rlinoz I see the video on this issue is presenting old ChatFinder functionality, but the code of ChatFinder is completely removed from App as of a few weeks. Probably when this issue was created, then ChatFinder was still in the code, but in end the final solution landed in SearchRouter (which had a similar problem).

As I'm refactoring SearchRouter right now (need it to behave a bit different) then I will simplify this code because the ref is completely unnecessary.

The previous value of query is right there in textInputValue, so a simple
if (updatedUserQuery || textInputValue.length > 0) {
is enough.

Just FYI - I have no idea if the isEmptyObject inside BaseSelectionList is needed or not, but I won't be touching it.
Here's the PR where SearchRouter is changing: #52568

CC @luacmartins just so you know

[luacmartins]

Thanks @Kicu! I see that the PR for this issue is already merged. I think we can make changes to the solution if needed though.

[Kicu]

solution changed here: https://github.com/Expensify/App/pull/52568/files#diff-f8e8e46965e7e099296f3ec791faf7d61b208fb68dfff52e20e8d98483479a89R225

I included an android video showing that the bug is still fixed on my code

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.65-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: User not scrolled to top of chats list when scrolling down and deleting search #52606

If no regressions arise, payment will be issued on 2024-11-29. 🎊

For reference, here are some details about the assignees on this issue:

• @wildan-m requires payment automatic offer (Contributor)
• @allgandalf requires payment automatic offer (Reviewer)

[melvin-bot]

@allgandalf @CortneyOfstad @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[CortneyOfstad]

@allgandalf can you please complete the checklist at your earliest convenience?

[CortneyOfstad]

Please note that I am going to be OoO on 11/29 due to Thanksgiving in the US, but I will get this paid as soon as I am back on Monday. Most of the team will be out, so I apologize for the delay!

[allgandalf]

No worries @CortneyOfstad , Hope you enjoy your time off 😄

[allgandalf]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: https://github.com/Expensify/App/pull/44222/files#r1861582359

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue: https://github.com/Expensify/Expensify/issues/449517

Regression Test Proposal

Precondition:

• Account with large list of chats (For the scroll behaviour to exist)

Test:

1. Open App > Tap on the search icon on the top right corner
2. Write a few letters to trigger some results
3. Scroll down to the middle of the results
4. Select all the content in the search input and delete it

Verify the chats list is updated and the user is navigated to the top of it

Do we agree 👍 or 👎

[melvin-bot]

@rlinoz, @CortneyOfstad, @wildan-m, @allgandalf Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[CortneyOfstad]

Payment Summary

@wildan-m — paid $250 via Upwork
@allgandalf — paid $250 via Upwork

Regression Test

https://github.com/Expensify/Expensify/issues/449517