Fix/8175 show tooltip instead of deposit guesstimate #8184
Conversation
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: +40 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
| @@ -119,6 +193,12 @@ const DepositsOverview: React.FC = () => { | |||
| depositsSchedule={ account.deposits_schedule } | |||
| showNextDepositDate={ availableFunds > 0 } | |||
| /> | |||
| <ClickTooltip | |||
| content={ nextDepositHelpContent } | |||
There was a problem hiding this comment.
I really wanted to put the tooltip markup in children, composition style:
<ClickTooltip>
Deposits are initiated based on the following criteria:
<ul>
<li>The pending period in your country</li>
<li>(etc)</li>
</ul>
</ClickTooltip>However, our tooltip uses children for something else – I think it allows the client to use a different element for the button/element that user clicks to show the tooltip.
Would love to refactor our ToolTip and friends to support arbitrary tooltip content as children … though not important to do now!
There was a problem hiding this comment.
…f github.com:Automattic/woocommerce-payments into fix/8175-show-tooltip-instead-of-deposit-guesstimate
Looks like I have some tests to fix 📓 |
I also need to update snapshots. |
|
Also tested with account soft blocked by platform, looking good: 
|
| <ClickTooltip | ||
| content={ nextDepositHelpContent } | ||
| className="wcpay-deposits-overview__schedule__tooltip" | ||
| buttonIcon={ <HelpOutlineIcon /> } | ||
| buttonLabel={ 'Deposit schedule tooltip' } | ||
| /> |
There was a problem hiding this comment.
⛏️ this className may be redundant, AFAICT it isn't references anywhere.
There was a problem hiding this comment.
This tests well @haszari 🙌
I've added some commits with minor code changes in 198303a, c3bd550 and 7de0b05.
I've tested the message against daily, weekly, monthly deposit schedules. ✅
The tooltip renders well and is navigable via keyboard. ✅
I believe this is a rare edge case, but we could bail on rendering the entire schedule message section if check if you think it's worth avoiding for manual scheduled.
💡 A future enhancement may involve adding this tooltip to the Deposits screen, related to #8026.
Thanks @Jinksi for catching these tidy ups!
I like this idea – though pulled back when I realised I need to add the same logic in I'm thinking when we adapt this for #8026 we can move the tooltip inside |
Looks like there's a bunch of unrelated failures breaking the tests. Unrelated to this PR so merging.
These are possibly due to recent release of Woo core 8.6? |
haszari
deleted the
fix/8175-show-tooltip-instead-of-deposit-guesstimate
branch
Co-authored-by: Rua Haszard <rua@automattic.com> Co-authored-by: Eric Jinks <3147296+Jinksi@users.noreply.github.com>
Fixes #8175
Tooltip text is as follows:
Changes proposed in this Pull Request
Testing instructions
Payments > OverviewBonus points:
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
more screenshots
With a monthly schedule: