[HOLD for @0xmiroslav payment] [$1000] mweb: Green background colour visible when scrolling on the Preference Page

[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 Open app
2 Go to Settings > Preference
3 Make a scroll

Expected Result:

When scrolling the colour remains Blue

Actual Result:

Some parts of the screen remain light green.

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 / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.44-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
Notes/Photos/Videos: Any additional supporting documentation

IMG_1465.MP4

KFKX5576.1.MP4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0135c9358446287985
• Upwork Job ID: 1683858764520570880
• Last Price Increase: 2023-09-05
• Automatic offers:
• 0xmiroslav | Reviewer | 26515161
• ginsuma | Contributor | 26515162
• narzullome | Reporter | 26515167
• ishpaul777 | Contributor | 26515517

[melvin-bot]

Triggered auto assignment to @slafortune (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

[ginsuma]

Proposal

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

mWeb: Green background colour visible when scrolling on the Preference Page

What is the root cause of that problem?

The overscrollSpacer here cannot applied to Safari.
This is a issue with stack order when the parent node has scrollbar on Safari.

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

Add willChange: "transform" to overscrollSpacer styles.

[slafortune]

Bug report looks good!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0135c9358446287985

[melvin-bot]

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

[melvin-bot]

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

[ginsuma]

@slafortune Could you explain me why you edit my proposal to @narzullome?

[slafortune]

I added their name to the proposal title so it's easy to recognize who the proposal is from - do you think there is an easier way to show that we should have done?

AH SHOOT! Sorry about that - I was referring to an incorrect slack thread, my mistake! Thanks for calling it out!

[slafortune]

@0xmiroslav - when you get a chance, can you review @ginsuma's proposal?

[melvin-bot]

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

[slafortune]

Hi @0xmiroslav - this seems as it would be a straight forward fix, can you review this proposal?

[narzullome]

@slafortune HI the same issue on the Workspace screen

IMG_1654.MP4

[aditygrg2]

Proposal

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

Non-matching background color on IllustratedHeaderPageLayout pages

What is the root cause of that problem?

The present view component is not placed correctly. It should be placed above the ScrollView.

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

1. The view should be made one level above here. This is incorrectly set presently.
2. We can add background colors to the children's content of the page as well as the FixedFooter component
3. The top can be made as 0 and zIndex as a negative constant because the view should be present behind the ScrollView.

overscrollSpacer: (backgroundColor, height) => ({
        backgroundColor,
        height,
        width: '100%',
        position: 'absolute',
        top: 0,
        zIndex: -2,
        left: 0,
        right: 0,
    }),

After these changes, the views will look like this:

Loom.Message.-.6.August.2023.mp4

This issue was also reproducible in Safari/Mac and it will look like this once implemented:

Improvement_safari_mac.mp4

Android and chrome are not affected by this because iOS and Safari native views only support the bouncing effect on over scroll.

[ishpaul777]

Proposal

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

on Safari - Green background colour visible when scrolling on the Preference Page and workspace page

What is the root cause of that problem?

The overscroller background is placed properly in component stack and styles need to be modified to handle the bounce effect in ios.

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

we need to put the overscrollSpacer out of ScrollView component and modify the styles.(we will add appropriate objects from style.js below is just rough implemetation)

 <View style={[styles.flex1]}>
                        <View
                            style={{
                                position: 'absolute',
                                top: 0,
                                left: 0,
                                width: '100%',
                                height: '100%',
                                zIndex: -1,
                            }}
                        >
                            <View style={{flex: 1, backgroundColor}} />
                            <View style={{flex: 1, backgroundColor: themeColors.appBG}} />
                        </View>
                        <ScrollView
                            contentContainerStyle={safeAreaPaddingBottomStyle}
                            showsVerticalScrollIndicator={false}
                        >
                            <View style={[styles.alignItemsCenter, styles.justifyContentEnd, StyleUtils.getBackgroundColorStyle(backgroundColor)]}>
                                <Lottie
                                    source={illustration}
                                    style={styles.w100}
                                    autoPlay
                                    loop
                                />
                            </View>
                            <View style={[styles.pt5, StyleUtils.getBackgroundColorStyle(themeColors.appBG)]}>{children}</View>
                        </ScrollView>
                        {!_.isNull(footer) && <FixedFooter style={[StyleUtils.getBackgroundColorStyle(themeColors.appBG)]}>{footer}</FixedFooter>}
                    </View>

and we need to modify overscrollSpacer flexbox with direction column. and make sure it is below the actual content by setting appropriate z-index value. the idea is to make it like below

As long as you don't bounce more than 50% of the scrollview height, you will see the appropriate background color. it is unlikely to bounce till 50% unless intentional.

I will add results soon...
Results-

Screen.Recording.2023-08-06.at.8.51.03.PM-1.mov

Alternative

1. we can play with onScroll Prop on ScrollView to dynamically set the background color. (this may result in performance regression)

                      onScroll={(event) => {
                                // 0 means the top of the screen, 100 would be scrolled 100px down
                                const currentYPosition = event.nativeEvent.contentOffset.y
                                const oldPosition = scrollYRef.current

                                if(oldPosition < currentYPosition) {
                                    // we scrolled down
                                    setOverScollBg(themeColors.appBG)
                                } else {
                                    // we scrolled up
                                    setOverScollBg(backgroundColor)
                                }
                                // save the current position for the next onScroll event
                                scrollYRef.current = currentYPosition
                            }}

2. set a max limit in overflow bounce (if possible)

[melvin-bot]

@slafortune @0xmiroslav this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

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

• fix overscroll spacer in safari #26873

If no regressions arise, payment will be issued on 2023-09-27. 🎊

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:

• @0xmiroslav requires payment offer (Reviewer)
• @ishpaul777 requires payment offer (Contributor)
• @narzullome requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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.
• [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[abekkala]

@roryabraham would a bonus be added here based on the PR being intentionally not merged?

[0xmiros]

@roryabraham would a bonus be added here based on the PR being intentionally not merged?

yes #26873 (comment)

[abekkala]

@0xmiroslav can you complete the checklist above?

[abekkala]

PAYMENTS TO BE MADE:

• @narzullome (Reporter) [$250] (reported before price change)

• @ishpaul777 (Contributor) [$1000 + 50% bonus]

• @0xmiroslav (Reviewer) [$1000 + 50% bonus]

[abekkala]

@ishpaul777 payment sent and contract ended - thank you! 🎉

@narzullome I've sent you the $250 reporting payout offer, once accepted I can initiate payment - thanks!

[0xmiros]

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: N/A
• [@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: N/A
• [@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: N/A
• [@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.

This is known limitation on safari and so we applied workaround to be consistent with iOS native.
No need regression test as this is minor visual inconsistency issue which doesn't affect app logic.

[ishpaul777]

Payment received! Thank you 🎉

[narzullome]

@abekkala Offer accepted

[melvin-bot]

@abekkala, @roryabraham, @0xmiroslav, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[abekkala]

@narzullome payment made and contract ended - thank you! 🎉

[abekkala]

waiting on payment for

@0xmiroslav (Reviewer) [$1000 + 50% bonus]

on hold until we determine how payments are to be made

[abekkala]

@0xmiroslav are you ready to accept payments via Upwork yet?

[0xmiros]

Let's close this for now. I am tracking locally