[HOLD App#11795] [$1000] Workspace - Invite new members list always scrolls to top after selecting 4 or more

[kavimuru]

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

---

Action Performed:

1. Click on avatar
2. Click on Workspace
3. Select Workspace
4. Click Manage Members
5. Click on Invite Members
6. Select at least 8 Members

Expected Result:

User should be able to view the most recent selected member

Actual Result:

List scrolls to top with first selected member

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.31-5
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Recording.999.mp4

Screen.Recording.2022-11-24.at.5.13.20.PM.mov

Screen.Recording.2022-11-24.at.5.12.44.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @syedsaroshfarrukhdot
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1669292832144069

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0174a96dd459e1553d
• Upwork Job ID: 1598666700904955904
• Last Price Increase: 2022-12-08

[melvin-bot]

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

[hungvu193]

I think this a feature, not a bug.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 110 to 114 in 39b2f99

	// If we just toggled an option on a multi-selection page, scroll to top
	if (this.props.selectedOptions.length !== prevProps.selectedOptions.length) {
	this.scrollToIndex(0);
	return;
	}

App/src/components/OptionsSelector/BaseOptionsSelector.js

Line 120 in 39b2f99

this.scrollToIndex(this.state.focusedIndex);

In case we don't want it scroll to top (which's kind of annoying with me too) we can comment or delete this piece of code.

-            // If we just toggled an option on a multi-selection page, scroll to top
-           // if (this.props.selectedOptions.length !== prevProps.selectedOptions.length) {
-            //     this.scrollToIndex(0);
-            //     return;
-           // }
-           this.scrollToIndex(this.state.focusedIndex);

[melvin-bot]

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

[melvin-bot]

@davidcardoza Huh... This is 4 days overdue. Who can take care of this?

[davidcardoza]

Hmm...This feels like a bug, but I want a second opinion before moving forward with creating a job. Applying the engineering label.

[melvin-bot]

Triggered auto assignment to @cristipaval (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[davidcardoza]

@cristipaval thoughts?

[cristipaval]

Hey @davidcardoza ! Yes, this is reproducible and I would also consider it a bug.

[davidcardoza]

Job posted - https://www.upwork.com/jobs/~01770333bb615437f2 waiting for proposals

[syedsaroshfarrukhdot]

Proposal:-

We can fix this issue by pushing most recently selected user to top

App/src/pages/workspace/WorkspaceInvitePage.js

Lines 193 to 196 in 39b2f99

	} else {
	newSelectedOptions = [...prevState.selectedOptions, option];
	}

We could change it to

     newSelectedOptions = [option, ...prevState.selectedOptions];

After Fix:-

Screen.Recording.2022-12-02.at.4.13.55.AM.mov

Screen.Recording.2022-12-02.at.4.30.18.AM.mov

[cristipaval]

@davidcardoza How did you create the upwork job? I think you should have applied the External label to this issue and everything would have been created automatically along with some others like C+ assignment. Or actually I think I should have done that after reviewing the issue. Sorry for missing it.

[melvin-bot]

Current assignee @davidcardoza is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Current assignee @cristipaval is eligible for the External assigner, not assigning anyone new.

[cristipaval]

Ok, I removed the Upwork job created by you @davidcardoza and applied External label. I had to remove it to avoid a duplicate job posting on Upwork.

[parasharrajat]

This is being discussed on slack. I will chime in later. There are some instructions but I need to check what things will be affected by the expected changes. If there are things that also needs to be fixed with the new expected changes.

[cristipaval]

We're still discussing on Slack. I hope we get a conclusion asap.

[mountiny]

After discussing, I am putting this issue on hold for #11795, there we will redesign the option lists so the UX is better and works fine across platforms. Better to address these issues which are not blockers in structured manner.

[cristipaval]

Thanks @mountiny ! I'll close the upwork job for now.

[syedsaroshfarrukhdot]

Am I eligible for reporting bonus ?

[davidcardoza]

The issue was put on hold. Reporting bonuses are paid out only after an issue/bug has been resolved.

[cristipaval]

@JmillsExpensify , replying to your comment here.

We discussed about this issue here. Please have a look when you've got a minute.

[JmillsExpensify]

Nice, thanks. I commented in the related Slack thread. Re-posing below.

Looks like we never really decided what to do here. I personally like the sticky header and feel that it’s better than the auto-scroll up. That said, I think this is squarely new feature territory and I’m not sure this is really critical right now. I think I’d opt to keep punting this.

[davidcardoza]

I concur with the sentiment @JmillsExpensify, the decision regarding the implementation of a sticky header versus auto-scroll functionality has yet to be determined and is probably deserving of a broader conversation to implement as a new feature.

@cristipaval Is there any reason to keep this issue on hold?

[cristipaval]

We can disable the auto scroll functionality as a solution for this issue or just simply close this issue and disable the auto-scroll when we start a broader conversation about the sticky header.

[davidcardoza]

I vote the latter of the two option, let's punt until we have begun speccing out the sticky header.

[cristipaval]

I'm also leaning towards closing this one and disable the auto-scroll when we start working on the sticky header.

[cristipaval]

Closing for now, @JmillsExpensify feel free to reopen it if you don't agree.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.