From ce968c0d3ef9609919118aabfb6a2e86ffc819dd Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Wed, 1 Jun 2016 17:13:07 -0700 Subject: [PATCH] update(sidebar): Moves accounts in sidebar state to FocusedPerspectiveStore Summary: - FocusedPerspectiveStore now holds the state of the accounts that are currently being displayed in the sidebar, i.e. unified inbox or not, which makes it globally accessible state. The SidebarStore just reads this state directly off of the FocusedPerspectiveStore to decide which sections and items to display in the sidebar. - Updates `Actions.focusDefaultMailboxPerspectiveForAccounts` to take an optional array of `sidebarAccountIds` to set in the sidebar. The default behavior is to show the accounts of the perspective that will be focused. E.g. when selecting an account via the account switcher, it will just show the sidebar items for the single account that was selected, but when adding a new account, we are setting the sidebar accounts to all account although we are still focusing the perspective for a single account. - Will now show unified inbox sidebar with correctly focused account when new account added. - Cleans up the code a little bit, but this package still needs major refactor Test Plan: Missing! Reviewers: jackie, bengotow Reviewed By: bengotow Differential Revision: https://phab.nylas.com/D3002 --- .../lib/account-commands.coffee | 6 +-- .../lib/components/account-sidebar.cjsx | 4 +- .../account-sidebar/lib/sidebar-item.coffee | 1 - .../lib/sidebar-section.coffee | 1 - .../account-sidebar/lib/sidebar-store.coffee | 39 +++++++++---------- src/flux/stores/account-store.coffee | 19 ++++++--- .../stores/focused-perspective-store.coffee | 32 +++++++++++---- 7 files changed, 62 insertions(+), 40 deletions(-) diff --git a/internal_packages/account-sidebar/lib/account-commands.coffee b/internal_packages/account-sidebar/lib/account-commands.coffee index b977a3320..eba022dae 100644 --- a/internal_packages/account-sidebar/lib/account-commands.coffee +++ b/internal_packages/account-sidebar/lib/account-commands.coffee @@ -1,13 +1,11 @@ _ = require 'underscore' -{AccountStore, MenuHelpers} = require 'nylas-exports' -SidebarActions = require './sidebar-actions' +{Actions, AccountStore, MenuHelpers} = require 'nylas-exports' class AccountCommands @_focusAccounts: (accounts) -> - NylasEnv.savedState.shouldRefocusSidebarAccounts = true - SidebarActions.focusAccounts(accounts) + Actions.focusDefaultMailboxPerspectiveForAccounts(accounts) NylasEnv.show() unless NylasEnv.isVisible() @_isSelected: (account, focusedAccounts) => diff --git a/internal_packages/account-sidebar/lib/components/account-sidebar.cjsx b/internal_packages/account-sidebar/lib/components/account-sidebar.cjsx index eddb35821..dc0298301 100644 --- a/internal_packages/account-sidebar/lib/components/account-sidebar.cjsx +++ b/internal_packages/account-sidebar/lib/components/account-sidebar.cjsx @@ -1,5 +1,6 @@ _ = require 'underscore' React = require 'react' +{AccountStore} = require 'nylas-exports' {OutlineView, ScrollRegion, Flexbox} = require 'nylas-component-kit' AccountSwitcher = require './account-switcher' SidebarStore = require '../sidebar-store' @@ -19,6 +20,7 @@ class AccountSidebar extends React.Component componentDidMount: => @unsubscribers = [] @unsubscribers.push SidebarStore.listen @_onStoreChange + @unsubscribers.push AccountStore.listen @_onStoreChange componentWillUnmount: => unsubscribe() for unsubscribe in @unsubscribers @@ -27,7 +29,7 @@ class AccountSidebar extends React.Component @setState @_getStateFromStores() _getStateFromStores: => - accounts: SidebarStore.accounts() + accounts: AccountStore.accounts() focusedAccounts: SidebarStore.focusedAccounts() userSections: SidebarStore.userSections() standardSection: SidebarStore.standardSection() diff --git a/internal_packages/account-sidebar/lib/sidebar-item.coffee b/internal_packages/account-sidebar/lib/sidebar-item.coffee index 72f9f83f7..92c127e7f 100644 --- a/internal_packages/account-sidebar/lib/sidebar-item.coffee +++ b/internal_packages/account-sidebar/lib/sidebar-item.coffee @@ -91,7 +91,6 @@ class SidebarItem return false unless target.canReceiveThreadsFromAccountIds(data.accountIds) return item.dataTransferType in event.dataTransfer.types onSelect: (item) -> - NylasEnv.savedState.shouldRefocusSidebarAccounts = false Actions.focusMailboxPerspective(item.perspective) }, opts) diff --git a/internal_packages/account-sidebar/lib/sidebar-section.coffee b/internal_packages/account-sidebar/lib/sidebar-section.coffee index 70878ba20..7ae953781 100644 --- a/internal_packages/account-sidebar/lib/sidebar-section.coffee +++ b/internal_packages/account-sidebar/lib/sidebar-section.coffee @@ -1,6 +1,5 @@ _ = require 'underscore' {Actions, - AccountStore, SyncbackCategoryTask, DestroyCategoryTask, CategoryStore, diff --git a/internal_packages/account-sidebar/lib/sidebar-store.coffee b/internal_packages/account-sidebar/lib/sidebar-store.coffee index 70c970234..4a38f1f07 100644 --- a/internal_packages/account-sidebar/lib/sidebar-store.coffee +++ b/internal_packages/account-sidebar/lib/sidebar-store.coffee @@ -21,13 +21,11 @@ class SidebarStore extends NylasStore constructor: -> NylasEnv.savedState.sidebarKeysCollapsed ?= {} - NylasEnv.savedState.shouldRefocusSidebarAccounts ?= true @_sections = {} @_sections[Sections.Standard] = {} @_sections[Sections.User] = [] - @_focusedAccounts = FocusedPerspectiveStore.current().accountIds.map (id) -> - AccountStore.accountForId(id) + @_focusedAccounts = FocusedPerspectiveStore.sidebarAccounts() @_registerCommands() @_registerMenuItems() @_registerListeners() @@ -48,7 +46,6 @@ class SidebarStore extends NylasStore _registerListeners: -> @listenTo Actions.setCollapsedSidebarItem, @_onSetCollapsedByName @listenTo SidebarActions.setKeyCollapsed, @_onSetCollapsedByKey - @listenTo SidebarActions.focusAccounts, @_onAccountsFocused @listenTo AccountStore, @_onAccountsChanged @listenTo FocusedPerspectiveStore, @_onFocusedPerspectiveChanged @listenTo WorkspaceStore, @_updateSections @@ -84,27 +81,29 @@ class SidebarStore extends NylasStore _registerMenuItems: (accounts = AccountStore.accounts()) => AccountCommands.registerMenuItems(accounts, @_focusedAccounts) - _onAccountsFocused: (accounts) => - Actions.focusDefaultMailboxPerspectiveForAccounts(accounts) - @_focusedAccounts = accounts - @_registerMenuItems() - @_updateSections() - + # TODO Refactor this + # Listen to changes on the account store only for when the account label + # changes. When accounts or added or removed, those changes will come in + # through the FocusedPerspectiveStore _onAccountsChanged: => - accounts = AccountStore.accounts() - @_focusedAccounts = accounts - @_registerCommands() - @_registerMenuItems() @_updateSections() + # TODO Refactor this + # The FocusedPerspectiveStore tells this store the accounts that should be + # displayed in the sidebar (i.e. unified inbox vs single account) and will + # trigger whenever an account is added or removed, as well as when a + # perspective is focused. + # However, when udpating the SidebarSections, we also depend on the actual + # accounts in the AccountStore. The problem is that the FocusedPerspectiveStore + # triggers before the AccountStore is actually updated, so we need to wait for + # the AccountStore to get updated (via `defer`) before updateing our sidebar + # sections _onFocusedPerspectiveChanged: => - currentIds = _.pluck(@_focusedAccounts, 'id') - newIds = FocusedPerspectiveStore.current().accountIds - # TODO get rid of this nasty global state - if NylasEnv.savedState.shouldRefocusSidebarAccounts is true - @_focusedAccounts = newIds.map (id) -> AccountStore.accountForId(id) + _.defer => + @_focusedAccounts = FocusedPerspectiveStore.sidebarAccounts() + @_registerCommands() @_registerMenuItems() - @_updateSections() + @_updateSections() _updateSections: => accounts = @_focusedAccounts diff --git a/src/flux/stores/account-store.coffee b/src/flux/stores/account-store.coffee index 3a28cdc82..be2179348 100644 --- a/src/flux/stores/account-store.coffee +++ b/src/flux/stores/account-store.coffee @@ -37,16 +37,19 @@ class AccountStore extends NylasStore return if @_version / 1 is change.newValue / 1 oldAccountIds = _.pluck(@_accounts, 'id') @_loadAccounts() - newAccountIds = _.pluck(@_accounts, 'id') - newAccountIds = _.difference(newAccountIds, oldAccountIds) + accountIds = _.pluck(@_accounts, 'id') + newAccountIds = _.difference(accountIds, oldAccountIds) if newAccountIds.length > 0 newId = newAccountIds[0] CategoryStore = require './category-store' CategoryStore.whenCategoriesReady(newId).then => + Actions.focusDefaultMailboxPerspectiveForAccounts([newId], sidebarAccountIds: accountIds) # TODO this Action is a hack, get rid of it in sidebar refactor - Actions.setCollapsedSidebarItem('Inbox', false) - Actions.focusDefaultMailboxPerspectiveForAccounts([newAccountIds[0]]) + # Wait until the FocusedPerspectiveStore triggers and the sidebar is + # updated to uncollapse the inbox for the new account + _.defer => + Actions.setCollapsedSidebarItem('Inbox', false) _loadAccounts: => try @@ -138,9 +141,15 @@ class AccountStore extends NylasStore @_caches = {} remainingAccounts = _.without(@_accounts, account) + # This action is called before saving because we need to unfocus the + # perspective of the account that is being removed before removing the + # account, otherwise when we trigger with the new set of accounts, the + # current perspective will still reference a stale accountId which will + # cause things to break if remainingAccounts.length > 0 - Actions.setCollapsedSidebarItem('Inbox', true) Actions.focusDefaultMailboxPerspectiveForAccounts(remainingAccounts) + _.defer => + Actions.setCollapsedSidebarItem('Inbox', true) @_accounts = remainingAccounts @_save() diff --git a/src/flux/stores/focused-perspective-store.coffee b/src/flux/stores/focused-perspective-store.coffee index 28fd29062..6497ab9a7 100644 --- a/src/flux/stores/focused-perspective-store.coffee +++ b/src/flux/stores/focused-perspective-store.coffee @@ -12,7 +12,7 @@ class FocusedPerspectiveStore extends NylasStore @listenTo CategoryStore, @_onCategoryStoreChanged @listenTo Actions.focusMailboxPerspective, @_onFocusPerspective - @listenTo Actions.focusDefaultMailboxPerspectiveForAccounts, @_onFocusAccounts + @listenTo Actions.focusDefaultMailboxPerspectiveForAccounts, @_onFocusPerspectiveForAccounts @_listenToCommands() _listenToCommands: => @@ -45,11 +45,11 @@ class FocusedPerspectiveStore extends NylasStore return perspective # Inbound Events - _onCategoryStoreChanged: -> if @_current.isEqual(MailboxPerspective.forNothing()) perspective = @_loadSavedPerspective(NylasEnv.savedState.perspective) - @_setPerspective(perspective) + {sidebarAccountIds} = NylasEnv.savedState + @_setPerspective(perspective, sidebarAccountIds ? perspective.accountIds) else accountIds = @_current.accountIds categories = @_current.categories() @@ -62,19 +62,32 @@ class FocusedPerspectiveStore extends NylasStore _onFocusPerspective: (perspective) => @_setPerspective(perspective) - _onFocusAccounts: (accountsOrIds) => + # Takes an optional array of `sidebarAccountIds`. By default, this method will + # set the sidebarAccountIds to the perspective's accounts if no value is + # provided + _onFocusPerspectiveForAccounts: (accountsOrIds, {sidebarAccountIds} = {}) => return unless accountsOrIds - @_setPerspective(@_defaultPerspective(accountsOrIds)) + perspective = @_defaultPerspective(accountsOrIds) + sidebarAccountIds ?= perspective.accountIds + @_setPerspective(perspective, sidebarAccountIds) _defaultPerspective: (accounts = AccountStore.accounts()) -> return MailboxPerspective.forNothing() unless accounts.length > 0 return MailboxPerspective.forInbox(accounts) - _setPerspective: (perspective) -> - unless perspective.isEqual(@_current) + _setPerspective: (perspective, sidebarAccountIds) -> + shouldTrigger = false + + if not perspective.isEqual(@_current) NylasEnv.savedState.perspective = perspective.toJSON() @_current = perspective - @trigger() + shouldTrigger = true + + if sidebarAccountIds and not _.isEqual(NylasEnv.savedState.sidebarAccountIds, sidebarAccountIds) + NylasEnv.savedState.sidebarAccountIds = sidebarAccountIds + shouldTrigger = true + + @trigger() if shouldTrigger if perspective.drafts desired = WorkspaceStore.Sheet.Drafts @@ -98,5 +111,8 @@ class FocusedPerspectiveStore extends NylasStore current: => @_current + sidebarAccounts: => + {sidebarAccountIds} = NylasEnv.savedState + sidebarAccountIds.map((id) => AccountStore.accountForId(id)) module.exports = new FocusedPerspectiveStore()