-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rename "deposit" to "payout": instant deposit button & modal #9554
rename "deposit" to "payout": instant deposit button & modal #9554
Conversation
- Use "payout" in modal and button UI - Update code comments - Update component and type names - Update CSS classes and relevant CSS rules
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: +16 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Moving this to Noting that I have still not tested manually, but I believe the automated tests should be all good now. As with all rename payouts changes, we'll need to decide how we merge/deploy these, until then this is blocked/should not merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a manual test and a round of review at the current stage. The changes seem to work fine. I have a small nitpick about copy on the button. Else the renaming seems to be done correctly across the button and modal copy, component names and style names too.
/* translators: %s: Monetary amount to deposit */ | ||
__( 'Deposit %s now', 'woocommerce-payments' ), | ||
/* translators: %s: Monetary amount to pay out */ | ||
__( 'Pay out %s now', 'woocommerce-payments' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pay out xxx now
can confuse a merchant. To them it may seem that they need to pay out, wherease they are actually going to receive the amount. The copy on the button on the main overview page reads as Get xxx now
. That sounds more unambiguous. What about using that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't want to change this without consulting design first – this flow was designed this way. Previously it used deposit
, so pay out
(or maybe payout
) IMO is consistent and clear. We're using payout
to mean "merchant gets money" consistently in the UI, so I would hope that they aren't confused about the meaning here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rogermattic are you ok with the instant payout modal button saying Pay out $nnn now
? Let us know (in team slack channel) if you want to test this in context, i.e. walk through the flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ping!
are you ok with the instant payout modal button saying
Pay out $nnn now
?
I think it sounds right!
Thanks for testing @nagpai – did you test full instant deposit flow? If not, let me know gaps so I can retest. Otherwise we could be good to go 🎉 |
I just tested the complete flow and found that we have this transient notice that would need to be updated too. I think this would be covered within Everything else looks fine! |
Thanks @nagpai ! Based on your testing I think we're good to go here, so removing |
I removed the Let me know if it's blocked from something else. |
@shendy-a8c (or anyone) feel free to merge this (into feature branch) on my behalf! Otherwise I can merge tomorrow morning 🚀 |
Good spotting @nagpai , I added that info to the relevant issue: |
Merging! |
Fixes #9542
to do
Work in progress this is close but untested. My local dev site is not currently eligible for instant payouts.Update: @nagpai has tested full flow, so I think we're good to go here.Changes proposed
Testing instructions
your store needs to be eligible for instant deposits
US
.USD
.4000000000000077
to skip pending period (speeds up TPV requirement).how to test this PR
Payments > Overview
.Get $12 now
should appear on balances card.deposit
and all UI & copy is accurate and helpful 🌞npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge