[HOLD for payment 2024-02-15] [$500] Room-After 2nd WS created, from WS selection moves up to public in visibility page

[izarutskaya]

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: 1.4.35
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: applause-Internal Team
Slack conversation:

Action Performed:

1. Go to https://staging.new.expensify.com/

2. Login into new account

3. Tap profile icon --- Workspaces

4. Tap new workspace

5. Navigate back to LHN

6. Tap fab--- start chat--- room

7. Tap visibility

8. Note, with one workspace created, opening create room visibility section shows, public selected and WS option with tick mark

9. Navigate to LHN

10. Tap profile icon---Workspaces

11. Tap new Workspace and create second workspace

12. Navigate to LHN

13. Tap fab--- start chat--- room

14. Tap visibility

15. Note after creating second Workspace, from WS option selection moves up to public

Expected Result:

With any number of workspaces created, the visibility page behaviour must be same.

Actual Result:

Login as new user, with 1WS created, opening create room visibility section shows, public selected and WS option with tick mark. Navigating back and create 2nd WS, in this case, from WS option selection moves up to public is shown.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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

Bug6364442_1706851443301.q.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fb13ef2922bb85d7
• Upwork Job ID: 1753360359996166144
• Last Price Increase: 2024-02-02
• Automatic offers:
• Ollyws | Reviewer | 28143074
• bernhardoj | Contributor | 28143075

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[shahinyan11]

@izarutskaya The video is not вуыскшиуе Actual Result: . I don't see the selected option change after adding the second WS in the video

[bernhardoj]

Proposal

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

Opening the visibility selection page of the new room shows the first option (Public) as highlighted even though the selected is the last (Workspace) option.

What is the root cause of that problem?

The highlight moves to the top because whenever the section object is changed, it will scroll and highlight the first option.

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 356 to 361 in cf8279f

	// set the focus on the first item when the sections list is changed
	if (sections.length > 0) {
	updateAndScrollToFocusedIndex(0);
	}
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [sections]);

In our case, the section data itself never changed, but we pass an inline array value to the props, so it's recreated every time the ValueSelectorModal is re-rendered.

App/src/components/ValuePicker/ValueSelectorModal.js

Lines 71 to 72 in cf8279f

	<SelectionList
	sections={[{data: sectionsData}]}

And one of the cases where it re-renders is when we open it, the modal visible state becomes true.

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

Memoize the section so it only changes when it really changes.

App/src/components/ValuePicker/ValueSelectorModal.js

Lines 45 to 50 in cf8279f

	const [sectionsData, setSectionsData] = useState([]);
	useEffect(() => {
	const itemsData = _.map(items, (item) => ({value: item.value, alternateText: item.description, keyForList: item.value, text: item.label, isSelected: item === selectedItem}));
	setSectionsData(itemsData);
	}, [items, selectedItem]);

So, instead of using the useEffect, we can do it like this:

const sections = useMemo(() => {
    const itemsData = _.map(items, (item) => ({value: item.value, alternateText: item.description, keyForList: item.value, text: item.label, isSelected: item === selectedItem}));
    return [{data: itemsData}];
}, [items, selectedItem]);

sections={sections}

[Ollyws]

Will get to this today.

[Ollyws]

@bernhardoj's proposal LGTM.
🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @amyevans, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[amyevans]

This is a pretty minor bug, but the fix looks straightforward so we may as well get it taken care of. Assigning @bernhardoj

[melvin-bot]

📣 @Ollyws 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[bernhardoj]

PR is ready

cc: @Ollyws

[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 1.4.38-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix highlight moves to top when opening a workspace selector page #35870

If no regressions arise, payment will be issued on 2024-02-15. 🎊

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

• @Ollyws requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@Ollyws] The PR that introduced the bug has been identified. Link to the PR:
• [@Ollyws] 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:
• [@Ollyws] A discussion in #expensify-bugs 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:
• [@Ollyws] Determine if we should create a regression test for this bug.
• [@Ollyws] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/372249

[Ollyws]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR:

#30958

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

#30958 (comment)

• A discussion in #expensify-bugs 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

• Determine if we should create a regression test for this bug.

Yes.

• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Press the FAB to open the start chat page
2. Switch to room tab
3. Open the room visibility page
4. Verify the highlight background is on the selected option, in this case the Workspace option

Do we agree 👍 or 👎

[melvin-bot]

@amyevans, @Ollyws, @muttmuure, @bernhardoj Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[muttmuure]

All paid up