"Add funds" notification: Reduce nag level and add button to "Turn off notifications" #5469
Conversation
|
I am worried that this new button will be the first choice for most people and the result is a drop off in re-ups for month 2. If we don’t want to do the work based on my suggestion, then let’s at least order the buttons so the CTA remains at the right side. [TON] [Later] [Add Funds]
|
|
Is it our convention to have primary buttons at the rightmost side? I put it on the left because I'd assumed it was reading order / left to right. RE Drop off: I think nagging people every month currently doesn't lead to much re-upping, so removing notifications would lose just a small number of users. Also, nagging annoys users. I feel auto monthly reload would fix these problems. |
|
Agree if someone wants to turn off notifications, then let them. They can go in preferences if they want it. We don't want to nag people in this way. Not every hour, not every 6 hours, not every day, not every month. |
|
I think the convention is that default button is rightmost and I think we should be consistent with it. |
|
Some examples of button order.
|
|
Nag them we should not. Not in a boat. Not with a goat. And yes, button order should be flipped. I wish the notify-off button text was shorter but I don't have a better solution at the moment.
|
Related: #5418 Auditors: @bsclifton Test Plan: 1. Trigger the "Add funds" notification. - Update reconcileStamp to <24 hours from now (or a few days in the past) - Ensure not enough balance 2. Change startup notification delay in ledger.js L484 from 15m to 5s 3. Open Brave and observe notification 4. Close Brave and reopen. Notification should not reappear. (Next Add funds notification timestamp is set in Application Support/brave-development/session-store-1 `notification-add-funds-timestamp`)
Fix #5418 Auditors: @bsclifton Test plan: 1. Trigger "Add funds" notification by having Payments enabled and editing session-store-1 reconcileDate to in the past 2. Observe notification with new "Turn off notification" button. 3. Click it 4. Open Preferences > Payments and click Advanced Settings... and observe notifications are disabled.
ayumi
force-pushed
the
fix/payments-add-funds-quiet
branch
from
1c2d16b
to
aa08129
Compare
|
Heads up (coming soon by @jkup !) We will be updating the styles throughout the app to be consistent 😄 EX: these flat buttons Versus the fancy 🎩 |
| @@ -320,12 +320,13 @@ if (ipc) { | |||
| const win = electron.BrowserWindow.getFocusedWindow() | |||
| if (message === addFundsMessage) { | |||
| appActions.hideMessageBox(message) | |||
| // See showNotificationAddFunds() for buttons. | |||
| // buttonIndex === 1 is "Later"; the timestamp until which to delay is set | |||
| // in showNotificationAddFunds() when triggering this notification. | |||
| if (buttonIndex === 0) { | |||
There was a problem hiding this comment.
These indexes (indices?) would be great as constants, IMO. Instead of checking against 0, it could look like:
if (buttonIndex === ledgerButton.Later) {
There was a problem hiding this comment.
I like your comment, i think it'd make the code easier to follow.. maybe button indexes should be replaced by button values or something .. like if buttonValue === ledgerButton.later
The index shouldn't matter, just the button action. This would also handle cases where button sets don't contain Later or Turn off because they're not applicable.
|
Dismissing (or choosing to turn off notifications) sets the timestamp as 3 days in the future. So I can re-enable if I change my mind afterwards and I'll be prompted again 3 days from when I chose hide (effectively same as dismiss) Tested locally and it works great; merging 😄 |
Fix #5418
Fix #5299
Auditors: @bsclifton
Test Plan / Quieter notification:
a. Update reconcileStamp to <24 hours from now (or a few days in the past)
b. Ensure not enough balance
(Next Add funds notification timestamp is set in Application Support/brave-development/session-store-1
notification-add-funds-timestamp)Test Plan / Turn off button:
5. Trigger the Add funds notification again by closing Brave and editing session-store-1
notification-add-funds-timestampto the past.6. Open Brave, and when the notification appears choose Turn off.
7. Open Preferences > Payments and click Advanced Settings... and observe notifications are disabled.
Image / Before
Image / After
Image 2 / After
cc @bbondy @bradleyrichter