Split - Participant list does not scroll to the top after selecting split participants

[kbecciv]

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: v1.4.43-2
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

1. Navigate to staging.new.expensify.com
2. Go to FAB > Request money > Manual.
3. Enter amount > Next.
4. Scroll down the list.
5. Click Split.

Expected Result:

The list will scroll up to the top each time a member is selected as split participant.

Actual Result:

The list does not scroll up to the top each time a member is selected as split participant.

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

Bug6385607_1708428523462.bandicam_2024-02-20_10-28-32-766.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[kbecciv]

@alexpensify 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.

[kbecciv]

We think that this bug might be related to #vip-split-p2p-chat-groups
CC @arielgreen

[Krishna2323]

Proposal

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

Split - Participant list does not scroll to the top after selecting split participants

What is the root cause of that problem?

In the useEffect which handles the scrolling, we return if the search term is not updated or selected options length is 0.

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 354 to 356 in a21cfbc

	if (prevTextInputValue === textInputValue || flattenedSections.allOptions.length === 0) {
	return;
	}

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

We need to also check for the length of previous selected options and the length of current selected options and if the current selected options is greater we should scroll instead of returning.

We will add this check, because we only need to scroll.

        if (flattenedSections.selectedOptions.length > prevSelectedOptions) {
            scrollToIndex(0, true);
        }

Or we can just check if there is any difference in flattenedSections.selectedOptions.length && prevSelectedOptions and if true, we will scroll.

Result

[aneequeahmad]

Proposal

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

Participant list doesn't scroll up to the top each time a member is selected as split participant.

What is the root cause of that problem?

This bug occured due to commit #ec73a6ca5d where this code was removed from useEffect which has the logic to scroll to top when section list changes.

 if (sections.length > 0) {
        updateAndScrollToFocusedIndex(0);
 }

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

We can add the logic back in useEffect to scroll to top when a new item is added in section.

Also the code to avoid changing focus if the textInputValue remains unchanged is added in useEffect which if needed can be added after the code i proposed so that when the item is selected it shouldn't return before doing anything

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.
N/A

[alexpensify]

I'm still catching up from being OOO and will test later this week.

[alexpensify]

Still on my testing list

[melvin-bot]

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

[alexpensify]

I didn't get a chance to test over the weekend, but it's on my list for tomorrow.

[alexpensify]

I ran out of time today and will have to test it next week.

Heads up, I will be offline until Tuesday, March 5, 2024. I will not be actively watching over this GitHub during that period. If anything urgent is needed here, please ask for help in the Open Source Slack Room-- thanks!

[melvin-bot]

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

[melvin-bot]

@alexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@alexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[alexpensify]

I'm catching up from being OOO and will test soon.

[alexpensify]

I confirmed this experience but will need to check with Design for feedback:

[alexpensify]

@Expensify/design - I need some feedback here. Should the list scroll to the top when you make a selection? I don't think it should and would make a weird jumping experience. You'd need to continue to scroll back down if you need to select more. I think we leave the list as is and the person should scroll up to confirm their selections. Thanks for the feedback, since I'm leaning toward closing this one!

[shawnborton]

Hmm I'm not too sure. I can totally see both sides of this... it's nice to scroll to the top to see the existing selections, but at the same time, scrolling might lose your position in a long list.

[alexpensify]

Yeah, I guess I'm thinking about a table of 6 versus a table of 2.

[dannymcclain]

I'm in favor of not automatically scrolling the list. I kinda think it would be more annoying than helpful. After each selection, you'll have to wait for the UI to automatically scroll to the top and then re-scroll back down to make further selections. Feels like over-solving to me.

[shawnborton]

Cool, I can get down with that! So maybe this is not a bug then and we can close. But of course, we can always wait until the third judge of the supreme design court weighs in.

[dubielzyk-expensify]

Agreed 100%. Don't think we should scroll up

[dannymcclain]

Alright! Let's go ahead and close this.

[shawnborton]

Done!

[alexpensify]

Thanks Design!