From 6a3142d3879a0e6ee32c6606bb297a407fc5d80b Mon Sep 17 00:00:00 2001 From: yan Date: Tue, 19 Dec 2017 20:11:40 +0000 Subject: [PATCH] Add property for global non-ledger notificationBar Fix https://github.com/brave/browser-laptop/issues/12216 Test Plan: 1. check that there are no new notificationBar test failures in travis 2. go to about:preferences 3. change PDF.JS setting. the 'Restart now?' notification should appear above the tabs bar. --- app/browser/api/ledgerNotifications.js | 17 +++++++---------- app/browser/windows.js | 1 + app/common/state/notificationBarState.js | 8 ++++---- app/index.js | 1 + .../components/main/notificationBar.js | 9 ++------- .../app/browser/api/ledgerNotificationsTest.js | 2 +- .../common/state/notificationBarStateTest.js | 18 +++++++++--------- 7 files changed, 25 insertions(+), 31 deletions(-) diff --git a/app/browser/api/ledgerNotifications.js b/app/browser/api/ledgerNotifications.js index 4d81add922b..18d32630150 100644 --- a/app/browser/api/ledgerNotifications.js +++ b/app/browser/api/ledgerNotifications.js @@ -260,17 +260,14 @@ const showDisabledNotifications = (state) => { } appActions.showNotification({ - from: 'ledger', + position: 'global', greeting: locale.translation('updateHello'), message: text.tryPayments, buttons: [ {text: locale.translation('noThanks')}, {text: locale.translation('notificationTryPaymentsYes'), className: 'primaryButton'} ], - options: { - style: 'greetingStyle', - persist: false - } + options: displayOptions }) } } @@ -279,7 +276,7 @@ const showReviewPublishers = (nextTime) => { appActions.changeSetting(settings.PAYMENTS_NOTIFICATION_RECONCILE_SOON_TIMESTAMP, nextTime) appActions.showNotification({ - from: 'ledger', + position: 'global', greeting: text.hello, message: text.reconciliation, buttons: [ @@ -296,7 +293,7 @@ const showAddFunds = () => { appActions.changeSetting(settings.PAYMENTS_NOTIFICATION_ADD_FUNDS_TIMESTAMP, nextTime) appActions.showNotification({ - from: 'ledger', + position: 'global', greeting: text.hello, message: text.addFunds, buttons: [ @@ -316,7 +313,7 @@ const showPaymentDone = (transactionContributionFiat) => { // Hide the 'waiting for deposit' message box if it exists appActions.hideNotification(text.addFunds) appActions.showNotification({ - from: 'ledger', + position: 'global', greeting: locale.translation('updateHello'), message: text.paymentDone, buttons: [ @@ -331,7 +328,7 @@ const showBraveWalletUpdated = () => { appActions.onBitcoinToBatNotified() appActions.showNotification({ - from: 'ledger', + position: 'global', greeting: text.hello, message: text.walletConvertedToBat, // Learn More. @@ -373,7 +370,7 @@ const showPromotionNotification = (state) => { } const data = notification.toJS() - data.from = 'ledger' + data.position = 'global' appActions.showNotification(data) } diff --git a/app/browser/windows.js b/app/browser/windows.js index 1e28fdfe394..07eb8577990 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -209,6 +209,7 @@ const api = { }) appActions.showNotification({ + position: 'global', buttons: [ {text: locale.translation('ok')} ], diff --git a/app/common/state/notificationBarState.js b/app/common/state/notificationBarState.js index cd4504aa2ae..7c951b6d79e 100644 --- a/app/common/state/notificationBarState.js +++ b/app/common/state/notificationBarState.js @@ -26,13 +26,13 @@ const notificationBarState = { }, /** - * Gets an immutable list of ledger notifications + * Gets an immutable list of global notifications (shown above tab bar) * @param {Map} appState - The app state object - * @return {List} - immutable list of ledger notifications + * @return {List} - immutable list of global notifications */ - getLedgerNotifications: (state) => { + getGlobalNotifications: (state) => { const notifications = notificationBarState.getNotifications(state) - return notifications.filter(item => item.get('from') === 'ledger') + return notifications.filter(item => item.get('position') === 'global') } } diff --git a/app/index.js b/app/index.js index 3832c7345d6..f1cd23a15a9 100644 --- a/app/index.js +++ b/app/index.js @@ -255,6 +255,7 @@ app.on('ready', () => { options: { persist: false }, + position: 'global', message }) prefsRestartCallbacks[message] = (buttonIndex, persist) => { diff --git a/app/renderer/components/main/notificationBar.js b/app/renderer/components/main/notificationBar.js index fdc1023eae5..19cd4217040 100644 --- a/app/renderer/components/main/notificationBar.js +++ b/app/renderer/components/main/notificationBar.js @@ -31,10 +31,7 @@ class NotificationBar extends React.Component { .filter((item) => { return item.get('frameOrigin') ? activeOrigin === item.get('frameOrigin') - // TODO: this should filter all cases - // for notifications that are not per-tab - // and not only ledger - : item.get('from') !== 'ledger' + : item.get('position') !== 'global' }) .takeLast(3) props.showNotifications = !props.activeNotifications.isEmpty() @@ -61,9 +58,7 @@ class NotificationBar extends React.Component { class BraveNotificationBar extends React.Component { mergeProps (state, ownProps) { const props = {} - // TODO: for now we cover only ledger notifications in this method - // this should cover all notifications that are global - props.notifications = notificationBarState.getLedgerNotifications(state) + props.notifications = notificationBarState.getGlobalNotifications(state) props.showNotifications = !props.notifications.isEmpty() return props } diff --git a/test/unit/app/browser/api/ledgerNotificationsTest.js b/test/unit/app/browser/api/ledgerNotificationsTest.js index a2314eb3185..4fc0a2ba64a 100644 --- a/test/unit/app/browser/api/ledgerNotificationsTest.js +++ b/test/unit/app/browser/api/ledgerNotificationsTest.js @@ -599,7 +599,7 @@ describe('ledgerNotifications unit test', function () { it('we set global notification', function () { const notification = state .getIn(['ledger', 'promotion', 'stateWallet', 'disabledWallet', 'notification']) - .set('from', 'ledger') + .set('position', 'global') ledgerNotificationsApi.showPromotionNotification(state) assert(showNotificationSpy.withArgs(notification.toJS()).calledOnce) }) diff --git a/test/unit/app/common/state/notificationBarStateTest.js b/test/unit/app/common/state/notificationBarStateTest.js index acf86c7c593..862c01ab089 100644 --- a/test/unit/app/common/state/notificationBarStateTest.js +++ b/test/unit/app/common/state/notificationBarStateTest.js @@ -52,32 +52,32 @@ describe('notificationBarState test', function () { }) }) - describe('getLedgerNotifications', function () { + describe('getGlobalNotifications', function () { it('returns a list of ledger-related notifications', function () { const notificationsList = Immutable.fromJS([ - { from: 'ledger', message: 'HELLO LEDGER' }, - { from: 'ledger', message: 'HELLO YOU' } + { position: 'global', message: 'HELLO LEDGER' }, + { position: 'global', message: 'HELLO YOU' } ]) state = state.mergeIn(['notifications'], notificationsList) - const result = notificationBarState.getLedgerNotifications(state) + const result = notificationBarState.getGlobalNotifications(state) assert.equal(result.size, 2) }) it('does not show notifications that are not ledger-related', function () { const notificationsList = Immutable.fromJS([ - { from: 'ledger', message: 'HELLO LEDGER' }, - { from: 'other place', message: 'HELLO YOU FROM OUTWHERE' } + { position: 'global', message: 'HELLO LEDGER' }, + { position: 'other place', message: 'HELLO YOU FROM OUTWHERE' } ]) state = state.mergeIn(['notifications'], notificationsList) - const result = notificationBarState.getLedgerNotifications(state) + const result = notificationBarState.getGlobalNotifications(state) assert.equal(result.size, 1) }) it('returns an empty Immutable list if no ledger-related notification is found', function () { state = state.mergeIn(['notifications'], [ - { from: 'Brazil', message: 'Hello from Brazil!' } + { position: 'Brazil', message: 'Hello from Brazil!' } ]) - const result = notificationBarState.getLedgerNotifications(state) + const result = notificationBarState.getGlobalNotifications(state) assert.equal(result.isEmpty(), true) assert.equal(isList(result), true) })