[HOLD for payment 2023-11-22] [$500] iOS - Members - When scrolling members WS list goes under the "background image"

[izarutskaya]

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

---

Issue found when executind PR: #30171
Version Number: 1.3.91-0
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:

rerequisites:
A WS must be created in the account

Steps:

1. Open New Expensify app
2. Log in with an expensifail account
3. Navigate to the WS settings
4. Navigate to Members
5. Invite 4 members
6. In the Members section, scroll through the list of members

Expected Result:

When scrolling through members, the WS list should not be missing or cut off

Actual Result:

When scrolling members WS list goes under the "background image"

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

Android: Native

Android: mWeb Chrome

iOS: Native

Bug6251545_1698313164061.RPReplay_Final1698262205.mp4

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c024409da152917d
• Upwork Job ID: 1717505060463128576
• Last Price Increase: 2023-11-02

Issue Owner

Current Issue Owner: @adelekennedy

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[bernhardoj]

Proposal

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

In the workspace members page, if have only a few members and scroll up, some of the bottom members list is hidden. This issue also happens on every page that uses SelectionList, for example, the Language and the Priority mode page.

What is the root cause of that problem?

This is because the list has a style of flex-grow: 0

App/src/components/SelectionList/BaseSelectionList.js

Line 437 in 414d591

style={[styles.flexGrow0]}

So, the height of the list is the same as the height of the list content.

The list content is only visible within the list view box. So when we scroll up, some of the items are outside of the list view box.

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

Remove the flex-grow: 0 style. This will allow the list to fill any empty space.

[esh-g]

Proposal

Please re-state the problem we are trying to solve

iOS - Members - When scrolling members WS list goes under the background content.

What is the root cause of this problem?

The root cause is that the height of the SectionList is limited to the content it has, therefore, when we scroll, it gets cut off at the edges.

Moreover, we allow the list on iOS to 'bounce' which means that we are able to scroll past the content which is not what we do for android and web.

What changes should be made to fix this?

We should add the prop bounces={false} to be more consistent with other platforms to the SectionList here:

App/src/components/SelectionList/BaseSelectionList.js

Line 418 in 414d591

<SectionList

This means that we are not able to scroll under the header part as well (which is the same for android).

What other alternatives did you explore?

To match the exact behaviour of android,
We can also selectively disable the bounce only when the contentHeight is less than the listHeight like this:

<SectionList
bounces={contentHeight > listHeight}
onContentSizeChange={(width, height) => setContentHeight(height)}
onLayout={({ nativeEvent: {layout: {height}} }) => setListHeight(height)}
...
/>

Here contentHeight and listHeight are defined as state.
I'm aware that currently we use the onLayout with scrollToFocusedIndexOnFirstRender method, but that can be easily modified to accomodate this logic.

Result
(Able to bounce for long list but not for short one [EXACTLY LIKE ANDROID])

Screen.Recording.2023-10-26.at.6.28.59.PM.mov

[hungvu193]

Proposal

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

When scrolling in members page, user list get cut off at the bottom.

What is the root cause of that problem?

We're using BaseSectionList for our WorkspaceMembersPage, and it has style styles.flexGrow0, it will make the list only take its available space, however in IOS the bounce is true by default, so the list can be scroll up and down in its space, which caused the issue.

App/src/components/SelectionList/BaseSelectionList.js

Line 437 in 9e50be4

style={[styles.flexGrow0]}

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

Change the style of SectionList to styles.flexGrow1 or styles.flex1 to make it fill all the available space.

What alternative solutions did you explore? (Optional)

N/A

[adelekennedy]

@0xmiroslav a few proposals to review above

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@adelekennedy, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[0xmiros]

@bernhardoj can you find out PR which added flexGrow0 and confirm no issue with that removal?
Also, test on all platforms, mainly mWeb

[bernhardoj]

It's added by this PR #27767 which is merged 23 days before this issue is created. The commit message that added it is

feat(selection-list): request money

I thought that it may be related to the money request participant list, but the money request participant list still uses OptionsSelector. The only component that is related to the money request page that uses SelectionList is the currency selection page and it still works fine.

I looked at the other usages of SelectionList on the web and native and nothing is broken so far.

Our old OptionsSelector/OptionsList doesn't have the flexGrow0 style and we have no issue with it, so I think we should be fine.

[0xmiros]

@bernhardoj's proposal looks good to me.
🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[esh-g]

@0xmiroslav Shouldn't we disable the bounce if there are not enough items? Because even after that solution, we would be able to drag the menu item below the upper part of the UI...

[0xmiros]

bounce is iOS lovely feature. No reason for disabling that. Please check past similar issues, where same discussion happened.

[pecanoro]

Sounds good, assigning @bernhardoj

[melvin-bot]

❌ There was an error making the offer to @0xmiroslav for the Reviewer role. The BZ member will need to manually hire the contributor.

[melvin-bot]

❌ There was an error making the offer to @bernhardoj for the Contributor role. The BZ member will need to manually hire the contributor.

[bernhardoj]

PR is ready

cc: @0xmiroslav

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

• Fix workspace members list go under the list container when scrolling #30878

If no regressions arise, payment will be issued on 2023-11-22. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @bernhardoj requires payment
• @0xmiroslav requires payment

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

• [@0xmiroslav] The PR that introduced the bug has been identified. Link to the PR:
• [@0xmiroslav] 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:
• [@0xmiroslav] 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:
• [@0xmiroslav] Determine if we should create a regression test for this bug.
• [@0xmiroslav] 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.
• [@adelekennedy] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[adelekennedy]

payday!

Payouts due:
Contributor: $500 @bernhardoj
Contributor+: $500 @0xmiroslav

I sent you a manual request on Upwork!

Upwork job is here.

[pecanoro]

@adelekennedy Can we close this one? 😄

[adelekennedy]

@0xmiroslav is the bugzero checklist complete?

[0xmiros]

• The PR that introduced the bug has been identified. Link to the PR: Selection List refactor phase 3: base #27767
• 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/27767/files#r1406388518
• 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. We can skip regression test as this is super minor UI "bug" (actually not bug)
• 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.

[adelekennedy]

@bernhardoj missed that - just ended the contract

[bernhardoj]

That was quick, thanks!