Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.52k stars 2.88k forks source link

[HOLD for payment 2024-09-23] [Search v2.3] - List scrolls back to the top after selecting saved search at the bottom of list #49218

Open IuliiaHerets opened 1 month ago

IuliiaHerets commented 1 month ago

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.0.35-0 Reproducible in staging?: Y Reproducible in production?: N/A - new feature, doesn't exist in prod Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  1. Go to staging.new.expensify.com
  2. Go to Search.
  3. Scroll the saved search list to the bottom.
  4. Note that the Saved title is also scrolled and hidden.
  5. Select a search from the bottom of the list.
  6. Note that the list scrolls up and the selected search is hidden.

Expected Result:

In Step 4, the Saved title should remain on top when the list is scrolled down. In Step 6, the list will not scroll up after selecting the search at the bottom of the list, or the selected search will move to the top.

Actual Result:

In Step 4, the Saved title is also scrolled and hidden. In Step 6, the list scrolls up and the selected search is hidden.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/b735f25e-a562-4cba-b558-682b0b7c26cf

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @anmurali
melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @anmurali (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[bot] commented 1 month ago

: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:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
dominictb commented 1 month ago

Proposal

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

What is the root cause of that problem?

  1. The list scrolls up and the selected search is hidden
  1. The Saved title is also scrolled

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

  1. The list scrolls up and the selected search is hidden
    const route = useRoute();
    const scrollViewRef = useRef<RNScrollView>(null);
    const {saveScrollOffset, getScrollOffset} = useContext(ScrollOffsetContext);
    const onScroll = useCallback<NonNullable<ScrollViewProps['onScroll']>>(
        (e) => {
            // If the layout measurement is 0, it means the flashlist is not displayed but the onScroll may be triggered with offset value 0.
            // We should ignore this case.
            if (e.nativeEvent.layoutMeasurement.height === 0) {
                return;
            }
            saveScrollOffset(route, e.nativeEvent.contentOffset.y);
        },
        [route, saveScrollOffset],
    );
    useLayoutEffect(() => {
        const scrollOffset = getScrollOffset(route);
        if (!scrollOffset || !scrollViewRef.current) {
            return;
        }
        scrollViewRef.current.scrollTo({y: scrollOffset, animated: false});
    }, [getScrollOffset, route]);
                    <ScrollView
                        onScroll={onScroll}
                        ref={scrollViewRef}
                    >
  1. The Saved title is also scrolled

and add it to:

https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/pages/Search/SearchTypeMenu.tsx#L227

The detail style can be add later.

What alternative solutions did you explore? (Optional)

luacmartins commented 1 month ago

@lakchote will address this as a follow up to his PR

luacmartins commented 1 month ago

@dominictb are you available to work on a fix?

dominictb commented 1 month ago

@luacmartins Yes. I can work on it

luacmartins commented 1 month ago

@dominictb how's the PR coming along?

dominictb commented 1 month ago

@luacmartins I drafted the PR. I am testing a few test cases.

lakchote commented 1 month ago

@dominictb how is it looking? Do you think you'd be able to raise a PR soon? Thank you!

dominictb commented 1 month ago

@lakchote PR is ready for review.

lakchote commented 1 month ago

@lakchote PR is ready for review.

Thank you, I've assigned a C+ available for review.

lakchote commented 1 month ago

Fix has been deployed to staging.

https://github.com/user-attachments/assets/b360b81f-bbfd-45b4-8607-32aa430deb01

Demoting from blocker.

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

If no regressions arise, payment will be issued on 2024-09-23. :confetti_ball:

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

melvin-bot[bot] commented 1 month ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

melvin-bot[bot] commented 1 month ago

Payment Summary

[Upwork Job]()

BugZero Checklist (@anmurali)

melvin-bot[bot] commented 1 month ago

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

dominictb commented 1 month ago

@anmurali Ready for payment.

lakchote commented 1 month ago

Not overdue. Waiting for payment.

melvin-bot[bot] commented 1 month ago

@anmurali, @lakchote, @luacmartins, @dominictb 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

luacmartins commented 1 month ago

Bump for payment @anmurali

melvin-bot[bot] commented 1 month ago

@anmurali, @lakchote, @luacmartins, @dominictb 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

melvin-bot[bot] commented 1 month ago

@anmurali, @lakchote, @luacmartins, @dominictb 10 days overdue. Is anyone even seeing these? Hello?

anmurali commented 1 month ago

@dominictb - I need your Upwork profile @luacmartins - Do we need to pay $250?

anmurali commented 4 weeks ago

@luacmartins @dominictb bump on ^^

luacmartins commented 4 weeks ago

Yes, that's $250 for @dominictb and @rushatgabhane for writing the PR and reviewing, respectively

melvin-bot[bot] commented 3 weeks ago

@anmurali, @lakchote, @luacmartins, @rushatgabhane, @dominictb Huh... This is 4 days overdue. Who can take care of this?

garrettmknight commented 3 weeks ago
anmurali commented 3 weeks ago

Waiting on @dominictb

melvin-bot[bot] commented 2 weeks ago

@anmurali, @lakchote, @luacmartins, @rushatgabhane, @dominictb Huh... This is 4 days overdue. Who can take care of this?

luacmartins commented 2 weeks ago

Waiting on payment

melvin-bot[bot] commented 2 weeks ago

@anmurali, @lakchote, @luacmartins, @rushatgabhane, @dominictb Still overdue 6 days?! Let's take care of this!

lakchote commented 1 week ago

@dominictb could you please give us your Upwork profile in order to pay you?

melvin-bot[bot] commented 1 week ago

@anmurali, @lakchote, @luacmartins, @rushatgabhane, @dominictb 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

lakchote commented 1 week ago

bump @dominictb for your Upwork profile.

dominictb commented 1 week ago

@lakchote Sorry I missed this, mine is https://www.upwork.com/freelancers/~01f70bed1934fd35d5

melvin-bot[bot] commented 1 week ago

@anmurali, @lakchote, @luacmartins, @rushatgabhane, @dominictb 12 days overdue. Walking. Toward. The. Light...

anmurali commented 1 week ago

@dominictb - offer is here

dominictb commented 1 week ago

Accepted, thank you @anmurali

lakchote commented 2 days ago

Not overdue. Should we close this?

luacmartins commented 2 days ago

Seems like we're all set. Gonna close the issue, please reopen if needed.

melvin-bot[bot] commented 2 days ago

@anmurali @lakchote @luacmartins Be sure to fill out the Contact List!

dominictb commented 2 days ago

@luacmartins @lakchote My contract was not paid by @anmurali yet, please help to reopen

luacmartins commented 2 days ago

Sorry, I thought that had been handled based on this comment. My bad 😅