-
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
Show a deposit schedule notice on the deposits list page #7883
Show a deposit schedule notice on the deposits list page #7883
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: +276 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
@@ -1,7 +1,7 @@ | |||
/** | |||
* Internal dependencies | |||
*/ | |||
import { | |||
import type { |
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 think I asked you this but forgot the reason.
Googled and found https://medium.com/@quizzesforyou/import-vs-import-type-in-typescript-8e5177b62bea
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.
This is my personal preference, I think it is more explicit === improved code readability.
I've now made this notice dismissable by the user in 3ed5c04. It will no show up again unless the user changes their deposit schedule settings. See screen recording Screen.Recording.2023-12-13.at.11.21.29.mov |
34215f7
to
9cec48f
Compare
Co-authored-by: Shendy <[email protected]>
f6135dd
to
ed4a651
Compare
…-zero-available-balance
Co-authored-by: Shendy <[email protected]>
when deposit schedule settings are changed, invalidate dismissal option
Co-authored-by: Timur Karimov <[email protected]>
d4bb460
to
c52f63b
Compare
…nce' into add/7877-deposit-list-next-deposit-notice
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.
Tested and working as expected. LGTM.
Rebased to |
@Jinksi it does look like the history is still muddled |
Yeh it looks like it includes commits from |
Fixes #7877
Based on PR from #7875 since it relies on changes to the
DepositSchedule
component.Changes proposed in this Pull Request
Adds a notice to the Deposits List screen to communicate the deposit schedule for the account. The "next deposit" part of the message is not rendered here since this screen can include multiple currencies with differing next deposit dates.
This notice will only be rendered if deposits are unrestricted (not blocked or on a restricted schedule).
This notice is dismissable by the user. When this occurs, the dismissal state will persist until the user changes their deposit schedule settings.
Screen.Recording.2023-12-13.at.11.21.29.mov
Testing instructions
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