diff --git a/internal_packages/composer/lib/composer-view.cjsx b/internal_packages/composer/lib/composer-view.cjsx index 4158115d4..21a11571d 100644 --- a/internal_packages/composer/lib/composer-view.cjsx +++ b/internal_packages/composer/lib/composer-view.cjsx @@ -443,6 +443,12 @@ class ComposerView extends React.Component showQuotedText: @isForwardedMessage() populated: true + # It's possible for another part of the application to manipulate the draft + # we're displaying. If they've added a CC or BCC, make sure we show those fields. + # We should never be hiding the field if it's populated. + state.showcc = true if not _.isEmpty(draft.cc) + state.showbcc = true if not _.isEmpty(draft.bcc) + @setState(state) _shouldShowSubject: => @@ -575,7 +581,9 @@ class ComposerView extends React.Component @focus('textFieldCc') _onSendingStateChanged: => - @setState isSending: DraftStore.isSendingDraft(@props.localId) + isSending = DraftStore.isSendingDraft(@props.localId) + if isSending isnt @state.isSending + @setState({isSending}) _onEmptyCc: => @setState showcc: false diff --git a/internal_packages/composer/spec/composer-view-spec.cjsx b/internal_packages/composer/spec/composer-view-spec.cjsx index 2542f1467..35c227324 100644 --- a/internal_packages/composer/spec/composer-view-spec.cjsx +++ b/internal_packages/composer/spec/composer-view-spec.cjsx @@ -246,7 +246,7 @@ describe "populated composer", -> advanceClock(1000) expect(@composer.refs['contentBody'].focus).toHaveBeenCalled() - describe "when emptying cc fields", -> + describe "when emptying cc/bcc fields", -> it "focuses on to when bcc is emptied and there's no cc field", -> useDraft.call(@, bcc: [u1]) @@ -255,6 +255,7 @@ describe "populated composer", -> spyOn(@composer.refs['textFieldBcc'], 'focus') bcc = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, ParticipantsTextField, field: "bcc")[0] + @draft.bcc = [] bcc.props.onEmptied() expect(@composer.state.showbcc).toBe false @@ -271,7 +272,7 @@ describe "populated composer", -> spyOn(@composer.refs['textFieldBcc'], 'focus') bcc = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, ParticipantsTextField, field: "bcc")[0] - + @draft.bcc = [] bcc.props.onEmptied() expect(@composer.state.showbcc).toBe false advanceClock(1000) @@ -287,14 +288,34 @@ describe "populated composer", -> spyOn(@composer.refs['textFieldBcc'], 'focus') cc = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, ParticipantsTextField, field: "cc")[0] + @draft.cc = [] cc.props.onEmptied() - expect(@composer.state.showcc).toBe false advanceClock(1000) expect(@composer.refs['textFieldTo'].focus).toHaveBeenCalled() expect(@composer.refs['textFieldCc']).not.toBeDefined() expect(@composer.refs['textFieldBcc'].focus).not.toHaveBeenCalled() + describe "when participants are added during a draft update", -> + it "shows the cc fields and bcc fields to ensure participants are never hidden", -> + useDraft.call(@, cc: [], bcc: []) + makeComposer.call(@) + expect(@composer.state.showbcc).toBe(false) + expect(@composer.state.showcc).toBe(false) + + # Simulate a change event fired by the DraftStoreProxy + @draft.cc = [u1] + @composer._onDraftChanged() + + expect(@composer.state.showbcc).toBe(false) + expect(@composer.state.showcc).toBe(true) + + # Simulate a change event fired by the DraftStoreProxy + @draft.bcc = [u2] + @composer._onDraftChanged() + expect(@composer.state.showbcc).toBe(true) + expect(@composer.state.showcc).toBe(true) + describe "When sending a message", -> beforeEach -> spyOn(atom, "isMainWindow").andReturn true diff --git a/internal_packages/message-list/lib/message-list.cjsx b/internal_packages/message-list/lib/message-list.cjsx index 224be24da..b6df93dfc 100755 --- a/internal_packages/message-list/lib/message-list.cjsx +++ b/internal_packages/message-list/lib/message-list.cjsx @@ -4,7 +4,10 @@ classNames = require 'classnames' MessageItem = require "./message-item" {Utils, Actions, + Message, + DraftStore, MessageStore, + DatabaseStore, ComponentRegistry, AddRemoveTagsTask} = require("nylas-exports") @@ -69,8 +72,8 @@ class MessageList extends React.Component commands = _.extend {}, 'core:star-item': => @_onStar() - 'application:reply': => @_onReply() - 'application:reply-all': => @_onReplyAll() + 'application:reply': => @_createReplyOrUpdateExistingDraft('reply') + 'application:reply-all': => @_createReplyOrUpdateExistingDraft('reply-all') 'application:forward': => @_onForward() @command_unsubscriber = atom.commands.add('body', commands) @@ -103,7 +106,7 @@ class MessageList extends React.Component if newMessageIds.length > 0 @_prepareContentForDisplay() else if newDraftIds.length > 0 - @_focusDraft(@refs["composerItem-#{newDraftIds[0]}"]) + @_focusDraft(@_getDraftElement(newDraftIds[0])) @_prepareContentForDisplay() _newDraftIds: (prevState) => @@ -116,9 +119,68 @@ class MessageList extends React.Component newMessageIds = _.map(_.reject((@state.messages ? []), (m) -> m.draft), (m) -> m.id) return _.difference(newMessageIds, oldMessageIds) ? [] + _getDraftElement: (draftId) => + @refs["composerItem-#{draftId}"] + _focusDraft: (draftElement) => draftElement.focus() + _createReplyOrUpdateExistingDraft: (type) => + unless type in ['reply', 'reply-all'] + throw new Error("_createReplyOrUpdateExistingDraft called with #{type}, not reply or reply-all") + return unless @state.currentThread + + last = _.last(@state.messages ? []) + + # If the last message on the thread is already a draft, fetch the message it's + # in reply to and the draft session and change the participants. + if last.draft is true + data = + session: DraftStore.sessionForLocalId(@state.messageLocalIds[last.id]) + replyToMessage: Promise.resolve(@state.messages[@state.messages.length - 2]) + + if last.replyToMessageId + msg = _.findWhere(@state.messages, {id: last.replyToMessageId}) + if msg + data.replyToMessage = Promise.resolve(msg) + else + data.replyToMessage = DatabaseStore.find(Message, last.replyToMessageId) + + Promise.props(data).then ({session, replyToMessage}) => + return unless replyToMessage and session + draft = session.draft() + updated = {to: [].concat(draft.to), cc: [].concat(draft.cc)} + + replySet = replyToMessage.participantsForReply() + replyAllSet = replyToMessage.participantsForReplyAll() + + if type is 'reply' + targetSet = replySet + + # Remove participants present in the reply-all set and not the reply set + for key in ['to', 'cc'] + updated[key] = _.reject updated[key], (contact) -> + inReplySet = _.findWhere(replySet[key], {email: contact.email}) + inReplyAllSet = _.findWhere(replyAllSet[key], {email: contact.email}) + return inReplyAllSet and not inReplySet + else + # Add participants present in the reply-all set and not on the draft + # Switching to reply-all shouldn't really ever remove anyone. + targetSet = replyAllSet + + for key in ['to', 'cc'] + for contact in targetSet[key] + updated[key].push(contact) unless _.findWhere(updated[key], {email: contact.email}) + + session.changes.add(updated) + @_focusDraft(@_getDraftElement(last.id)) + + else + if type is 'reply' + Actions.composeReply(thread: @state.currentThread, message: last) + else + Actions.composeReplyAll(thread: @state.currentThread, message: last) + _onStar: => return unless @state.currentThread if @state.currentThread.isStarred() @@ -127,14 +189,6 @@ class MessageList extends React.Component task = new AddRemoveTagsTask(@state.currentThread, ['starred'], []) Actions.queueTask(task) - _onReply: => - return unless @state.currentThread - Actions.composeReply(thread: @state.currentThread) - - _onReplyAll: => - return unless @state.currentThread - Actions.composeReplyAll(thread: @state.currentThread) - _onForward: => return unless @state.currentThread Actions.composeForward(thread: @state.currentThread) @@ -191,17 +245,14 @@ class MessageList extends React.Component # Either returns "reply" or "reply-all" _replyType: => lastMsg = _.last(_.filter((@state.messages ? []), (m) -> not m.draft)) - return "reply" if lastMsg?.cc.length is 0 and lastMsg?.to.length is 1 - return "reply-all" + if lastMsg?.cc.length is 0 and lastMsg?.to.length is 1 + return "reply" + else + return "reply-all" _onClickReplyArea: => - return unless @state.currentThread?.id - lastMsg = _.last(_.filter((@state.messages ? []), (m) -> not m.draft)) - - if @_replyType() is "reply-all" - Actions.composeReplyAll(thread: @state.currentThread, message: lastMsg) - else - Actions.composeReply(thread: @state.currentThread, message: lastMsg) + return unless @state.currentThread + @_createReplyOrUpdateExistingDraft(@_replyType()) # There may be a lot of iframes to load which may take an indeterminate # amount of time. As long as there is more content being painted onto @@ -264,7 +315,7 @@ class MessageList extends React.Component if message.draft components.push else @@ -345,7 +396,7 @@ class MessageList extends React.Component # If messageId and location are defined, that means we want to scroll # smoothly to the top of a particular message. _onRequestScrollToComposer: ({messageId, location, selectionTop}={}) => - composer = React.findDOMNode(@refs["composerItem-#{messageId}"]) + composer = React.findDOMNode(@_getDraftElement(messageId)) if selectionTop messageWrap = React.findDOMNode(@refs.messageWrap) wrapRect = messageWrap.getBoundingClientRect() @@ -356,7 +407,6 @@ class MessageList extends React.Component else done = -> location ?= "bottom" - composer = React.findDOMNode(@refs["composerItem-#{messageId}"]) @scrollToMessage(composer, done, location, 1) _makeRectVisible: (rect) -> diff --git a/internal_packages/message-list/spec/message-list-spec.cjsx b/internal_packages/message-list/spec/message-list-spec.cjsx index f3f113e3d..213f74c5c 100644 --- a/internal_packages/message-list/spec/message-list-spec.cjsx +++ b/internal_packages/message-list/spec/message-list-spec.cjsx @@ -11,6 +11,7 @@ TestUtils = React.addons.TestUtils Actions, Message, Namespace, + DraftStore, MessageStore, NamespaceStore, NylasTestUtils, @@ -28,31 +29,25 @@ MessageList = proxyquire("../lib/message-list", { MessageParticipants = require "../lib/message-participants" -me = new Namespace( - "name": "User One", - "email": "user1@nylas.com" - "provider": "inbox" -) +me = new Namespace + name: "User One", + emailAddress: "user1@nylas.com" + provider: "inbox" NamespaceStore._current = me -user_headers = - id: null - object: null - namespace_id: null - -user_1 = _.extend _.clone(user_headers), +user_1 = new Contact name: "User One" email: "user1@nylas.com" -user_2 = _.extend _.clone(user_headers), +user_2 = new Contact name: "User Two" email: "user2@nylas.com" -user_3 = _.extend _.clone(user_headers), +user_3 = new Contact name: "User Three" email: "user3@nylas.com" -user_4 = _.extend _.clone(user_headers), +user_4 = new Contact name: "User Four" email: "user4@nylas.com" -user_5 = _.extend _.clone(user_headers), +user_5 = new Contact name: "User Five" email: "user5@nylas.com" @@ -270,6 +265,102 @@ describe "MessageList", -> cs = TestUtils.scryRenderedDOMComponentsWithClass(@messageList, "footer-reply-area") expect(cs.length).toBe 0 + describe "reply behavior (_createReplyOrUpdateExistingDraft)", -> + beforeEach -> + @messageList.setState(currentThread: test_thread) + + it "should throw an exception unless you provide `reply` or `reply-all`", -> + expect( => @messageList._createReplyOrUpdateExistingDraft('lala')).toThrow() + + describe "when there is already a draft at the bottom of the thread", -> + beforeEach -> + @replyToMessage = new Message + id: "reply-id", + threadId: test_thread.id + date: new Date() + @draft = new Message + id: "666", + draft: true, + date: new Date() + replyToMessage: @replyToMessage.id + + spyOn(@messageList, '_focusDraft') + spyOn(@replyToMessage, 'participantsForReplyAll').andCallFake -> + {to: [user_3], cc: [user_2, user_4] } + spyOn(@replyToMessage, 'participantsForReply').andCallFake -> + {to: [user_3], cc: [] } + + MessageStore._items = [@replyToMessage, @draft] + MessageStore.trigger() + @messageList.setState(currentThread: test_thread) + + @sessionStub = + draft: => @draft + changes: + add: jasmine.createSpy('session.changes.add') + spyOn(DraftStore, 'sessionForLocalId').andCallFake => + Promise.resolve(@sessionStub) + + it "should not fire a composer action", -> + spyOn(Actions, 'composeReplyAll') + @messageList._createReplyOrUpdateExistingDraft('reply-all') + advanceClock() + expect(Actions.composeReplyAll).not.toHaveBeenCalled() + + it "should focus the existing draft", -> + @messageList._createReplyOrUpdateExistingDraft('reply-all') + advanceClock() + expect(@messageList._focusDraft).toHaveBeenCalled() + + describe "when reply-all is passed", -> + it "should add missing participants", -> + @draft.to = [ user_3 ] + @draft.cc = [] + @messageList._createReplyOrUpdateExistingDraft('reply-all') + advanceClock() + expect(@sessionStub.changes.add).toHaveBeenCalledWith({to: [user_3], cc: [user_2, user_4]}) + + it "should not blow away other participants who have been added to the draft", -> + user_random_a = new Contact(email: 'other-guy-a@gmail.com') + user_random_b = new Contact(email: 'other-guy-b@gmail.com') + @draft.to = [ user_3, user_random_a ] + @draft.cc = [ user_random_b ] + @messageList._createReplyOrUpdateExistingDraft('reply-all') + advanceClock() + expect(@sessionStub.changes.add).toHaveBeenCalledWith({to: [user_3, user_random_a], cc: [user_random_b, user_2, user_4]}) + + describe "when reply is passed", -> + it "should remove participants present in the reply-all participant set and not in the reply set", -> + @draft.to = [ user_3 ] + @draft.cc = [ user_2, user_4 ] + @messageList._createReplyOrUpdateExistingDraft('reply') + advanceClock() + expect(@sessionStub.changes.add).toHaveBeenCalledWith({to: [user_3], cc: []}) + + it "should not blow away other participants who have been added to the draft", -> + user_random_a = new Contact(email: 'other-guy-a@gmail.com') + user_random_b = new Contact(email: 'other-guy-b@gmail.com') + @draft.to = [ user_3, user_random_a ] + @draft.cc = [ user_2, user_4, user_random_b ] + @messageList._createReplyOrUpdateExistingDraft('reply') + advanceClock() + expect(@sessionStub.changes.add).toHaveBeenCalledWith({to: [user_3, user_random_a], cc: [user_random_b]}) + + describe "when there is not an existing draft at the bottom of the thread", -> + beforeEach -> + MessageStore._items = [m5, m3] + MessageStore.trigger() + @messageList.setState(currentThread: test_thread) + + it "should fire a composer action based on the reply type", -> + spyOn(Actions, 'composeReplyAll') + @messageList._createReplyOrUpdateExistingDraft('reply-all') + expect(Actions.composeReplyAll).toHaveBeenCalledWith(thread: test_thread, message: m3) + + spyOn(Actions, 'composeReply') + @messageList._createReplyOrUpdateExistingDraft('reply') + expect(Actions.composeReply).toHaveBeenCalledWith(thread: test_thread, message: m3) + describe "Message minification", -> beforeEach -> @messageList.MINIFY_THRESHOLD = 3 diff --git a/src/components/scroll-region.cjsx b/src/components/scroll-region.cjsx index 8053c42a6..492849ba0 100644 --- a/src/components/scroll-region.cjsx +++ b/src/components/scroll-region.cjsx @@ -199,7 +199,8 @@ class ScrollRegion extends React.Component _setSharedState: (state) -> scrollbar = @props.getScrollbar?() ? @refs.scrollbar - scrollbar.setStateFromScrollRegion(state) + if scrollbar + scrollbar.setStateFromScrollRegion(state) @setState(state) _onScroll: (event) => diff --git a/src/flux/models/message.coffee b/src/flux/models/message.coffee index 3193a6be7..c44e10709 100644 --- a/src/flux/models/message.coffee +++ b/src/flux/models/message.coffee @@ -4,6 +4,7 @@ File = require './file' Model = require './model' Contact = require './contact' Attributes = require '../attributes' +NamespaceStore = require '../stores/namespace-store' ### Public: The Message model represents a Message object served by the Nylas Platform API. @@ -177,6 +178,40 @@ class Message extends Model participants["#{(contact?.email ? "").toLowerCase().trim()} #{(contact?.name ? "").toLowerCase().trim()}"] = contact if contact? return _.values(participants) + # Returns a hash with `to` and `cc` keys for authoring a new draft in response + # to this message. Takes `replyTo` and other important state into account. + participantsForReplyAll: -> + to = [] + cc = [] + + if @from[0].email is NamespaceStore.current().emailAddress + to = @to + cc = @cc + else + excluded = @from.map (c) -> c.email + excluded.push(NamespaceStore.current().emailAddress) + if @replyTo.length + to = @replyTo + else + to = @from + cc = [].concat(@cc, @to).filter (p) -> + !_.contains(excluded, p.email) + + {to, cc} + + participantsForReply: -> + to = [] + cc = [] + + if @from[0].email is NamespaceStore.current().emailAddress + to = @to + else if @replyTo.length + to = @replyTo + else + to = @from + + {to, cc} + # Public: Returns an {Array} of {File} IDs fileIds: -> _.map @files, (file) -> file.id diff --git a/src/flux/stores/draft-store.coffee b/src/flux/stores/draft-store.coffee index 760079f6d..a51e32528 100644 --- a/src/flux/stores/draft-store.coffee +++ b/src/flux/stores/draft-store.coffee @@ -179,18 +179,9 @@ class DraftStore return unless containsDraft @trigger(change) - _isMe: (contact={}) => - contact.email is NamespaceStore.current().me().email - _onComposeReply: (context) => @_newMessageWithContext context, (thread, message) => - if @_isMe(message.from[0]) - to = message.to - else if message.replyTo.length - to = message.replyTo - else - to = message.from - + {to, cc} = message.participantsForReply() return { replyToMessage: message to: to @@ -198,19 +189,7 @@ class DraftStore _onComposeReplyAll: (context) => @_newMessageWithContext context, (thread, message) => - if @_isMe(message.from[0]) - to = message.to - cc = message.cc - else - excluded = message.from.map (c) -> c.email - excluded.push(NamespaceStore.current().me().email) - if message.replyTo.length - to = message.replyTo - else - to = message.from - cc = [].concat(message.cc, message.to).filter (p) -> - !_.contains(excluded, p.email) - + {to, cc} = message.participantsForReplyAll() return { replyToMessage: message to: to