Add confirmation page #13744

Puneet-here · 2022-12-20T22:41:46Z

Details

Fixed Issues

$ #13151
PROPOSAL: #13151 (comment)

Tests

Go to concierge > Click call icon
Fill data > Request call
Verify you see a confirmation page with fireworks gif

Go to settings > security
Change password > Fill data > save
Verify you see a confirmation page with fireworks gif

Go to settings > payments > Add payment method
Bank account > Fill data to add the bank account
Verify you see a confirmation page with fireworks gif

Import and render TransferBalancePage in some other component (for example ProfilePage, don't forget to remove the code for profile page)
Return true in the if statement

App/src/pages/settings/Payments/TransferBalancePage.js

Line 155 in 3ecec61

if (this.props.walletTransfer.shouldShowSuccess && !this.props.walletTransfer.loading) {
Go to the page where you rendered the TransferBalancePage
Verify you see a new gif for dark theme

Import and render ActivateStep in some other component
Go to the page where you rendered the ActivateStep
Verify you see a loading gif and there's no button
Make goldWallet true

App/src/pages/EnablePayments/ActivateStep.js

Line 39 in 3ecec61

const isGoldWallet = props.userWallet.tierName === CONST.WALLET.TIER_NAME.GOLD;
Verify you see the confirmation fireworks gif and a button
Note: At ios you will see that the header is near the status bar. It is because in ActivateStep the ScreenWrapper isn't present as the parent because the parent component of ActivateStep already contains a ScreenWrapper.

Go to settings > workspaces >
Open a workspace > Connect bank account
Choose connect manually > Fill data > Save
Fill company info form and save
Verify you see a new gif for dark theme (Note: you will see the loading indicator first, the gif only shows up for brief moment)

Verify that no errors appear in the JS console

Offline tests

No offline specific steps

QA Steps

Go to concierge > Click call icon
Fill data > Request call
Verify you see a confirmation page with fireworks gif

Go to settings > security
Change password > Fill data > save
Verify you see a confirmation page with fireworks gif

Go to settings > payments > Add payment method
Bank account > Fill data to add the bank account
Verify you see a confirmation page with fireworks gif

Go to settings > payments
Transfer balance > go through the steps
Verify you see a confirmation page with fireworks gif

Go to settings > workspaces >
Open a workspace > Connect bank account
Choose connect manually > Fill data > Save
Fill company info form and save
Verify you see a new gif for dark theme (Note: you will see the loading indicator first, the gif only shows up for brief moment)

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2022-12-22.at.2.03.33.AM.mov

Screen.Recording.2022-12-22.at.3.14.04.AM.mov

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

Puneet-here · 2022-12-21T22:52:38Z

I found a bug (not related to the PR) on pressing cross at confirmation screen of transfer balance page it navigates to the payments pages instead of closing the modal

melvin-bot · 2022-12-21T22:52:53Z

@tgolen @sobitneupane One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

sobitneupane · 2022-12-22T07:48:56Z

Thanks @Puneet-here. Changes are looking good. Will review it today.

luacmartins

LGTM and tests well!

mountiny · 2022-12-22T18:12:48Z

@shawnborton Tagging you here for a review because I am jsut a bit concerneed about the background colour on some of the gifs. For example look at this:

I am not sure if it was raised in other places, but I could not see it in the comments. The Hex of the gif green bg does not seem exactly the same as the app background. You can also check the screenshots in the PR body from Puneet and you can see the colour different there too.

Since the images are hosted in cloud we can update that without touching this PR. so I think it is NAB, but wanted to note this

shawnborton · 2022-12-22T18:17:02Z

Good catch. I think we might have to live with this until we replace these GIFs with Lottie animations, where we will have perfectly crisp SVG animations and no background color.

shawnborton · 2022-12-22T18:17:37Z

That being said, I'll flag this to Augenblick and see if they have any ideas!

mountiny · 2022-12-22T18:26:24Z

Great, thanks, wanted to get a confirmation this is not a blocker 🙇

sobitneupane

Screenshots/Videos

Web

Web_Enable_Payment.mov

Web_TransferBalance_Confirmation.mov

Web_Company_Information.mov

Web_Change_Password_Confirmation.mov

Web_Request_Call_Confirmation.mov

Web_AddPayment_Confirmation.mov

Web_Enable_Payment_Confirmatioin.mov

Mobile Web - Chrome

Chrome_Company_Information.mov

Chrome_AddPayment_Confirmation.mov

Chrome_Change_Password.mov

Chrom_Request_Call.mov

Chrome_TransferBalance_confirmation.mov

Mobile Web - Safari

Safari_ChangePassword.mov

Safari_RequestCall.mov

Desktop

Desktop_Company_Information.mov

Desktop_AddPayment_Confirmation.mov

Desktop_Change_Password_Confirmation.mov

Desktop_Request_Call_Confirmation.mov

iOS

IOS_Company_information.mov

IOS_Change_Password.mov

IOS_RequestCall.mov

IOS_Add_payment_method.mov

Android

Android_Add_payment_method.mov

Android_Company_information.mov

Android_Change_password.mov

Android_Requestcall.mov

shawnborton · 2022-12-22T19:02:18Z

Nice, these look great 🎆

mountiny

@Puneet-here One minor thing

src/components/ConfirmationPage.js

shawnborton · 2022-12-22T20:31:05Z

Okay we have two new gifs we can try to fix the BG issue that @mountiny spotted, @Puneet-here can you test with these locally and show us some screenshots? If these look better, I will upload to Cloudfront.

animations.zip

Puneet-here · 2022-12-22T20:58:38Z

@shawnborton, it still doesn't match completely with the background

Screen.Recording.2022-12-23.at.2.22.13.AM.mov

shawnborton · 2022-12-22T21:00:12Z

Okay no worries, appreciate you giving that a shot! I say we just keep what we have for now and we'll revisit these with Lottie down the road.

mountiny · 2022-12-22T21:48:12Z

Thank you for updating that, I see this is not super enforced 🙇

We are missing a checklist from @sobitneupane :/

sobitneupane

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

mountiny

Thank you guys @Puneet-here @sobitneupane 🙇

OSBotify · 2022-12-23T10:11:46Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2022-12-23T10:31:32Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
TTI	811.718 ms → 815.663 ms (+3.945 ms, ±0.0%)
nativeLaunch	9.667 ms → 9.903 ms (+0.237 ms, +2.4%)
regularAppStart	0.014 ms → 0.015 ms (+0.001 ms, +4.6%)
runJsBundle	185.645 ms → 184.625 ms (-1.020 ms, -0.5%)

Show details

Name	Duration
TTI	Baseline Mean: 811.718 ms Stdev: 43.794 ms (5.4%) Runs: 748.12847100012 749.98244300019 753.2723399996758 757.5037930002436 761.3756130002439 767.7094780001789 769.814915000461 778.507179999724 781.7683100001886 791.2354560000822 791.8988690003753 797.3880460001528 800.098505999893 800.4654510002583 804.441042999737 804.8405170002952 806.9756370000541 808.2387150004506 814.2342259995639 815.4079750003293 819.0739949997514 825.5793789997697 839.2781480001286 842.2696200003847 844.032692999579 846.6975670000538 854.7700890004635 886.3899860000238 890.154265999794 899.8985660001636 911.823369000107 Current Mean: 815.663 ms Stdev: 43.826 ms (5.4%) Runs: 752.5090570002794 755.139033999294 761.6871899999678 767.7082119993865 772.6614270005375 773.1865860000253 776.2212279997766 785.1062700003386 788.1761569995433 788.7935899998993 793.6647159997374 794.1606580000371 796.522600999102 797.5516749992967 798.1893509998918 805.8280950002372 806.5959140006453 809.5929949991405 822.4293969999999 824.1723939999938 827.1593990009278 830.8274869993329 838.824040999636 848.7601709999144 856.2598519995809 856.4966829996556 860.8537239991128 877.1761089991778 884.0215579997748 908.3540109992027 926.9145919997245
nativeLaunch	Baseline Mean: 9.667 ms Stdev: 1.468 ms (15.2%) Runs: 8 8 8 8 8 8 8 8 9 9 9 9 9 9 9 10 10 10 10 10 10 10 10 11 11 11 12 12 13 13 Current Mean: 9.903 ms Stdev: 1.593 ms (16.1%) Runs: 8 8 8 8 8 8 8 9 9 9 9 9 9 9 9 10 10 10 10 10 11 11 11 11 11 11 12 12 12 13 14
regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (9.6%) Runs: 0.012045000679790974 0.01285799965262413 0.013224000111222267 0.013265000656247139 0.013306000269949436 0.013387000188231468 0.013427000492811203 0.013467999175190926 0.013590999878942966 0.013672000728547573 0.013793999329209328 0.013956000097095966 0.014161000028252602 0.014201000332832336 0.014241000637412071 0.0143630001693964 0.014403999783098698 0.014403999783098698 0.014485999941825867 0.014566999860107899 0.014607000164687634 0.014607999473810196 0.01476999931037426 0.015339999459683895 0.015786999836564064 0.01590999960899353 0.017008000053465366 0.017943999730050564 0.018228999339044094 Current Mean: 0.015 ms Stdev: 0.001 ms (4.7%) Runs: 0.01371300034224987 0.013996999710798264 0.0142000000923872 0.014201000332832336 0.014322999864816666 0.01440500095486641 0.014527000486850739 0.01464799977838993 0.01464799977838993 0.014688998460769653 0.014689000323414803 0.014892999082803726 0.01493300125002861 0.01493300125002861 0.015015000477433205 0.015137000009417534 0.015178000554442406 0.015217998996376991 0.015258999541401863 0.015380999073386192 0.015502998605370522 0.015584999695420265 0.015665998682379723 0.0157880000770092 0.0157880000770092 0.015869000926613808 0.016071999445557594 0.01619499921798706 0.016276000067591667 0.016479000449180603
runJsBundle	Baseline Mean: 185.645 ms Stdev: 18.706 ms (10.1%) Runs: 157 162 163 163 169 170 171 171 174 177 177 178 179 181 182 182 183 183 184 185 185 189 192 196 197 207 210 211 216 223 238 Current Mean: 184.625 ms Stdev: 17.874 ms (9.7%) Runs: 161 164 166 168 168 168 169 170 171 172 174 175 175 177 177 178 180 180 182 183 184 188 196 196 199 199 199 210 212 212 223 232

OSBotify · 2022-12-27T19:10:23Z

🚀 Deployed to staging by @mountiny in version: 1.2.44-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

mvtglobally · 2022-12-27T20:40:11Z

@mountiny @shawnborton Can you pls validate the following steps internally? We do not have active wallet to be able to transfer. Rest steps we are QA-ing

Go to settings > payments
Transfer balance > go through the steps
Verify you see a confirmation page with fireworks gif

AndrewGable · 2022-12-28T18:29:01Z

I see fireworks on the transfer page!

OSBotify · 2022-12-28T19:04:40Z

🚀 Deployed to production by @chiragsalian in version: 1.2.44-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

Santhosh-Sellavel · 2023-02-14T01:13:41Z

Seems this PR caused a regression, i.e missed catching this bug where the welcome text is not centered in Spanish!

sobitneupane · 2023-02-14T04:23:54Z

Thanks @Santhosh-Sellavel for linking the issue. Though the bug precedes this PR and was existent in some confirmation screens like PasswordConfirmationScreen and RequestCallConfirmationScreen, It should have been caught in this PR.

Puneet-here added 10 commits December 21, 2022 03:16

add fireworks gif

80e73bf

create ConfirmationPage

19a4864

use ConfirmationPage in PasswordPage

075f10b

remove PasswordConfirmationScreen

63a595e

use ConfirmationPage

ad353f0

remove RequestCallConfirmationScreen

3a12e2b

rename

7475b7e

increase size

8a68379

use ConfirmationPage

d388cb4

use ConfirmationPage

4b623b4

Puneet-here mentioned this pull request Dec 20, 2022

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

Closed

Puneet-here added 6 commits December 22, 2022 01:23

Use gif from cloudfront

f6e3102

remove unused gif

7cef936

update ActivateStep to use ConfirmationPage

d64ed38

update url

ee6b8b9

decrease width and height

c0f9b15

use uri

4deeae6

Puneet-here marked this pull request as ready for review December 21, 2022 22:52

Puneet-here requested a review from a team as a code owner December 21, 2022 22:52

melvin-bot bot requested review from sobitneupane and tgolen and removed request for a team December 21, 2022 22:52

mountiny assigned Puneet-here Dec 22, 2022

mountiny requested review from mountiny and removed request for tgolen December 22, 2022 17:09

luacmartins self-requested a review December 22, 2022 17:33

luacmartins previously approved these changes Dec 22, 2022

View reviewed changes

sobitneupane reviewed Dec 22, 2022

View reviewed changes

mountiny reviewed Dec 22, 2022

View reviewed changes

src/components/ConfirmationPage.js Outdated Show resolved Hide resolved

style change

209b51e

Puneet-here dismissed luacmartins’s stale review via 209b51e December 22, 2022 21:46

mountiny approved these changes Dec 22, 2022

View reviewed changes

luacmartins approved these changes Dec 22, 2022

View reviewed changes

sobitneupane approved these changes Dec 23, 2022

View reviewed changes

mountiny approved these changes Dec 23, 2022

View reviewed changes

mountiny merged commit 5459d9d into Expensify:main Dec 23, 2022

OSBotify mentioned this pull request Dec 27, 2022

Deploy Checklist: New Expensify 2022-12-27 #13843

Closed

17 tasks

Santhosh-Sellavel mentioned this pull request Feb 14, 2023

[HOLD for payment 2023-02-13] [$1000] Web - Payment - The welcome message in Spanish not centered #14734

Closed

6 tasks

blazejkustra mentioned this pull request Nov 27, 2023

[TS migration] Migrate 'ConfirmationPage.js' component to TypeScript #30267

Merged

56 tasks

alitoshmatov mentioned this pull request Jan 25, 2024

[HOLD for payment 2024-01-25] [$500] BA - Animation in "One moment" page is smaller than other environments #33134

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add confirmation page #13744

Add confirmation page #13744

Puneet-here commented Dec 20, 2022 •

edited

Loading

Puneet-here commented Dec 21, 2022

melvin-bot bot commented Dec 21, 2022

sobitneupane commented Dec 22, 2022

luacmartins left a comment

mountiny commented Dec 22, 2022

shawnborton commented Dec 22, 2022

shawnborton commented Dec 22, 2022

mountiny commented Dec 22, 2022

sobitneupane left a comment •

edited

Loading

shawnborton commented Dec 22, 2022

mountiny left a comment

shawnborton commented Dec 22, 2022

Puneet-here commented Dec 22, 2022

shawnborton commented Dec 22, 2022

mountiny commented Dec 22, 2022

sobitneupane left a comment

mountiny left a comment

OSBotify commented Dec 23, 2022

github-actions bot commented Dec 23, 2022

OSBotify commented Dec 27, 2022

mvtglobally commented Dec 27, 2022

AndrewGable commented Dec 28, 2022

OSBotify commented Dec 28, 2022

Santhosh-Sellavel commented Feb 14, 2023

sobitneupane commented Feb 14, 2023

Add confirmation page #13744

Add confirmation page #13744

Conversation

Puneet-here commented Dec 20, 2022 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Puneet-here commented Dec 21, 2022

melvin-bot bot commented Dec 21, 2022

sobitneupane commented Dec 22, 2022

luacmartins left a comment

Choose a reason for hiding this comment

mountiny commented Dec 22, 2022

shawnborton commented Dec 22, 2022

shawnborton commented Dec 22, 2022

mountiny commented Dec 22, 2022

sobitneupane left a comment • edited Loading

Choose a reason for hiding this comment

Screenshots/Videos

shawnborton commented Dec 22, 2022

mountiny left a comment

Choose a reason for hiding this comment

shawnborton commented Dec 22, 2022

Puneet-here commented Dec 22, 2022

shawnborton commented Dec 22, 2022

mountiny commented Dec 22, 2022

sobitneupane left a comment

Choose a reason for hiding this comment

Reviewer Checklist

mountiny left a comment

Choose a reason for hiding this comment

OSBotify commented Dec 23, 2022

github-actions bot commented Dec 23, 2022

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

OSBotify commented Dec 27, 2022

mvtglobally commented Dec 27, 2022

AndrewGable commented Dec 28, 2022

OSBotify commented Dec 28, 2022

Santhosh-Sellavel commented Feb 14, 2023

sobitneupane commented Feb 14, 2023

Puneet-here commented Dec 20, 2022 •

edited

Loading

sobitneupane left a comment •

edited

Loading