[$2000] Empty state icons for `reimbursementAccountLoadingAnimation` don't have a transparent background in Dark Theme. #13151

kavimuru · 2022-11-29T19:44:09Z

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:

Go to any workspace and attempt the Connect Bank Account step
Enter the bank account details, go to the company step
Fill in the company step, and wait for the reimbursement account loading animation (screen with the title One moment)
Check that the icons don't have a transparent background (instead have a gray background).

Expected Result:

The Icons should have a transparent background

Actual Result:

the icons have a gray background.

Workaround:

unknown

Platform:

Where is this issue occurring?

Mobile Web

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

RPReplay_Final1669717357.MP4

JFZT3783.1.MP4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01d555366565521d92
Upwork Job ID: 1597976370465353728
Last Price Increase: 2022-12-20

melvin-bot · 2022-11-29T19:44:14Z

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

mananjadhav · 2022-11-29T19:45:11Z

Tagging @Expensify/design and @shawnborton too

abekkala · 2022-11-29T20:21:43Z

@shawnborton is this going to be fixed internally?
If so, then I can just create an Upwork posting to hire/pay the contributor who reported the issue.

shawnborton · 2022-11-29T20:22:53Z

The new .gif can be created internally, but we can use a contributor to update the .gif (it's just a file path that points to cloudfront) in the app.

Puneet-here · 2022-11-30T00:00:53Z

@shawnborton, I can make the change (it needs to be done here). We also need to update other gif that we are using locally (check the one below)

https://github.com/Expensify/App/blob/main/assets/images/confetti-pop.gif

shawnborton · 2022-11-30T00:01:52Z

Where are we using confetti pop? I thought we stopped using it with the new workspace illustrations that we merged?

Puneet-here · 2022-11-30T00:18:10Z

It's at password confirmation screen and request call confirmation
Once you change your password successfully you will see it or after requesting a call.

App/src/pages/RequestCallConfirmationScreen.js

Lines 18 to 19 in 3c22789

    
           <Image 
        
               source={confettiPop}

App/src/pages/settings/PasswordConfirmationScreen.js

Lines 18 to 19 in 11072a6

    
           <Image 
        
               source={confettiPop}

shawnborton · 2022-11-30T00:22:10Z

Can you share screenshots of those two pages?

Puneet-here · 2022-11-30T00:28:36Z

Puneet-here · 2022-11-30T00:31:08Z

We can use Illustrations.TadaBlue instead

            <Icon
                src={Illustrations.TadaBlue}
                height={100}
                width={100}
                fill={defaultTheme.iconSuccessFill}
            />

and maybe some margin below the icon.

shawnborton · 2022-11-30T00:31:52Z

I think we can use the new simple thumbs up illustration for those:

Puneet-here · 2022-11-30T00:35:44Z

Looks good to me.

mananjadhav · 2022-11-30T03:24:29Z

In this are we covering all the illustrations? One is the Reimbursement Loading Animation, and second is the Request a callback?

Puneet-here · 2022-11-30T07:46:17Z

One is the Reimbursement Loading Animation, and second is the Request a callback?

PasswordConfirmationScreen will be changed too

App/src/pages/settings/PasswordConfirmationScreen.js

Lines 18 to 19 in 11072a6

    
           <Image 
        
               source={confettiPop}

Puneet-here · 2022-11-30T08:01:33Z

Proposal

I think we can create a new confirmationPage component and we can pass props to it. This way we can reuse it.
We are using same logic at multiple pages -
PasswordConfirmationScreen
RequestCallConfirmationScreen
AddPersonalBankAccountPage
TransferBalancePage
ActivateStep

The code will look like this -

     <>
        <View style={[styles.screenCenteredContainer, styles.alignItemsCenter]}>
            <Icon
                src={props.illustration}
                height={100}
                width={100}
                fill={defaultTheme.iconSuccessFill}
            />
            <Text
                style={[
                    styles.textStrong,
                    styles.textLarge,
                    styles.mv2,
                ]}
            >
                {props.heading}
            </Text>
            <Text style={styles.textAlignCenter}>
                {props.description}
            </Text>
        </View>
        {props.shouldShowButton && (
        <FixedFooter>
            <Button
                success
                text={props.buttonText}
                style={styles.mt6}
                pressOnEnter
                onPress={props.onButtonPress}
            />
        </FixedFooter>
        )}
    </>

I have added props.shouldShowButton so that it can be used at ActivateStep too

Puneet-here · 2022-11-30T08:49:05Z

@shawnborton at AddPersonalBankAccountPage
TransferBalancePage we are using TadaBlue should we also change the illustration for them?

melvin-bot · 2022-11-30T15:30:26Z

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

melvin-bot · 2022-11-30T15:30:35Z

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

melvin-bot · 2022-11-30T15:30:41Z

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

melvin-bot · 2022-11-30T15:30:47Z

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

joekaufmanexpensify · 2022-12-22T17:14:57Z

On that note, I am going to be OOO from tomorrow - January 2nd. Given the PR is still in review, it's unlikely to be ready for payment by the time I'm back (as we still need to finish reviewing, merge and deploy, and then wait the 7 day regression period).

With that in mind, I'm not going to re-assign this to another member of Bug Zero team while I'm OOO as there should be no urgent action needed on my part. I'm also going to change the prioritization here to weekly until I'm back. If at any time someone from the bug-zero team is needed here while I'm out, feel free to raise in #contributor-plus or #expensify-open-source and someone will assist.

Otherwise, I'll change the prioritization back to daily once I'm back!

joekaufmanexpensify · 2022-12-22T17:25:06Z

Okay @mountiny confirmed they can assist with reviewing the PR, so assigning them to the issue as well!

JmillsExpensify · 2022-12-22T19:21:50Z

Nice! Also, I'm happy to keep an eye on this one while you're out. Will help move things along, if required.

joekaufmanexpensify · 2022-12-22T19:31:22Z

Sounds great, thank you!

mountiny · 2022-12-23T10:09:29Z

Merged 🎉 nice job @Puneet-here

Puneet-here · 2022-12-23T13:40:31Z

Thanks for the help reviewing it.

JmillsExpensify · 2022-12-23T16:37:03Z

Nice, we'll circle back on this for payment as the regression period passes.

joekaufmanexpensify · 2023-01-03T13:32:54Z

Hm, looks like the bug zero checklist / payment notification was not added to this issue when the PR was deployed to production 5 days ago? I'm thinking maybe they weren't added because I manually changed the issue to weekly?

Regardless, I will manually create the BZ checklist, and payment checklist on the issue. Payment will be issued on 2023-01-05.

joekaufmanexpensify · 2023-01-05T19:44:49Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.44-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

#13744

If no regressions arise, payment will be issued on 2023-01-05. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

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

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

joekaufmanexpensify · 2023-01-05T19:47:05Z

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:

[@tgolen / @mountiny / @sobitneupane] The PR that introduced the bug has been identified. Link to the PR:
[@tgolen / @mountiny / @sobitneupane] 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:
[@tgolen / @mountiny / @sobitneupane] 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: https://expensify.slack.com/archives/C049HHMV9SM/p1672994272661819
[@joekaufmanexpensify] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: https://github.com/Expensify/Expensify/issues/253400

joekaufmanexpensify · 2023-01-05T20:22:25Z

Okay 7 day regression period has passed, and we're all set to issue payment here. Given the PR was merged within 3 business days from when the contributor was assigned to the issue, both the contributor and C+ qualify for a 50% bonus here. We need to pay the following amounts:

@mananjadhav - $250 for reporting bug.
@Puneet-here - $3,000 for PR fixing the bug (includes 50% bonus of $1,000).
@sobitneupane - $3,000 for C+ review (includes 50% bonus of $1,000).

joekaufmanexpensify · 2023-01-05T20:24:47Z

@mananjadhav $250 sent, and contract ended!

joekaufmanexpensify · 2023-01-05T20:26:26Z

@Puneet-here $3,000 sent, and contract ended!

joekaufmanexpensify · 2023-01-05T20:29:46Z

@sobitneupane $3,000 sent, and contract ended!

joekaufmanexpensify · 2023-01-05T20:30:08Z

Job closed in upwork.

joekaufmanexpensify · 2023-01-05T20:46:02Z

Proposed new regression test here!

mountiny · 2023-01-06T08:38:20Z

To complete the checklist, I have commented on this Pr where we have made the switch to the dark mode #12944 (comment) and we should have probably update the icons there or at least have a follow up ready.

Posted a discussion in Slack https://expensify.slack.com/archives/C049HHMV9SM/p1672994272661819

I think the regression tests Joe posted should be enough for this going forwards.

joekaufmanexpensify · 2023-01-06T19:32:09Z

Added new regression test issue above!

joekaufmanexpensify · 2023-01-06T19:32:41Z

Okay, bug is fixed, payment is handled, and BZ checklist is complete. This is all set. Thanks everyone!

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 29, 2022

melvin-bot bot assigned abekkala Nov 29, 2022

shawnborton self-assigned this Nov 29, 2022

Puneet-here mentioned this issue Nov 29, 2022

The popups in NewDot are white in dark mode #13162

Closed

abekkala added the External Added to denote the issue can be worked on by a contributor label Nov 30, 2022

melvin-bot bot changed the title ~~Empty state icons for reimbursementAccountLoadingAnimation don't have a transparent background in Dark Theme.~~ [$1000] Empty state icons for reimbursementAccountLoadingAnimation don't have a transparent background in Dark Theme. Nov 30, 2022

melvin-bot bot assigned sobitneupane Nov 30, 2022

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 30, 2022

melvin-bot bot assigned tgolen Nov 30, 2022

joekaufmanexpensify added Weekly KSv2 and removed Daily KSv2 labels Dec 22, 2022

joekaufmanexpensify assigned mountiny Dec 22, 2022

joekaufmanexpensify added Daily KSv2 and removed Weekly KSv2 labels Jan 5, 2023

mountiny mentioned this issue Jan 6, 2023

Make the Switch to Dark Mode #12944

Merged

100 tasks

joekaufmanexpensify closed this as completed Jan 6, 2023

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Nov 29, 2023

[$2000] Empty state icons for reimbursementAccountLoadingAnimation don't have a transparent background in Dark Theme. #13151

[$2000] Empty state icons for reimbursementAccountLoadingAnimation don't have a transparent background in Dark Theme. #13151

Comments

kavimuru commented Nov 29, 2022 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platform:

melvin-bot bot commented Nov 29, 2022

mananjadhav commented Nov 29, 2022

abekkala commented Nov 29, 2022

shawnborton commented Nov 29, 2022

Puneet-here commented Nov 30, 2022

shawnborton commented Nov 30, 2022

Puneet-here commented Nov 30, 2022 • edited Loading

shawnborton commented Nov 30, 2022

Puneet-here commented Nov 30, 2022

Puneet-here commented Nov 30, 2022

shawnborton commented Nov 30, 2022 • edited Loading

Puneet-here commented Nov 30, 2022

mananjadhav commented Nov 30, 2022

Puneet-here commented Nov 30, 2022

Puneet-here commented Nov 30, 2022 • edited Loading

Proposal

Puneet-here commented Nov 30, 2022

melvin-bot bot commented Nov 30, 2022

melvin-bot bot commented Nov 30, 2022

melvin-bot bot commented Nov 30, 2022

melvin-bot bot commented Nov 30, 2022

joekaufmanexpensify commented Dec 22, 2022

joekaufmanexpensify commented Dec 22, 2022

JmillsExpensify commented Dec 22, 2022

joekaufmanexpensify commented Dec 22, 2022

mountiny commented Dec 23, 2022

Puneet-here commented Dec 23, 2022

JmillsExpensify commented Dec 23, 2022

joekaufmanexpensify commented Jan 3, 2023

joekaufmanexpensify commented Jan 5, 2023 • edited Loading

joekaufmanexpensify commented Jan 5, 2023 • edited Loading

joekaufmanexpensify commented Jan 5, 2023

joekaufmanexpensify commented Jan 5, 2023

joekaufmanexpensify commented Jan 5, 2023

joekaufmanexpensify commented Jan 5, 2023

joekaufmanexpensify commented Jan 5, 2023

joekaufmanexpensify commented Jan 5, 2023

mountiny commented Jan 6, 2023

joekaufmanexpensify commented Jan 6, 2023

joekaufmanexpensify commented Jan 6, 2023

[$2000] Empty state icons for `reimbursementAccountLoadingAnimation` don't have a transparent background in Dark Theme. #13151

[$2000] Empty state icons for `reimbursementAccountLoadingAnimation` don't have a transparent background in Dark Theme. #13151

kavimuru commented Nov 29, 2022 •

edited by melvin-bot bot

Loading

Puneet-here commented Nov 30, 2022 •

edited

Loading

shawnborton commented Nov 30, 2022 •

edited

Loading

Puneet-here commented Nov 30, 2022 •

edited

Loading

joekaufmanexpensify commented Jan 5, 2023 •

edited

Loading

joekaufmanexpensify commented Jan 5, 2023 •

edited

Loading