Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Add property for global non-ledger notificationBar
Browse files Browse the repository at this point in the history
Fix #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.
  • Loading branch information
diracdeltas committed Dec 19, 2017
1 parent d621440 commit 6a3142d
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 31 deletions.
17 changes: 7 additions & 10 deletions app/browser/api/ledgerNotifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
}
Expand All @@ -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: [
Expand All @@ -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: [
Expand All @@ -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: [
Expand All @@ -331,7 +328,7 @@ const showBraveWalletUpdated = () => {
appActions.onBitcoinToBatNotified()

appActions.showNotification({
from: 'ledger',
position: 'global',
greeting: text.hello,
message: text.walletConvertedToBat,
// Learn More.
Expand Down Expand Up @@ -373,7 +370,7 @@ const showPromotionNotification = (state) => {
}

const data = notification.toJS()
data.from = 'ledger'
data.position = 'global'

appActions.showNotification(data)
}
Expand Down
1 change: 1 addition & 0 deletions app/browser/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ const api = {
})

appActions.showNotification({
position: 'global',
buttons: [
{text: locale.translation('ok')}
],
Expand Down
8 changes: 4 additions & 4 deletions app/common/state/notificationBarState.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
}

Expand Down
1 change: 1 addition & 0 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ app.on('ready', () => {
options: {
persist: false
},
position: 'global',
message
})
prefsRestartCallbacks[message] = (buttonIndex, persist) => {
Expand Down
9 changes: 2 additions & 7 deletions app/renderer/components/main/notificationBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion test/unit/app/browser/api/ledgerNotificationsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
18 changes: 9 additions & 9 deletions test/unit/app/common/state/notificationBarStateTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down

0 comments on commit 6a3142d

Please sign in to comment.