From 5479509618e601391586d0851acee4e408523c4f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 16 Aug 2016 15:39:40 -0700 Subject: [PATCH 01/14] Set up MVP for popup-based notifications. --- app/notification.html | 11 +++ app/scripts/background.js | 2 +- app/scripts/lib/notifications.js | 138 +++---------------------------- app/scripts/notification.js | 85 +++++++++++++++++++ gulpfile.js | 5 +- ui/notification.js | 52 ++++++++++++ 6 files changed, 162 insertions(+), 131 deletions(-) create mode 100644 app/notification.html create mode 100644 app/scripts/notification.js create mode 100644 ui/notification.js diff --git a/app/notification.html b/app/notification.html new file mode 100644 index 000000000..927f1634c --- /dev/null +++ b/app/notification.html @@ -0,0 +1,11 @@ + + + + + MetaMask Notification + + +
+ + + diff --git a/app/scripts/background.js b/app/scripts/background.js index e04309e74..79f8f9fd9 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -69,7 +69,7 @@ function showUnconfirmedTx (txParams, txData, onTxDoneCb) { extension.runtime.onConnect.addListener(connectRemote) function connectRemote (remotePort) { - var isMetaMaskInternalProcess = (remotePort.name === 'popup') + var isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' var portStream = new PortStream(remotePort) if (isMetaMaskInternalProcess) { // communication with popup diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index 6c1601df1..75fb60dd0 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -18,142 +18,24 @@ const notifications = { module.exports = notifications window.METAMASK_NOTIFIER = notifications -setupListeners() - -function setupListeners () { - // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!extension.notifications) return console.error('Chrome notifications API missing...') - - // notification button press - extension.notifications.onButtonClicked.addListener(function (notificationId, buttonIndex) { - var handlers = notificationHandlers[notificationId] - if (buttonIndex === 0) { - handlers.confirm() - } else { - handlers.cancel() - } - extension.notifications.clear(notificationId) - }) - - // notification teardown - extension.notifications.onClosed.addListener(function (notificationId) { - delete notificationHandlers[notificationId] - }) -} - -// creation helper function createUnlockRequestNotification (opts) { - // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!extension.notifications) return console.error('Chrome notifications API missing...') - var message = 'An Ethereum app has requested a signature. Please unlock your account.' - - var id = createId() - extension.notifications.create(id, { - type: 'basic', - iconUrl: '/images/icon-128.png', - title: opts.title, - message: message, - }) + showNotification() } function createTxNotification (state) { - // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!extension.notifications) return console.error('Chrome notifications API missing...') - - renderTxNotificationSVG(state, function (err, notificationSvgSource) { - if (err) throw err - - showNotification(extend(state, { - title: 'New Unsigned Transaction', - imageUrl: toSvgUri(notificationSvgSource), - })) - }) + showNotification() } function createMsgNotification (state) { - // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!extension.notifications) return console.error('Chrome notifications API missing...') + showNotification() +} - renderMsgNotificationSVG(state, function (err, notificationSvgSource) { - if (err) throw err - - showNotification(extend(state, { - title: 'New Unsigned Message', - imageUrl: toSvgUri(notificationSvgSource), - })) +function showNotification() { + chrome.windows.create({ + url:"notification.html", + type:"panel", + width:360, + height:500, }) } -function showNotification (state) { - // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!extension.notifications) return console.error('Chrome notifications API missing...') - - var id = createId() - extension.notifications.create(id, { - type: 'image', - requireInteraction: true, - iconUrl: '/images/icon-128.png', - imageUrl: state.imageUrl, - title: state.title, - message: '', - buttons: [{ - title: 'Approve', - }, { - title: 'Reject', - }], - }) - notificationHandlers[id] = { - confirm: state.onConfirm, - cancel: state.onCancel, - } -} - -function renderTxNotificationSVG (state, cb) { - var content = h(PendingTxDetails, state) - renderNotificationSVG(content, cb) -} - -function renderMsgNotificationSVG (state, cb) { - var content = h(PendingMsgDetails, state) - renderNotificationSVG(content, cb) -} - -function renderNotificationSVG (content, cb) { - var container = document.createElement('div') - var confirmView = h('div.app-primary', { - style: { - width: '360px', - height: '240px', - padding: '16px', - // background: '#F7F7F7', - background: 'white', - }, - }, [ - h('style', MetaMaskUiCss()), - content, - ]) - - render(confirmView, container, function ready() { - var rootElement = findDOMNode(this) - var viewSource = rootElement.outerHTML - unmountComponentAtNode(container) - var svgSource = svgWrapper(viewSource) - // insert content into svg wrapper - cb(null, svgSource) - }) -} - -function svgWrapper (content) { - var wrapperSource = ` - - - {{content}} - - - ` - return wrapperSource.split('{{content}}').join(content) -} - -function toSvgUri (content) { - return 'data:image/svg+xml;utf8,' + encodeURIComponent(content) -} diff --git a/app/scripts/notification.js b/app/scripts/notification.js new file mode 100644 index 000000000..90c00b32d --- /dev/null +++ b/app/scripts/notification.js @@ -0,0 +1,85 @@ +const url = require('url') +const EventEmitter = require('events').EventEmitter +const async = require('async') +const Dnode = require('dnode') +const Web3 = require('web3') +const MetaMaskNotification = require('../../ui/notification') +const MetaMaskUiCss = require('../../ui/css') +const injectCss = require('inject-css') +const PortStream = require('./lib/port-stream.js') +const StreamProvider = require('web3-stream-provider') +const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex +const extension = require('./lib/extension') + +// setup app +var css = MetaMaskUiCss() +injectCss(css) + +async.parallel({ + currentDomain: getCurrentDomain, + accountManager: connectToAccountManager, +}, setupApp) + +function connectToAccountManager (cb) { + // setup communication with background + var pluginPort = extension.runtime.connect({name: 'notification'}) + var portStream = new PortStream(pluginPort) + // setup multiplexing + var mx = setupMultiplex(portStream) + // connect features + setupControllerConnection(mx.createStream('controller'), cb) + setupWeb3Connection(mx.createStream('provider')) +} + +function setupWeb3Connection (stream) { + var remoteProvider = new StreamProvider() + remoteProvider.pipe(stream).pipe(remoteProvider) + stream.on('error', console.error.bind(console)) + remoteProvider.on('error', console.error.bind(console)) + global.web3 = new Web3(remoteProvider) +} + +function setupControllerConnection (stream, cb) { + var eventEmitter = new EventEmitter() + var background = Dnode({ + sendUpdate: function (state) { + eventEmitter.emit('update', state) + }, + }) + stream.pipe(background).pipe(stream) + background.once('remote', function (accountManager) { + // setup push events + accountManager.on = eventEmitter.on.bind(eventEmitter) + cb(null, accountManager) + }) +} + +function getCurrentDomain (cb) { + const unknown = '' + if (!extension.tabs) return cb(null, unknown) + extension.tabs.query({active: true, currentWindow: true}, function (results) { + var activeTab = results[0] + var currentUrl = activeTab && activeTab.url + var currentDomain = url.parse(currentUrl).host + if (!currentUrl) { + return cb(null, unknown) + } + cb(null, currentDomain) + }) +} + +function setupApp (err, opts) { + if (err) { + alert(err.stack) + throw err + } + + var container = document.getElementById('app-content') + + MetaMaskNotification({ + container: container, + accountManager: opts.accountManager, + currentDomain: opts.currentDomain, + networkVersion: opts.networkVersion, + }) +} diff --git a/gulpfile.js b/gulpfile.js index aeaf3e674..48f30835b 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -108,6 +108,7 @@ const jsFiles = [ 'contentscript', 'background', 'popup', + 'notification', ] jsFiles.forEach((jsFile) => { @@ -115,9 +116,9 @@ jsFiles.forEach((jsFile) => { gulp.task(`build:js:${jsFile}`, bundleTask({ watch: false, filename: `${jsFile}.js` })) }) -gulp.task('dev:js', gulp.parallel('dev:js:inpage','dev:js:contentscript','dev:js:background','dev:js:popup')) +gulp.task('dev:js', gulp.parallel('dev:js:inpage','dev:js:contentscript','dev:js:background','dev:js:popup', 'dev:js:notification')) -gulp.task('build:js', gulp.parallel('build:js:inpage','build:js:contentscript','build:js:background','build:js:popup')) +gulp.task('build:js', gulp.parallel('build:js:inpage','build:js:contentscript','build:js:background','build:js:popup', 'dev:js:notification')) // clean dist diff --git a/ui/notification.js b/ui/notification.js new file mode 100644 index 000000000..8cf74f6ee --- /dev/null +++ b/ui/notification.js @@ -0,0 +1,52 @@ +const render = require('react-dom').render +const h = require('react-hyperscript') +const Root = require('./app/root') +const actions = require('./app/actions') +const configureStore = require('./app/store') + +module.exports = launchApp + +function launchApp (opts) { + var accountManager = opts.accountManager + actions._setAccountManager(accountManager) + + // check if we are unlocked first + accountManager.getState(function (err, metamaskState) { + if (err) throw err + startApp(metamaskState, accountManager, opts) + }) +} + +function startApp (metamaskState, accountManager, opts) { + // parse opts + var store = configureStore({ + + // metamaskState represents the cross-tab state + metamask: metamaskState, + + // appState represents the current tab's popup state + appState: { + currentDomain: opts.currentDomain, + }, + + // Which blockchain we are using: + networkVersion: opts.networkVersion, + }) + + // if unconfirmed txs, start on txConf page + if (Object.keys(metamaskState.unconfTxs || {}).length) { + store.dispatch(actions.showConfTxPage()) + } + + accountManager.on('update', function (metamaskState) { + store.dispatch(actions.updateMetamaskState(metamaskState)) + }) + + // start app + render( + h(Root, { + // inject initial state + store: store, + } + ), opts.container) +} From 030bdec27a95390207b9147c95b810893756db6d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 16 Aug 2016 16:46:44 -0700 Subject: [PATCH 02/14] Unify notification and popup ui files --- app/notification.html | 7 ++- app/scripts/lib/extension-instance.js | 6 +++ app/scripts/lib/is-popup-or-notification.js | 8 ++++ app/scripts/lib/notifications.js | 2 +- app/scripts/popup.js | 7 ++- gulpfile.js | 5 +- ui/notification.js | 52 --------------------- 7 files changed, 29 insertions(+), 58 deletions(-) create mode 100644 app/scripts/lib/is-popup-or-notification.js delete mode 100644 ui/notification.js diff --git a/app/notification.html b/app/notification.html index 927f1634c..cc485da7f 100644 --- a/app/notification.html +++ b/app/notification.html @@ -3,9 +3,14 @@ MetaMask Notification +
- + diff --git a/app/scripts/lib/extension-instance.js b/app/scripts/lib/extension-instance.js index eb3b8a1e9..d284895bc 100644 --- a/app/scripts/lib/extension-instance.js +++ b/app/scripts/lib/extension-instance.js @@ -41,6 +41,12 @@ function Extension () { } } catch (e) {} + try { + if (browser[api]) { + _this[api] = browser[api] + } + } + try { _this.api = browser.extension[api] } catch (e) {} diff --git a/app/scripts/lib/is-popup-or-notification.js b/app/scripts/lib/is-popup-or-notification.js new file mode 100644 index 000000000..5c38ac823 --- /dev/null +++ b/app/scripts/lib/is-popup-or-notification.js @@ -0,0 +1,8 @@ +module.exports = function isPopupOrNotification() { + const url = window.location.href + if (url.match(/popup.html$/)) { + return 'popup' + } else { + return 'notification' + } +} diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index 75fb60dd0..e6bdeff09 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -31,7 +31,7 @@ function createMsgNotification (state) { } function showNotification() { - chrome.windows.create({ + extension.windows.create({ url:"notification.html", type:"panel", width:360, diff --git a/app/scripts/popup.js b/app/scripts/popup.js index 20be15df7..4c729e227 100644 --- a/app/scripts/popup.js +++ b/app/scripts/popup.js @@ -9,6 +9,7 @@ const injectCss = require('inject-css') const PortStream = require('./lib/port-stream.js') const StreamProvider = require('web3-stream-provider') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex +const isPopupOrNotification = require('./lib/is-popup-or-notification') const extension = require('./lib/extension') // setup app @@ -22,7 +23,10 @@ async.parallel({ function connectToAccountManager (cb) { // setup communication with background - var pluginPort = extension.runtime.connect({name: 'popup'}) + + var name = isPopupOrNotification() + window.METAMASK_UI_TYPE = name + var pluginPort = extension.runtime.connect({ name }) var portStream = new PortStream(pluginPort) // setup multiplexing var mx = setupMultiplex(portStream) @@ -93,3 +97,4 @@ function setupApp (err, opts) { networkVersion: opts.networkVersion, }) } + diff --git a/gulpfile.js b/gulpfile.js index 48f30835b..aeaf3e674 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -108,7 +108,6 @@ const jsFiles = [ 'contentscript', 'background', 'popup', - 'notification', ] jsFiles.forEach((jsFile) => { @@ -116,9 +115,9 @@ jsFiles.forEach((jsFile) => { gulp.task(`build:js:${jsFile}`, bundleTask({ watch: false, filename: `${jsFile}.js` })) }) -gulp.task('dev:js', gulp.parallel('dev:js:inpage','dev:js:contentscript','dev:js:background','dev:js:popup', 'dev:js:notification')) +gulp.task('dev:js', gulp.parallel('dev:js:inpage','dev:js:contentscript','dev:js:background','dev:js:popup')) -gulp.task('build:js', gulp.parallel('build:js:inpage','build:js:contentscript','build:js:background','build:js:popup', 'dev:js:notification')) +gulp.task('build:js', gulp.parallel('build:js:inpage','build:js:contentscript','build:js:background','build:js:popup')) // clean dist diff --git a/ui/notification.js b/ui/notification.js deleted file mode 100644 index 8cf74f6ee..000000000 --- a/ui/notification.js +++ /dev/null @@ -1,52 +0,0 @@ -const render = require('react-dom').render -const h = require('react-hyperscript') -const Root = require('./app/root') -const actions = require('./app/actions') -const configureStore = require('./app/store') - -module.exports = launchApp - -function launchApp (opts) { - var accountManager = opts.accountManager - actions._setAccountManager(accountManager) - - // check if we are unlocked first - accountManager.getState(function (err, metamaskState) { - if (err) throw err - startApp(metamaskState, accountManager, opts) - }) -} - -function startApp (metamaskState, accountManager, opts) { - // parse opts - var store = configureStore({ - - // metamaskState represents the cross-tab state - metamask: metamaskState, - - // appState represents the current tab's popup state - appState: { - currentDomain: opts.currentDomain, - }, - - // Which blockchain we are using: - networkVersion: opts.networkVersion, - }) - - // if unconfirmed txs, start on txConf page - if (Object.keys(metamaskState.unconfTxs || {}).length) { - store.dispatch(actions.showConfTxPage()) - } - - accountManager.on('update', function (metamaskState) { - store.dispatch(actions.updateMetamaskState(metamaskState)) - }) - - // start app - render( - h(Root, { - // inject initial state - store: store, - } - ), opts.container) -} From a167bbc5a0f29568ec8e53ecdd942724aa15604b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 16 Aug 2016 17:32:54 -0700 Subject: [PATCH 03/14] MVP Popup Notifications Working I'm unsure which will be more performant: A notification using a trimmed down version of the UI, or using them both, letting the browser cache them both. In any case, here I've modified the normal UI to recognize when it's a popup, and change the UX accordingly in a few ways: - Hide the menu bar - Hide the back button from the notifications view. - When confirming the last tx, close the window. --- app/scripts/lib/extension-instance.js | 2 +- app/scripts/lib/notifications.js | 3 +- app/scripts/notification.js | 85 --------------------------- ui/app/app.js | 5 ++ ui/app/conf-tx.js | 5 +- ui/app/reducers/app.js | 12 ++++ 6 files changed, 23 insertions(+), 89 deletions(-) delete mode 100644 app/scripts/notification.js diff --git a/app/scripts/lib/extension-instance.js b/app/scripts/lib/extension-instance.js index d284895bc..1098130e3 100644 --- a/app/scripts/lib/extension-instance.js +++ b/app/scripts/lib/extension-instance.js @@ -45,7 +45,7 @@ function Extension () { if (browser[api]) { _this[api] = browser[api] } - } + } catch (e) {} try { _this.api = browser.extension[api] diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index e6bdeff09..de9cf26e3 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -33,7 +33,8 @@ function createMsgNotification (state) { function showNotification() { extension.windows.create({ url:"notification.html", - type:"panel", + type:"detached_panel", + focused:true, width:360, height:500, }) diff --git a/app/scripts/notification.js b/app/scripts/notification.js deleted file mode 100644 index 90c00b32d..000000000 --- a/app/scripts/notification.js +++ /dev/null @@ -1,85 +0,0 @@ -const url = require('url') -const EventEmitter = require('events').EventEmitter -const async = require('async') -const Dnode = require('dnode') -const Web3 = require('web3') -const MetaMaskNotification = require('../../ui/notification') -const MetaMaskUiCss = require('../../ui/css') -const injectCss = require('inject-css') -const PortStream = require('./lib/port-stream.js') -const StreamProvider = require('web3-stream-provider') -const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex -const extension = require('./lib/extension') - -// setup app -var css = MetaMaskUiCss() -injectCss(css) - -async.parallel({ - currentDomain: getCurrentDomain, - accountManager: connectToAccountManager, -}, setupApp) - -function connectToAccountManager (cb) { - // setup communication with background - var pluginPort = extension.runtime.connect({name: 'notification'}) - var portStream = new PortStream(pluginPort) - // setup multiplexing - var mx = setupMultiplex(portStream) - // connect features - setupControllerConnection(mx.createStream('controller'), cb) - setupWeb3Connection(mx.createStream('provider')) -} - -function setupWeb3Connection (stream) { - var remoteProvider = new StreamProvider() - remoteProvider.pipe(stream).pipe(remoteProvider) - stream.on('error', console.error.bind(console)) - remoteProvider.on('error', console.error.bind(console)) - global.web3 = new Web3(remoteProvider) -} - -function setupControllerConnection (stream, cb) { - var eventEmitter = new EventEmitter() - var background = Dnode({ - sendUpdate: function (state) { - eventEmitter.emit('update', state) - }, - }) - stream.pipe(background).pipe(stream) - background.once('remote', function (accountManager) { - // setup push events - accountManager.on = eventEmitter.on.bind(eventEmitter) - cb(null, accountManager) - }) -} - -function getCurrentDomain (cb) { - const unknown = '' - if (!extension.tabs) return cb(null, unknown) - extension.tabs.query({active: true, currentWindow: true}, function (results) { - var activeTab = results[0] - var currentUrl = activeTab && activeTab.url - var currentDomain = url.parse(currentUrl).host - if (!currentUrl) { - return cb(null, unknown) - } - cb(null, currentDomain) - }) -} - -function setupApp (err, opts) { - if (err) { - alert(err.stack) - throw err - } - - var container = document.getElementById('app-content') - - MetaMaskNotification({ - container: container, - accountManager: opts.accountManager, - currentDomain: opts.currentDomain, - networkVersion: opts.networkVersion, - }) -} diff --git a/ui/app/app.js b/ui/app/app.js index 2d8b46ce8..3b21e4477 100644 --- a/ui/app/app.js +++ b/ui/app/app.js @@ -95,6 +95,11 @@ App.prototype.render = function () { } App.prototype.renderAppBar = function () { + + if (window.METAMASK_UI_TYPE === 'notification') { + return null + } + const props = this.props const state = this.state || {} const isNetworkMenuOpen = state.isNetworkMenuOpen || false diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index db876dd9b..8c6a136bf 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -36,6 +36,7 @@ ConfirmTxScreen.prototype.render = function () { var unconfTxList = txHelper(unconfTxs, unconfMsgs) var index = state.index !== undefined ? state.index : 0 var txData = unconfTxList[index] || {} + var isNotification = window.METAMASK_UI_TYPE === 'notification' return ( @@ -43,9 +44,9 @@ ConfirmTxScreen.prototype.render = function () { // subtitle and nav h('.section-title.flex-row.flex-center', [ - h('i.fa.fa-arrow-left.fa-lg.cursor-pointer', { + !isNotification ? h('i.fa.fa-arrow-left.fa-lg.cursor-pointer', { onClick: this.goHome.bind(this), - }), + }) : null, h('h2.page-subtitle', 'Confirm Transaction'), ]), diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index 95b60f929..cbf64bf95 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -1,6 +1,7 @@ const extend = require('xtend') const actions = require('../actions') const txHelper = require('../../lib/tx-helper') +const extension = require('../../../app/scripts/lib/extension') module.exports = reduceApp @@ -250,6 +251,17 @@ function reduceApp (state, action) { warning: null, }) } else { + + const isNotification = window.METAMASK_UI_TYPE === 'notification' + if (isNotification) { + return extension.windows.getCurrent({}, function(win) { + extension.windows.remove(win.id, function(err) { + if (err) console.err(err) + }) + }) + } else { + debugger + } return extend(appState, { transForward: false, warning: null, From dfaac78e39ef1bd06e224ab56e493b4fa3201bef Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 16 Aug 2016 17:50:51 -0700 Subject: [PATCH 04/14] Linted --- app/scripts/lib/notifications.js | 20 +++++--------------- ui/app/reducers/app.js | 11 ++++------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index de9cf26e3..d33db0ef9 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -1,14 +1,4 @@ -const createId = require('hat') -const extend = require('xtend') -const unmountComponentAtNode = require('react-dom').unmountComponentAtNode -const findDOMNode = require('react-dom').findDOMNode -const render = require('react-dom').render -const h = require('react-hyperscript') -const PendingTxDetails = require('../../../ui/app/components/pending-tx-details') -const PendingMsgDetails = require('../../../ui/app/components/pending-msg-details') -const MetaMaskUiCss = require('../../../ui/css') const extension = require('./extension') -var notificationHandlers = {} const notifications = { createUnlockRequestNotification: createUnlockRequestNotification, @@ -32,11 +22,11 @@ function createMsgNotification (state) { function showNotification() { extension.windows.create({ - url:"notification.html", - type:"detached_panel", - focused:true, - width:360, - height:500, + url: 'notification.html', + type: 'detached_panel', + focused: true, + width: 360, + height: 500, }) } diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index cbf64bf95..deebf02be 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -254,14 +254,11 @@ function reduceApp (state, action) { const isNotification = window.METAMASK_UI_TYPE === 'notification' if (isNotification) { - return extension.windows.getCurrent({}, function(win) { - extension.windows.remove(win.id, function(err) { - if (err) console.err(err) - }) - }) - } else { - debugger + extension.windows.getCurrent({}, (win) => { + extension.windows.remove(win.id, console.error) + }) } + return extend(appState, { transForward: false, warning: null, From 270dbacbc472dae660fe8b143df9c64b7f8279cf Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 16 Aug 2016 17:52:38 -0700 Subject: [PATCH 05/14] Bump changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7833e9d1..767aec41f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Current Master - Added feature to reflect current conversion rates of current vault balance. +- Changed transaction approval from notifications system to popup system. ## 2.8.0 2016-08-15 From 642e84da6c6fa03d8848c9e1ecb082949b990b9a Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 22 Aug 2016 15:04:13 -0700 Subject: [PATCH 06/14] Merge master --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2626616..d2db94a4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,13 @@ ## Current Master +- Changed transaction approval from notifications system to popup system. + ## 2.9.0 2016-08-22 - Added ShapeShift to the transaction history - Added affiliate key to Shapeshift requests - Added feature to reflect current conversion rates of current vault balance. -- Changed transaction approval from notifications system to popup system. - Modify balance display logic. ## 2.8.0 2016-08-15 From 361e26fad779dc35e480cf5333b6237f437732a5 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 22 Aug 2016 17:18:14 -0700 Subject: [PATCH 07/14] Limit to one popup, re-focus on additional notifications. --- app/scripts/lib/notifications.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index d33db0ef9..278c7674e 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -21,12 +21,23 @@ function createMsgNotification (state) { } function showNotification() { - extension.windows.create({ - url: 'notification.html', - type: 'detached_panel', - focused: true, - width: 360, - height: 500, + extension.windows.getAll({}, (windows) => { + + let popupWindow = windows.find((win) => { + return win.type === 'popup' + }) + + if (popupWindow) { + return extension.windows.update(popupWindow.id, { focused: true }) + } + + extension.windows.create({ + url: 'notification.html', + type: 'detached_panel', + focused: true, + width: 360, + height: 500, + }) }) } From a9b53530b64cbe33f85d62c8042a72dd41353f26 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 22 Aug 2016 17:29:47 -0700 Subject: [PATCH 08/14] Fix back button hiding on popup --- ui/app/conf-tx.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index 8c6a136bf..f832b8a4b 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -5,6 +5,7 @@ const h = require('react-hyperscript') const connect = require('react-redux').connect const actions = require('./actions') const txHelper = require('../lib/tx-helper') +const isPopupOrNotification = require('../../app/scripts/lib/is-popup-or-notification') const PendingTx = require('./components/pending-tx') const PendingMsg = require('./components/pending-msg') @@ -36,7 +37,7 @@ ConfirmTxScreen.prototype.render = function () { var unconfTxList = txHelper(unconfTxs, unconfMsgs) var index = state.index !== undefined ? state.index : 0 var txData = unconfTxList[index] || {} - var isNotification = window.METAMASK_UI_TYPE === 'notification' + var isNotification = isPopupOrNotification() === 'notification' return ( From e5ca83d2bf7e97131e20da0ad352a38c7f8a2f86 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 23 Aug 2016 11:15:56 -0700 Subject: [PATCH 09/14] Emit updates to all listeners on pending tx updates Previously the metamask controller only supported a single UI event listener, which wasn't useful for having a separate notification UI open at the same time. Also reduced the notification's complexity down to a single method, which is heavily re-used. Still has an outstanding bug where if the plugin ui dismisses the last tx, it does not close the notification popup. --- app/scripts/background.js | 38 ++++-------------------------- app/scripts/lib/notifications.js | 16 +------------ app/scripts/metamask-controller.js | 22 +++++++++++++---- 3 files changed, 23 insertions(+), 53 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 79f8f9fd9..9a324047b 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -3,9 +3,7 @@ const extend = require('xtend') const Dnode = require('dnode') const eos = require('end-of-stream') const PortStream = require('./lib/port-stream.js') -const createUnlockRequestNotification = require('./lib/notifications.js').createUnlockRequestNotification -const createTxNotification = require('./lib/notifications.js').createTxNotification -const createMsgNotification = require('./lib/notifications.js').createMsgNotification +const notification = require('./lib/notifications.js') const messageManager = require('./lib/message-manager') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex const MetamaskController = require('./metamask-controller') @@ -26,41 +24,15 @@ const controller = new MetamaskController({ const idStore = controller.idStore function unlockAccountMessage () { - createUnlockRequestNotification({ - title: 'Account Unlock Request', - }) + notification.show() } function showUnconfirmedMessage (msgParams, msgId) { - var controllerState = controller.getState() - - createMsgNotification({ - imageifyIdenticons: false, - txData: { - msgParams: msgParams, - time: (new Date()).getTime(), - }, - identities: controllerState.identities, - accounts: controllerState.accounts, - onConfirm: idStore.approveMessage.bind(idStore, msgId, noop), - onCancel: idStore.cancelMessage.bind(idStore, msgId), - }) + notification.show() } function showUnconfirmedTx (txParams, txData, onTxDoneCb) { - var controllerState = controller.getState() - - createTxNotification({ - imageifyIdenticons: false, - txData: { - txParams: txParams, - time: (new Date()).getTime(), - }, - identities: controllerState.identities, - accounts: controllerState.accounts, - onConfirm: idStore.approveTransaction.bind(idStore, txData.id, noop), - onCancel: idStore.cancelTransaction.bind(idStore, txData.id), - }) + notification.show() } // @@ -109,7 +81,7 @@ function setupControllerConnection (stream) { dnode.on('remote', (remote) => { // push updates to popup controller.ethStore.on('update', controller.sendUpdate.bind(controller)) - controller.remote = remote + controller.listeners.push(remote) idStore.on('update', controller.sendUpdate.bind(controller)) // teardown on disconnect diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index 278c7674e..4c2aa91de 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -1,25 +1,11 @@ const extension = require('./extension') const notifications = { - createUnlockRequestNotification: createUnlockRequestNotification, - createTxNotification: createTxNotification, - createMsgNotification: createMsgNotification, + show: showNotification, } module.exports = notifications window.METAMASK_NOTIFIER = notifications -function createUnlockRequestNotification (opts) { - showNotification() -} - -function createTxNotification (state) { - showNotification() -} - -function createMsgNotification (state) { - showNotification() -} - function showNotification() { extension.windows.getAll({}, (windows) => { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 218f1f72a..81b232730 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -12,6 +12,7 @@ module.exports = class MetamaskController { constructor (opts) { this.opts = opts + this.listeners = [] this.configManager = new ConfigManager(opts) this.idStore = new IdentityStore({ configManager: this.configManager, @@ -112,9 +113,9 @@ module.exports = class MetamaskController { } sendUpdate () { - if (this.remote) { - this.remote.sendUpdate(this.getState()) - } + this.listeners.forEach((remote) => { + remote.sendUpdate(this.getState()) + }) } initializeProvider (opts) { @@ -130,10 +131,17 @@ module.exports = class MetamaskController { }, // tx signing approveTransaction: this.newUnsignedTransaction.bind(this), - signTransaction: idStore.signTransaction.bind(idStore), + signTransaction: (...args) => { + idStore.signTransaction(...args) + this.sendUpdate() + }, + // msg signing approveMessage: this.newUnsignedMessage.bind(this), - signMessage: idStore.signMessage.bind(idStore), + signMessage: (...args) => { + idStore.signMessage(...args) + this.sendUpdate() + }, } var provider = MetaMaskProvider(providerOpts) @@ -193,6 +201,8 @@ module.exports = class MetamaskController { // It's locked if (!state.isUnlocked) { + + // Allow the environment to define an unlock message. this.opts.unlockAccountMessage() idStore.addUnconfirmedTransaction(txParams, onTxDoneCb, noop) @@ -200,6 +210,7 @@ module.exports = class MetamaskController { } else { idStore.addUnconfirmedTransaction(txParams, onTxDoneCb, (err, txData) => { if (err) return onTxDoneCb(err) + this.sendUpdate() this.opts.showUnconfirmedTx(txParams, txData, onTxDoneCb) }) } @@ -211,6 +222,7 @@ module.exports = class MetamaskController { this.opts.unlockAccountMessage() } else { this.addUnconfirmedMessage(msgParams, cb) + this.sendUpdate() } } From 4fb49dfb4b3f23a5510c5a958671e9454d214a11 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 23 Aug 2016 11:40:08 -0700 Subject: [PATCH 10/14] Close popup even if last tx is dismissed from main UI --- app/scripts/lib/notifications.js | 22 ++++++++++++++-------- ui/app/reducers/app.js | 15 ++++++++------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index 4c2aa91de..cf4e1c216 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -2,19 +2,15 @@ const extension = require('./extension') const notifications = { show: showNotification, + getPopup, } module.exports = notifications window.METAMASK_NOTIFIER = notifications function showNotification() { - extension.windows.getAll({}, (windows) => { - - let popupWindow = windows.find((win) => { - return win.type === 'popup' - }) - - if (popupWindow) { - return extension.windows.update(popupWindow.id, { focused: true }) + getPopup((popup) => { + if (popup) { + return extension.windows.update(popup.id, { focused: true }) } extension.windows.create({ @@ -27,3 +23,13 @@ function showNotification() { }) } +function getPopup(cb) { + extension.windows.getAll({}, (windows) => { + let popup = windows.find((win) => { + return win.type === 'popup' + }) + + cb(popup) + }) +} + diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index 94b7e8bf7..7bd1dfd1f 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -2,6 +2,7 @@ const extend = require('xtend') const actions = require('../actions') const txHelper = require('../../lib/tx-helper') const extension = require('../../../app/scripts/lib/extension') +const notification = require('../../../app/scripts/lib/notifications') module.exports = reduceApp @@ -252,12 +253,7 @@ function reduceApp (state, action) { }) } else { - const isNotification = window.METAMASK_UI_TYPE === 'notification' - if (isNotification) { - extension.windows.getCurrent({}, (win) => { - extension.windows.remove(win.id, console.error) - }) - } + closePopupIfOpen() return extend(appState, { transForward: false, @@ -524,4 +520,9 @@ function indexForPending (state, txId) { return idx } - +function closePopupIfOpen() { + notification.getPopup((popup) => { + if (!popup) return + extension.windows.remove(popup.id, console.error) + }) +} From 983f3938daf7e8f718faed94b0b76054182719a3 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 23 Aug 2016 11:42:54 -0700 Subject: [PATCH 11/14] Linted --- app/scripts/background.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 9a324047b..5dae8235f 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -161,4 +161,3 @@ function setData (data) { window.localStorage[STORAGE_KEY] = JSON.stringify(data) } -function noop () {} From b3887ffd0a33096ef63fd4bbba9b5ff559775b3e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 23 Aug 2016 11:48:46 -0700 Subject: [PATCH 12/14] Skip popup dismissal in tests --- app/scripts/lib/notifications.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index cf4e1c216..c8ecad805 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -24,6 +24,12 @@ function showNotification() { } function getPopup(cb) { + + // Ignore in test environment + if (!extension.windows) { + return cb(null) + } + extension.windows.getAll({}, (windows) => { let popup = windows.find((win) => { return win.type === 'popup' From 671ca33abb93fafeb55a18543e81942c5963da8e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 23 Aug 2016 15:44:50 -0700 Subject: [PATCH 13/14] Close notification on opening main UI --- app/scripts/lib/notifications.js | 11 +++++++++-- app/scripts/popup.js | 7 +++++++ ui/app/reducers/app.js | 8 +------- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index c8ecad805..dcb946845 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -1,13 +1,14 @@ const extension = require('./extension') const notifications = { - show: showNotification, + show, getPopup, + closePopup, } module.exports = notifications window.METAMASK_NOTIFIER = notifications -function showNotification() { +function show () { getPopup((popup) => { if (popup) { return extension.windows.update(popup.id, { focused: true }) @@ -39,3 +40,9 @@ function getPopup(cb) { }) } +function closePopup() { + getPopup((popup) => { + if (!popup) return + extension.windows.remove(popup.id, console.error) + }) +} diff --git a/app/scripts/popup.js b/app/scripts/popup.js index 4c729e227..90b90a7af 100644 --- a/app/scripts/popup.js +++ b/app/scripts/popup.js @@ -11,6 +11,7 @@ const StreamProvider = require('web3-stream-provider') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex const isPopupOrNotification = require('./lib/is-popup-or-notification') const extension = require('./lib/extension') +const notification = require('./lib/notifications') // setup app var css = MetaMaskUiCss() @@ -25,6 +26,7 @@ function connectToAccountManager (cb) { // setup communication with background var name = isPopupOrNotification() + closePopupIfOpen(name) window.METAMASK_UI_TYPE = name var pluginPort = extension.runtime.connect({ name }) var portStream = new PortStream(pluginPort) @@ -98,3 +100,8 @@ function setupApp (err, opts) { }) } +function closePopupIfOpen(name) { + if (name !== 'notification') { + notification.closePopup() + } +} diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index 7bd1dfd1f..132e73f69 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -253,7 +253,7 @@ function reduceApp (state, action) { }) } else { - closePopupIfOpen() + notification.closePopup() return extend(appState, { transForward: false, @@ -520,9 +520,3 @@ function indexForPending (state, txId) { return idx } -function closePopupIfOpen() { - notification.getPopup((popup) => { - if (!popup) return - extension.windows.remove(popup.id, console.error) - }) -} From 65f765648735fec9b5180d3a7706048d611e48ba Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 23 Aug 2016 15:48:16 -0700 Subject: [PATCH 14/14] Linted --- ui/app/reducers/app.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index 132e73f69..da2c6e371 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -1,7 +1,6 @@ const extend = require('xtend') const actions = require('../actions') const txHelper = require('../../lib/tx-helper') -const extension = require('../../../app/scripts/lib/extension') const notification = require('../../../app/scripts/lib/notifications') module.exports = reduceApp