From 21a32ac57fff7fc386cc4df6339bb82287b377df Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Thu, 7 May 2015 14:42:39 -0700 Subject: [PATCH] fix(*): Message list container styles, logout exceptions, composer logic cleanup Summary: This diff makes a couple changes: - New drafts are no longer created in the composer component. Draft creation is back in the draft store where it belongs! - This means that the Draft List doesn't *always* contain two extra drafts, which are the unopened hot windows. - Windows never show until they're loaded, which means windows open slowly vs. opening empty. I think the former is preferable but it's easy to change. - Login window is now sized *before* it's shown instead of afterwards - The mailto: behavior is now handled by the DraftStore, which gets rid of the whole draftInitialJSON thing that never made any sense. fix(login) Make login window show at the correct size logout from edgehill completely Fix draft delete issue Don't show windows, hot or cold, until they've loaded Move logic for responding to mailto links into the draft store Always fire `windowPropsChanged` after packages load or the window is shown Show more error codes in red in activity bar Make sure DraftStoreProxy doesn't interrupt the rendering of the composer with a new draft Fix account-sidebar scroll issue, maybe? Test Plan: Run tests! Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1479 --- .../stylesheets/account-sidebar.less | 6 +- internal_packages/composer/lib/main.cjsx | 71 +++++------------- .../stylesheets/activity-bar.less | 4 + .../message-list/lib/message-list.cjsx | 5 +- .../thread-list/lib/draft-list.cjsx | 3 +- src/atom.coffee | 30 ++++---- src/browser/atom-window.coffee | 19 +++++ src/browser/edgehill-application.coffee | 36 ++------- src/flux/models/contact.coffee | 6 +- src/flux/stores/draft-store.coffee | 74 +++++++++++++------ src/package-manager.coffee | 4 +- src/package.coffee | 4 +- 12 files changed, 129 insertions(+), 133 deletions(-) diff --git a/internal_packages/account-sidebar/stylesheets/account-sidebar.less b/internal_packages/account-sidebar/stylesheets/account-sidebar.less index b818d507f..92179135f 100644 --- a/internal_packages/account-sidebar/stylesheets/account-sidebar.less +++ b/internal_packages/account-sidebar/stylesheets/account-sidebar.less @@ -4,7 +4,7 @@ #account-sidebar { order: 2; height: 100%; - overflow: auto; + overflow-y: auto; background-color: @source-list-bg; @@ -44,11 +44,11 @@ &.selected { background: @source-list-active-bg; color: @source-list-active-color; - img.colorfill { + img.colorfill { background: @source-list-active-color; } } - + &:hover { background: darken(@source-list-bg, 5%); cursor: default; diff --git a/internal_packages/composer/lib/main.cjsx b/internal_packages/composer/lib/main.cjsx index e70e41453..e76769222 100644 --- a/internal_packages/composer/lib/main.cjsx +++ b/internal_packages/composer/lib/main.cjsx @@ -10,10 +10,8 @@ NewComposeButton = require('./new-compose-button') ComposerView = require('./composer-view') module.exports = - item: null # The DOM item the main React component renders into activate: (@state={}) -> - atom.registerHotWindow windowType: "composer" replenishNum: 2 @@ -26,66 +24,33 @@ module.exports = @_activateComposeButton() else @_setupContainer() - windowProps = atom.getLoadSettings().windowProps ? {} - @windowPropsChanged(windowProps) - windowPropsChanged: (newWindowProps) -> - @_prepareDraft(newWindowProps).then (draftLocalId) => - React.render( - , @item - ) - if newWindowProps.errorMessage - @_showInitialErrorDialog(newWindowProps.errorMessage) - .catch (error) -> - console.error(error.stack) + windowPropsReceived: ({draftLocalId, errorMessage}) -> + return unless @_container + React.render( + , @_container + ) + if errorMessage + @_showInitialErrorDialog(errorMessage) deactivate: -> if atom.isMainWindow() - React.unmountComponentAtNode(@new_compose_button) - @new_compose_button.remove() - @new_compose_button = null + React.unmountComponentAtNode(@_composeButton) + @_composeButton.remove() + @_composeButton = null else - React.unmountComponentAtNode(@item) - @item.remove() - @item = null + React.unmountComponentAtNode(@_container) + @_container.remove() + @_container = null serialize: -> @state _setupContainer: -> - if @item? then return # Activate once - @item = document.createElement("div") - @item.setAttribute("id", "composer-full-window") - @item.setAttribute("class", "composer-full-window") - document.body.appendChild(@item) - - # This logic used to be in the DraftStore (which is where it should be). It - # got moved here becaues of an obscure atom-shell/Chrome bug whereby database - # requests firing right before the new-window loaded would cause the - # new-window to load with about:blank instead of its contents. By moving the - # DB logic here, we can get around this. - _prepareDraft: ({draftLocalId, draftInitialJSON}={}) -> - # The NamespaceStore isn't set yet in the new window, populate it first. - NamespaceStore.populateItems().then -> - new Promise (resolve, reject) -> - if draftLocalId? - resolve(draftLocalId) - else - # Create a new draft - draft = new Message - body: "" - from: [NamespaceStore.current().me()] - date: (new Date) - draft: true - pristine: true - namespaceId: NamespaceStore.current().id - # If initial JSON was provided, apply it to the new model. - # This is used to apply the values in mailto: links to new drafts - if draftInitialJSON - draft.fromJSON(draftInitialJSON) - - DatabaseStore.persistModel(draft).then -> - DatabaseStore.localIdForModel(draft).then(resolve).catch(reject) - .catch(reject) + if @_container? then return # Activate once + @_container = document.createElement("div") + @_container.setAttribute("id", "composer-full-window") + @_container.setAttribute("class", "composer-full-window") + document.body.appendChild(@_container) _activateComposeButton: -> ComponentRegistry.register NewComposeButton, diff --git a/internal_packages/inbox-activity-bar/stylesheets/activity-bar.less b/internal_packages/inbox-activity-bar/stylesheets/activity-bar.less index dbb3f2d7e..aa12e7036 100755 --- a/internal_packages/inbox-activity-bar/stylesheets/activity-bar.less +++ b/internal_packages/inbox-activity-bar/stylesheets/activity-bar.less @@ -121,6 +121,10 @@ padding-bottom:3px; } .item.status-code-500, + .item.status-code-501, + .item.status-code-502, + .item.status-code-503, + .item.status-code-504, .item.status-code-400, .item.status-code-404, .item.status-code-409 { diff --git a/internal_packages/message-list/lib/message-list.cjsx b/internal_packages/message-list/lib/message-list.cjsx index ae0d60c9b..578fe43f6 100755 --- a/internal_packages/message-list/lib/message-list.cjsx +++ b/internal_packages/message-list/lib/message-list.cjsx @@ -12,6 +12,9 @@ MessageItem = require "./message-item" class MessageList extends React.Component @displayName: 'MessageList' @containerRequired: false + @containerStyles: + minWidth: 500 + maxWidth: 900 constructor: (@props) -> @state = @_getStateFromStores() @@ -253,7 +256,5 @@ class MessageList extends React.Component _wasAtBottom: => (@_lastScrollTop + @_lastHeight) >= @_lastScrollHeight -MessageList.minWidth = 500 -MessageList.maxWidth = 900 module.exports = MessageList diff --git a/internal_packages/thread-list/lib/draft-list.cjsx b/internal_packages/thread-list/lib/draft-list.cjsx index 31fcc457a..2ff531b5b 100644 --- a/internal_packages/thread-list/lib/draft-list.cjsx +++ b/internal_packages/thread-list/lib/draft-list.cjsx @@ -69,11 +69,10 @@ class DraftList extends React.Component # Additional Commands _onDelete: ({focusedId}) => - item = @state.dataView.getById(focusedId) + item = DraftListStore.view().getById(focusedId) return unless item DatabaseStore.localIdForModel(item).then (localId) -> Actions.destroyDraft(localId) - @_onShiftSelectedIndex(-1) module.exports = DraftList diff --git a/src/atom.coffee b/src/atom.coffee index 83c0a3176..52a689e52 100644 --- a/src/atom.coffee +++ b/src/atom.coffee @@ -84,10 +84,8 @@ class Atom extends Model # Returns the load settings hash associated with the current window. @getLoadSettings: -> - # We pull from the window object instead of the url so the - # loadSettings can change post bootup. This is very useful for - # starting hot windows. - @loadSettings ?= @getCurrentWindow().loadSettings + @loadSettings ?= JSON.parse(decodeURIComponent(location.search.substr(14))) + cloned = _.deepClone(@loadSettings) # The loadSettings.windowState could be large, request it only when needed. cloned.__defineGetter__ 'windowState', => @@ -228,8 +226,6 @@ class Atom extends Model @subscribe @packages.onDidActivateInitialPackages => @watchThemes() @windowEventHandler = new WindowEventHandler - ipc.on("window-props-changed", => @windowPropsChanged()) - # Start our error reporting to the backend and attach error handlers # to the window and the Bluebird Promise library, converting things # back through the sourcemap as necessary. @@ -452,19 +448,16 @@ class Atom extends Model reload: -> ipc.send('call-window-method', 'restart') - # Calls the `windowPropsChanged` method of all packages that are + # Calls the `windowPropsReceived` method of all packages that are # currently loaded - windowPropsChanged: -> - # This will cause it to get refreshed the next time they're queried - @constructor.loadSettings = null + loadSettingsChanged: (loadSettings) => + @loadSettings = loadSettings + {width, height, windowProps} = loadSettings - {width, - height, - windowProps} = @getLoadSettings() + @packages.windowPropsReceived(windowProps ? {}) - @packages.windowPropsChanged(windowProps) - - @setWindowDimensions({width, height}) if width and height + if width and height + @setWindowDimensions({width, height}) # Extended: Returns a {Boolean} true when the current window is maximized. isMaximixed: -> @@ -620,6 +613,7 @@ class Atom extends Model {width, height, windowType, + windowProps, windowPackages} = @getLoadSettings() @loadConfig() @@ -633,6 +627,9 @@ class Atom extends Model @packages.loadPackage(pack) for pack in (windowPackages ? []) @packages.activate() + ipc.on("load-settings-changed", @loadSettingsChanged) + @packages.windowPropsReceived(windowProps ? {}) + @keymaps.loadUserKeymap() @setWindowDimensions({width, height}) if width and height @@ -645,6 +642,7 @@ class Atom extends Model logout: -> if @isLoggedIn() @config.set('inbox', null) + @config.set('edgehill', null) Actions = require './flux/actions' Actions.logout() @hide() diff --git a/src/browser/atom-window.coffee b/src/browser/atom-window.coffee index 87a081de6..f00471c70 100644 --- a/src/browser/atom-window.coffee +++ b/src/browser/atom-window.coffee @@ -20,6 +20,8 @@ class AtomWindow constructor: (settings={}) -> {frame, title, + width, + height, resizable, pathToOpen, hideMenuBar, @@ -38,6 +40,8 @@ class AtomWindow show: false title: title ? 'Nilas' frame: frame ? true + width: width + height: height resizable: resizable ? true icon: @constructor.iconPath 'auto-hide-menu-bar': hideMenuBar @@ -78,6 +82,8 @@ class AtomWindow @browserWindow.once 'window:loaded', => @emit 'window:loaded' @loaded = true + if @browserWindow.loadSettingsChangedSinceGetURL + @browserWindow.webContents.send('load-settings-changed', @browserWindow.loadSettings) @browserWindow.loadUrl @getUrl(loadSettings) @browserWindow.focusOnWebView() if @isSpec @@ -88,6 +94,8 @@ class AtomWindow setLoadSettings: (loadSettings) -> @browserWindow.loadSettings = loadSettings + @browserWindow.loadSettingsChangedSinceGetURL = true + @browserWindow.webContents.send('load-settings-changed', loadSettings) if @loaded getUrl: (loadSettingsObj) -> # Ignore the windowState when passing loadSettings via URL, since it could @@ -95,6 +103,8 @@ class AtomWindow loadSettings = _.clone(loadSettingsObj) delete loadSettings['windowState'] + @browserWindow.loadSettingsChangedSinceGetURL = false + url.format protocol: 'file' pathname: "#{@resourcePath}/static/index.html" @@ -212,6 +222,15 @@ class AtomWindow show: -> @browserWindow.show() + showWhenLoaded: -> + if @loaded + @show() + @focus() + else + @once 'window:loaded', => + @show() + @focus() + focus: -> @browserWindow.focus() minimize: -> @browserWindow.minimize() diff --git a/src/browser/edgehill-application.coffee b/src/browser/edgehill-application.coffee index e94ce4eff..0fa5f29bd 100644 --- a/src/browser/edgehill-application.coffee +++ b/src/browser/edgehill-application.coffee @@ -11,7 +11,6 @@ path = require 'path' os = require 'os' net = require 'net' url = require 'url' -qs = require 'querystring' exec = require('child_process').exec querystring = require 'querystring' {EventEmitter} = require 'events' @@ -121,7 +120,7 @@ class AtomApplication # # This means that when `newWindow` is called, instead of going through # the bootup process, it simply replaces key parameters and does a soft - # reload via `windowPropsChanged`. + # reload via `windowPropsReceived`. # # Since the window is already loaded, there are only some options that # can be soft-reloaded. If you attempt to pass options that a soft @@ -213,9 +212,8 @@ class AtomApplication newColdWindow: (options={}) -> options = _.extend(@defaultWindowOptions(), options) - w = new AtomWindow options - w.show() - w.focus() + w = new AtomWindow(options) + w.showWhenLoaded() # Tries to create a new hot window. Since we're updating an existing # window instead of creatinga new one, there are limitations in the @@ -232,19 +230,10 @@ class AtomApplication options.windowPackages = hotWindowParams.windowPackages @newColdWindow(options) else - win = hotWindowParams.loadedWindows.pop() + [win] = hotWindowParams.loadedWindows.splice(0,1) newLoadSettings = _.extend(win.loadSettings(), options) - - # This will update the internal instance variable that the window will - # query for its load settings. win.setLoadSettings(newLoadSettings) - - # This is expected to be caught by the main application to re-fetch - # the loadSettings and re-render itself accordingly. - win.browserWindow.webContents.send('window-props-changed') - - win.show() - win.focus() + win.showWhenLoaded() @_replenishHotWindows() @@ -283,6 +272,7 @@ class AtomApplication @_replenishQueue.push(optionsArray.shift()) @_processReplenishQueue() + _replenishHotWindows: _.debounce(AtomApplication::__replenishHotWindows, 100) _processReplenishQueue: -> @@ -619,19 +609,7 @@ class AtomApplication # Attempt to parse the mailto link into Message object JSON # and then open a composer window if parts.protocol is 'mailto:' - query = qs.parse(parts.query) - query.to = "#{parts.auth}@#{parts.host}" - - json = { - subject: query.subject || '', - body: query.body || '', - } - - emailToObj = (email) -> {email: email, object: 'Contact'} - for attr in ['to', 'cc', 'bcc'] - json[attr] = query[attr]?.split(',').map(emailToObj) || [] - - @mainWindow.browserWindow.webContents.send('mailto', json) + @mainWindow.browserWindow.webContents.send('mailto', urlToOpen) # The host of the URL being opened is assumed to be the package name # responsible for opening the URL. A new window will be created with diff --git a/src/flux/models/contact.coffee b/src/flux/models/contact.coffee index 78f4f17b9..82bdb3ade 100644 --- a/src/flux/models/contact.coffee +++ b/src/flux/models/contact.coffee @@ -45,15 +45,15 @@ class Contact extends Model # - `name` if the contact has a populated name value # - `email` in all other cases. displayName: -> - return "You" if @email == NamespaceStore.current().emailAddress + return "You" if @email is NamespaceStore.current()?.emailAddress @_nameParts().join(' ') displayFirstName: -> - return "You" if @email == NamespaceStore.current().emailAddress + return "You" if @email is NamespaceStore.current()?.emailAddress @firstName() displayLastName: -> - return "" if @email == NamespaceStore.current().emailAddress + return "" if @email is NamespaceStore.current()?.emailAddress @lastName() firstName: -> diff --git a/src/flux/stores/draft-store.coffee b/src/flux/stores/draft-store.coffee index 015105728..ba7e70f3b 100644 --- a/src/flux/stores/draft-store.coffee +++ b/src/flux/stores/draft-store.coffee @@ -5,11 +5,13 @@ ipc = require 'ipc' DraftStoreProxy = require './draft-store-proxy' DatabaseStore = require './database-store' NamespaceStore = require './namespace-store' +ContactStore = require './contact-store' SendDraftTask = require '../tasks/send-draft' DestroyDraftTask = require '../tasks/destroy-draft' Thread = require '../models/thread' +Contact = require '../models/contact' Message = require '../models/message' MessageUtils = require '../models/message-utils' Actions = require '../actions' @@ -40,11 +42,11 @@ class DraftStore @listenTo Actions.composeReply, @_onComposeReply @listenTo Actions.composeForward, @_onComposeForward @listenTo Actions.composeReplyAll, @_onComposeReplyAll - @listenTo Actions.composePopoutDraft, @_onComposePopoutDraft - @listenTo Actions.composeNewBlankDraft, @_onComposeNewBlankDraft + @listenTo Actions.composePopoutDraft, @_onPopoutDraftLocalId + @listenTo Actions.composeNewBlankDraft, @_onPopoutBlankDraft atom.commands.add 'body', - 'application:new-message': => @_onComposeNewBlankDraft() + 'application:new-message': => @_onPopoutBlankDraft() @listenTo Actions.sendDraft, @_onSendDraft @listenTo Actions.destroyDraft, @_onDestroyDraft @@ -59,9 +61,7 @@ class DraftStore @_sendingState = {} @_extensions = [] - ipc.on 'mailto', (mailToJSON) => - return unless atom.isMainWindow() - atom.newWindow @_composerWindowProps(draftInitialJSON: mailToJSON) + ipc.on 'mailto', @_onHandleMailtoLink # TODO: Doesn't work if we do window.addEventListener, but this is # fragile. Pending an Atom fix perhaps? @@ -301,22 +301,54 @@ class DraftStore re = new RegExp("", "igm") body.replace(re, "") - # The logic to create a new Draft used to be in the DraftStore (which is - # where it should be). It got moved to composer/lib/main.cjsx becaues - # of an obscure atom-shell/Chrome bug whereby database requests firing right - # before the new-window loaded would cause the new-window to load with - # about:blank instead of its contents. By moving the DB logic there, we can - # get around this. - _onComposeNewBlankDraft: => - atom.newWindow @_composerWindowProps() + _onPopoutBlankDraft: => + draft = new Message + body: "" + from: [NamespaceStore.current().me()] + date: (new Date) + draft: true + pristine: true + namespaceId: NamespaceStore.current().id + DatabaseStore.persistModel(draft).then => + DatabaseStore.localIdForModel(draft).then(@_onPopoutDraftLocalId) - _onComposePopoutDraft: (draftLocalId) => - atom.newWindow @_composerWindowProps(draftLocalId: draftLocalId) + _onPopoutDraftLocalId: (draftLocalId, options = {}) => + options.draftLocalId = draftLocalId - _composerWindowProps: (props={}) => - title: "Message" - windowType: "composer" - windowProps: _.extend {}, props + atom.newWindow + title: "Message" + windowType: "composer" + windowProps: options + + _onHandleMailtoLink: (urlString) => + namespace = NamespaceStore.current() + return unless namespace + + url = require 'url' + qs = require 'querystring' + parts = url.parse(urlString) + query = qs.parse(parts.query) + query.to = "#{parts.auth}@#{parts.host}" + + draft = new Message + body: query.body || '' + subject: query.subject || '', + from: [namespace.me()] + date: (new Date) + draft: true + pristine: true + namespaceId: namespace.id + + contactForEmail = (email) -> + match = ContactStore.searchContacts(email, 1) + return match[0] if match[0] + return new Contact({email}) + + for attr in ['to', 'cc', 'bcc'] + draft[attr] = query[attr]?.split(',').map(contactForEmail) || [] + + DatabaseStore.persistModel(draft).then => + DatabaseStore.localIdForModel(draft).then(@_onPopoutDraftLocalId) _onDestroyDraft: (draftLocalId) => # Immediately reset any pending changes so no saves occur @@ -354,7 +386,7 @@ class DraftStore _onSendDraftError: (draftLocalId, errorMessage) -> @_sendingState[draftLocalId] = false if atom.getWindowType() is "composer" - atom.newWindow @_composerWindowProps({errorMessage, draftLocalId}) + @_onPopoutDraftLocalId(draftLocalId, {errorMessage}) @trigger() _onSendDraftSuccess: (draftLocalId) => diff --git a/src/package-manager.coffee b/src/package-manager.coffee index 7dab4c599..5905952bf 100644 --- a/src/package-manager.coffee +++ b/src/package-manager.coffee @@ -445,5 +445,5 @@ class PackageManager delete @activePackages[pack.name] @emitter.emit 'did-deactivate-package', pack - windowPropsChanged: (windowProps) -> - pack.windowPropsChanged?(windowProps) for pack in @getLoadedPackages() + windowPropsReceived: (windowProps) -> + pack.windowPropsReceived?(windowProps) for pack in @getLoadedPackages() diff --git a/src/package.coffee b/src/package.coffee index f39c4cad8..c5d07f37e 100644 --- a/src/package.coffee +++ b/src/package.coffee @@ -154,8 +154,8 @@ class Package Q.all([@grammarsPromise, @settingsPromise, @activationDeferred.promise]) - windowPropsChanged: (windowProps) -> - @mainModule?.windowPropsChanged?(windowProps) + windowPropsReceived: (windowProps) -> + @mainModule?.windowPropsReceived?(windowProps) activateNow: -> try