add right-click to delete ledger entry, add Bravery site settings page #4118
Conversation
Test plan: 1. go to about:preferences#security 2. right click on a site and select 'never include this site' 3. make sure the entry disappears from the table Fix #3829
Fix #2545 Test Plan: 1. browse to some sites and change the Brave settings on them 2. go to about:preferences#shields, and verify that the changed settings appear there 3. change the bravery global defaults and verify that the changed settings disappear
| @@ -111,6 +111,18 @@ const doAction = (action) => { | |||
| if (action.key === settings.PAYMENTS_CONTRIBUTION_AMOUNT) return setPaymentInfo(action.value) | |||
| break | |||
|
|
|||
| case appConstants.APP_CHANGE_SITE_SETTING: | |||
| if (action.key === 'ledgerPaymentsShown') { | |||
| // TODO | |||
There was a problem hiding this comment.
@mrose17 if action.value === false then the user has right-clicked to delete a ledger synopsis entry
|
|
||
| case appConstants.APP_REMOVE_SITE_SETTING: | ||
| if (action.key === 'ledgerPaymentsShown') { | ||
| // TODO |
There was a problem hiding this comment.
@mrose17 this event indicates that a deleted ledger synopsis entry has been un-deleted so it should be included again
| .waitUntil(function () { | ||
| return this.getText('div').then((val) => val === 'done') | ||
| }) | ||
| .windowByUrl(Brave.browserWindowUrl) |
There was a problem hiding this comment.
@mrose17 this test seems to not pass because the ledger is still requiring us to be on the page for 8 seconds even though process.env.NODE_ENV === 'test'. i.e., if you add a .debug() here and wait for at least 8 seconds, then the page shows up on about:preferences as expected.
| @@ -111,6 +111,18 @@ const doAction = (action) => { | |||
| if (action.key === settings.PAYMENTS_CONTRIBUTION_AMOUNT) return setPaymentInfo(action.value) | |||
| break | |||
|
|
|||
| case appConstants.APP_CHANGE_SITE_SETTING: | |||
| if (action.key === 'ledgerPaymentsShown') { | |||
| // TODO | |||
| @@ -967,7 +979,9 @@ var run = (delayTime) => { | |||
| var result | |||
| var siteSetting = siteSettings.get(`https?://${winner}`) | |||
|
|
|||
| if ((siteSetting) && (siteSetting.get('ledgerPayments') === false)) return | |||
| if ((siteSetting) && | |||
There was a problem hiding this comment.
i don't think this change is needed given my change. do you agree?
There was a problem hiding this comment.
pls do in an audit, will merge so we can get it in builds.
There was a problem hiding this comment.
@bbondy - just so i'm on the same page: the change here doesn't break anything, i merely believe that it is unnecessary. there's no harm in keeping it...
git rebase -ito squash commits (if needed).Fix #2545
Test Plan:
Fix #3829
Test plan:
Auditors: @bbondy @mrose17