[HOLD for payment 2024-02-01] [HOLD for payment 2024-01-25] [$500] Referral page - Animation on Referral page does not move

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

---

Version Number: 1.4.12-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:

1. Click on the FAB.
2. Click on "Request money"
3. Enter any amount.
4. Click on "Next".
5. Click on "Request money, get $250.".
6. Click "Learn more"

Expected Result:

Animation on Referral page should move

Actual Result:

Animation on Referral page does not move

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

Bug6312397_1702513962704.Recording__1551.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0192e339c759eff93d
• Upwork Job ID: 1735235226940305408
• Last Price Increase: 2023-12-14
• Automatic offers:
• akinwale | Reviewer | 28066892
• BhuvaneshPatil | Contributor | 28066893

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0192e339c759eff93d

[melvin-bot]

Triggered auto assignment to @mallenexpensify (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 - @akinwale (External)

[ZhenjaHorbach]

Proposal

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

Referral page - Animation on Referral page does not move

What is the root cause of that problem?

We use just icon
Not Lottie animation

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

We need to update components and instead icon use Lottie animation

App/src/pages/ReferralDetailsPage.js

Lines 65 to 71 in e81ed8f

	headerContent={
	<Icon
	src={PaymentHands}
	width={178}
	height={232}
	/>
	}

For example

 headerContent={ 
                <Lottie
                    source={LottieAnimations.Hands}
                    autoPlay
                    loop
                    style={styles.w100} // styles can be updated
                    webStyle={styles.w100}
                />
}

Or use the implementation from Preferences page

App/src/pages/settings/Preferences/PreferencesPage.js

Lines 50 to 55 in 5c0f3e4

	<IllustratedHeaderPageLayout
	title={translate('common.preferences')}
	onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS)}
	backgroundColor={theme.PAGE_THEMES[SCREENS.SETTINGS.PREFERENCES.ROOT].backgroundColor}
	illustration={LottieAnimations.PreferencesDJ}
	>

For example

        <IllustratedHeaderPageLayout
            title={translate('common.referral')}
            illustration={LottieAnimations.Hands}
            shouldShowBackButton
            backgroundColor={theme.PAGE_THEMES[SCREENS.RIGHT_MODAL.REFERRAL].backgroundColor}
        >

Plus I notice that this file is only place where we use PaymentHands from '@components/Icon/Illustrations';

So we can delete PaymentHands from our project

What alternative solutions did you explore? (Optional)

NA

[BhuvaneshPatil]

Proposal

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

The hands on referral page are not animated

What is the root cause of that problem?

We have used SVG icon instead of lottie animation.

App/src/pages/ReferralDetailsPage.js

Lines 63 to 74 in 5c0f3e4

	<HeaderPageLayout
	title={translate('common.referral')}
	headerContent={
	<Icon
	src={PaymentHands}
	width={178}
	height={232}
	/>
	}
	headerContainerStyles={[styles.staticHeaderImage, styles.justifyContentEnd]}
	backgroundColor={theme.PAGE_THEMES[SCREENS.RIGHT_MODAL.REFERRAL].backgroundColor}
	>

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

What alternative solutions did you explore? (Optional)

We shall use IllustratedHeaderPageLayout component with LottieAnimations.Hands to wrap the content, similar to what we have used in WorkspaceListPage
IllustratedHeaderPageLayout wraps the HeaderPageLayout so we shall consider the component that is built for specific purpose.

[melvin-bot]

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

[mallenexpensify]

@akinwale please review the above proposals

[akinwale]

This is a straightforward change and we can go with @ZhenjaHorbach's proposal here, considering the chronological order of the posted proposals.

🎀 👀 🎀 C+ reviewed.

[melvin-bot]

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

[BhuvaneshPatil]

Hello @akinwale
Can you please check the proposal that was posted early, because as. far as I remember when I posted proposal selected proposal didn't mention about IllustratedHeaderPageLayout. They simply mentioned about changing value of headerContent prop. And I agree the fix is simple so there should be emphasis on implementation details. Please re-consider.
And looking at the edits of selected proposal, it wasn't well prepared at the begin.

[akinwale]

@BhuvaneshPatil

Looking at the timestamps, it appears that both proposals were being edited simultaneously, which makes it difficult to determine who arrived at the correct solution.

In the future, it's best to follow the guidelines when making updates to proposals by creating a corresponding Updated Proposal post, so that it's easier to follow.

I will leave this up to the internal engineer to decide.

[BhuvaneshPatil]

@akinwale I understand it's quite hectic to go through timestamps.
If we consider usage of IllustratedHeaderPageLayout. Then my proposal was the first one to propose (in the initial version, without edit).

Here are the details -
For selected proposal -
Original proposal posted at 3:25, it didn't mention about what component to use. Then It was updated frequently (once a minute) for 3-4 times.
At 3:35 it mentioned about IllustratedHeaderPageLayout.

My Proposal -
First posted at 3:28, It mentioned root cause and Idea of using IllustratedHeaderPageLayout. Edited at same minute (grammar). at 3:46 Added explanation for using IllustratedHeaderPageLayout.

Sorry for being pushy, but this has happened with me before, the frequently edited proposal was selected just because it was posted earlier(incomplete).

@mallenexpensify @rlinoz Please consider these details before finalising.

[rlinoz]

Looking at the timestamps the first proposal to mention IllustratedHeaderPageLayout is indeed from @BhuvaneshPatil , @akinwale are you ok if we go with his proposal?

[akinwale]

Looking at the timestamps the first proposal to mention IllustratedHeaderPageLayout is indeed from @BhuvaneshPatil, @akinwale are you ok if we go with his proposal?

@rlinoz Yes, we can move forward with @BhuvaneshPatil's proposal since it was first with the solution chronologically.

[melvin-bot]

📣 @akinwale 🎉 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]

📣 @BhuvaneshPatil 🎉 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 📖

[rlinoz]

Thanks, assigned @BhuvaneshPatil to continue!

[shawnborton]

Curious what @mallenexpensify thinks, but I think this is actually just follow up fixes from @rezkiy37 's original implementation, so we should not be hiring or paying anything out for this.

[mallenexpensify]

@BhuvaneshPatil at this point, since most of the work is being done already by @rezkiy37 as a followup, let's hold on having you do any work and we'll discuss once it's done (unless you're specifically asked/tagged to do work. If so, please help).

Since you were hired, you'll be compensated, we'll discuss the percentage/amount upon completion. That work for you?

[BhuvaneshPatil]

sure @mallenexpensify . Thanks

[rezkiy37]

The PR - #34099 - is ready for review 🙂

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

• Tweak styles of the Referral page #34099

If no regressions arise, payment will be issued on 2024-01-25. 🎊

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

• @akinwale requires payment automatic offer (Reviewer)
• @BhuvaneshPatil 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:

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

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

• Paint the status bar of the Referral page #34806

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

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

• @akinwale requires payment automatic offer (Reviewer)
• @BhuvaneshPatil 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:

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

[akinwale]

• [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale] 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:
• [@akinwale] 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:

Not a regression. This was a design change.

• [@akinwale] Determine if we should create a regression test for this bug.
• [@akinwale] 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 Steps

1. Launch the Expensify app.
2. Open the Referral page.
3. Verify that the design matches the mockups.
4. On iOS Safari, verify that the status bar background colour matches.

[akinwale]

@mallenexpensify Bumping here for payment. Thanks.

cc @BhuvaneshPatil

[mallenexpensify]

Contributor: @BhuvaneshPatil paid $500 via Upwork
Contributor+: @akinwale paid $500 via Upwork

Thanks!