Instant Deposits: Show amount on a notice and button label #8970
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 9 commits
June 17, 2024 15:45
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: +365 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
|
nagpai
changed the title
haszari
reviewed
Jun 19, 2024
There was a problem hiding this comment.
Based on a code review this is looking good. I didn't test because my dev is not setup for instant deposits. Happy to do another test review later if needed (I can test on a different site).
There's one blocker: the notice dismissal should be persisted to user options. Look in wcpaySettings for similar examples, e.g. isNextDepositNoticeDismissed (aka wcpay_next_deposit_notice_dismissed in database).
Whether we need a #8220 (comment) prevent confusion.
The current wording was approved earlier (IIRC!) so hopefully we don't get bogged down 😁
That said, it looks like we have agreement from @rogermattic and @aheckler 🎉 so I recommend going with that new wording. If there is some technical problem with new wording, maybe we can push ahead with the previously approved wording.
New wording:
Get $239.66 via instant deposit. Funds are typically in your bank account within 30 minutes. Fee: 1.5%.
Get $239.66 now
| import HelpOutlineIcon from 'gridicons/dist/help-outline'; | ||
| import InlineNotice from '../inline-notice'; | ||
| import InstantDepositButton from 'deposits/instant-deposits'; | ||
| import SendMoneyIcon from 'assets/images/icons/send-money.svg?asset'; |
There was a problem hiding this comment.
Why the ?asset – what is the purpose of this? I haven't seen it before.
There was a problem hiding this comment.
I see this is used throughout WooPayments for SVGs, assuming this a way to load them as asset files at runtime. Asked for clarification (so I understand) in front end guild channel internally.
There was a problem hiding this comment.
I first picked up the pattern from how SVGs were embedded earlier, and then read about it. I hope I am right about this: ?asset invokes the asset module of Webpack. Ref. It bundles the SVG file to a base 64 URI, that can be easily used in HTML.
Another helpful read I found.
| Significance: patch | ||
| Type: fix | ||
|
|
||
| Clearly display available instant deposit amount on notice and button label on Payment Overview page |
There was a problem hiding this comment.
Nice clear merchant-relevant changelog 👌🏻
brucealdridge
approved these changes
Jun 20, 2024
There was a problem hiding this comment.
Tested and works well. Glad to see this issue progress!
Good work @nagpai
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8220
The PR implements a notice, updated button text and a tooltip for
Instant depositson theOverviewpage. The UI update will clearly inform the merchant of the exact amount available for instant deposit.Screenshots
With notice
After notice is dismissed
Tooltip
Testing instructions
acct_1LqHU4QocNIKfAAZ. This account is approved for instant deposit and should show you the notice and button.With notice. Note that there is no tooltip icon visibleTooltipLearn morelink should lead to - https://woocommerce.com/document/woopayments/deposits/instant-deposits/wcpay_instant_deposit_notice_dismissedfrom the client test site DB.Points of discussion / decision needed
These are minor changes and can be easily actioned depending on the outcome of the discussion.
Whether we need a change in the text to prevent confusion.Opinion needed - Not directly related, and minor test refactorHow to persist notice dismissal better - 1718694806082599-slack-C02BW3Z8SHKDev notes
npm run changelogto add a changelog file, choosepatchto 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