feat(message-list): better sending state

Summary:
We now have a `MessageItemContainer` class that handles the logic of
deciding what kind of message to show. We introduce a new `PendingMessage`
(which is just a sublcass of `MessageItem`) that has the spinner and
stuff.

Also tests

Test Plan: edgehill --test

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1833

internal_packages/composer/lib/composer-view.cjsx

@@ -60,7 +60,6 @@ class ComposerView extends React.Component
       showbcc: false
       showsubject: false
       showQuotedText: false
-      isSending: DraftStore.isSendingDraft(@props.localId)
       uploads: FileUploadStore.uploadsForMessage(@props.localId) ? []
 
   componentWillMount: =>
@@ -71,7 +70,6 @@ class ComposerView extends React.Component
     not Utils.isEqualReact(nextState, @state)
 
   componentDidMount: =>
-    @_draftStoreUnlisten = DraftStore.listen @_onSendingStateChanged
     @_uploadUnlisten = FileUploadStore.listen @_onFileUploadStoreChange
     @_keymapUnlisten = atom.commands.add '.composer-outer-wrap', {
       'composer:show-and-focus-bcc': @_showAndFocusBcc
@@ -92,7 +90,6 @@ class ComposerView extends React.Component
     @_teardownForDraft()
     @_deleteDraftIfEmpty()
     @_uploadUnlisten() if @_uploadUnlisten
-    @_draftStoreUnlisten() if @_draftStoreUnlisten
     @_keymapUnlisten.dispose() if @_keymapUnlisten
 
   componentDidUpdate: =>
@@ -165,7 +162,6 @@ class ComposerView extends React.Component
               shouldAcceptDrop={@_shouldAcceptDrop}
               onDragStateChange={ ({isDropping}) => @setState({isDropping}) }
               onDrop={@_onDrop}>
-      <div className="composer-cover" style={display: if @state.isSending then 'block' else 'none'}></div>
       <div className="composer-drop-cover" style={display: if @state.isDropping then 'block' else 'none'}>
         <div className="centered">
           <RetinaImg name="composer-drop-to-attach.png" mode={RetinaImg.Mode.ContentIsMask}/>
@@ -572,10 +568,10 @@ class ComposerView extends React.Component
   _sendDraft: (options = {}) =>
     return unless @_proxy
 
-    # We need to check the `DraftStore` instead of `@state.isSending`
-    # because the `DraftStore` is immediately and synchronously updated as
-    # soon as this function fires. Since `setState` is asynchronous, if we
-    # used that as our only check, then we might get a false reading.
+    # We need to check the `DraftStore` because the `DraftStore` is
+    # immediately and synchronously updated as soon as this function
+    # fires. Since `setState` is asynchronous, if we used that as our only
+    # check, then we might get a false reading.
     return if DraftStore.isSendingDraft(@props.localId)
 
     draft = @_proxy.draft()
@@ -629,11 +625,6 @@ class ComposerView extends React.Component
           @_sendDraft({force: true})
       return
 
-    # There can be a delay between when the send request gets initiated
-    # by a user and when the draft is prepared on on the TaskQueue, which
-    # is how we detect that the draft is sending.
-    @setState(isSending: true)
-
     Actions.sendDraft(@props.localId)
 
   _mentionsAttachment: (body) =>
@@ -654,11 +645,6 @@ class ComposerView extends React.Component
     @setState {showcc: true}
     @focus('textFieldCc')
 
-  _onSendingStateChanged: =>
-    isSending = DraftStore.isSendingDraft(@props.localId)
-    if isSending isnt @state.isSending
-      @setState({isSending})
-
   _onEmptyCc: =>
     @setState showcc: false
     @focus('textFieldTo')

internal_packages/composer/spec/composer-view-spec.cjsx

@@ -473,33 +473,6 @@ describe "populated composer", ->
       expect(Actions.sendDraft).toHaveBeenCalledWith(DRAFT_LOCAL_ID)
       expect(Actions.sendDraft.calls.length).toBe 1
 
-    it "disables the composer once sending has started", ->
-      useFullDraft.apply(@); makeComposer.call(@)
-      sendBtn = React.findDOMNode(@composer.refs.sendButton)
-      cover = ReactTestUtils.findRenderedDOMComponentWithClass(@composer, "composer-cover")
-      expect(React.findDOMNode(cover).style.display).toBe "none"
-      ReactTestUtils.Simulate.click sendBtn
-      @isSending.state = true
-      DraftStore.trigger()
-      expect(React.findDOMNode(cover).style.display).toBe "block"
-      expect(@composer.state.isSending).toBe true
-
-    it "re-enables the composer if sending threw an error", ->
-      @isSending.state = null
-      useFullDraft.apply(@); makeComposer.call(@)
-      sendBtn = React.findDOMNode(@composer.refs.sendButton)
-      ReactTestUtils.Simulate.click sendBtn
-
-      @isSending.state = true
-      DraftStore.trigger()
-
-      expect(@composer.state.isSending).toBe true
-
-      @isSending.state = false
-      DraftStore.trigger()
-
-      expect(@composer.state.isSending).toBe false
-
     describe "when sending a message with keyboard inputs", ->
       beforeEach ->
         useFullDraft.apply(@)

internal_packages/composer/stylesheets/composer.less

@@ -15,13 +15,6 @@
   display: flex;
   flex-direction: column;
 
-  .composer-cover {
-    position: absolute;
-    top: -1 * @spacing-double; right: 0; bottom: 0; left: 0;
-    z-index: 1000;
-    background: rgba(255,255,255,0.7);
-  }
-
   .composer-drop-cover {
     position: absolute;
     top: 0; right: 0; bottom: 0; left: 0;

internal_packages/message-list/lib/message-item-container.cjsx

@@ -0,0 +1,85 @@
+React = require 'react'
+classNames = require 'classnames'
+
+MessageItem = require './message-item'
+PendingMessageItem = require './pending-message-item'
+
+{DraftStore,
+ MessageStore} = require 'nylas-exports'
+
+{InjectedComponent} = require 'nylas-component-kit'
+
+class MessageItemContainer extends React.Component
+  @displayName = 'MessageItemContainer'
+
+  @propTypes =
+    thread: React.PropTypes.object.isRequired
+    message: React.PropTypes.object.isRequired
+
+    # The messageId (in the case of draft's local ID) is a derived
+    # property that only the parent MessageList knows about.
+    messageId: React.PropTypes.string
+
+    collapsed: React.PropTypes.bool
+    isLastMsg: React.PropTypes.bool
+    isBeforeReplyArea: React.PropTypes.bool
+    onRequestScrollTo: React.PropTypes.func
+
+  constructor: (@props) ->
+    @state = @_getStateFromStores()
+
+  componentWillReceiveProps: (newProps) ->
+    @setState(@_getStateFromStores())
+
+  componentDidMount: =>
+    if @props.message?.draft
+      @unlisten = DraftStore.listen @_onSendingStateChanged
+
+  componentWillUnmount: =>
+    @unlisten() if @unlisten
+
+  focus: => @refs.message.focus?()
+
+  render: =>
+    if @props.message.draft
+      if @state.isSending
+        @_renderMessage(PendingMessageItem)
+      else
+        @_renderComposer()
+    else
+      @_renderMessage(MessageItem)
+
+  _renderMessage: (component) =>
+    <component ref="message"
+               thread={@props.thread}
+               message={@props.message}
+               className={@_classNames()}
+               collapsed={@props.collapsed}
+               isLastMsg={@props.isLastMsg} />
+
+  _renderComposer: =>
+    props =
+      mode: "inline"
+      localId: @props.messageId
+      threadId: @props.thread.id
+      onRequestScrollTo: @props.onRequestScrollTo
+
+    <InjectedComponent ref="message"
+                       matching={role: "Composer"}
+                       className={@_classNames()}
+                       exposedProps={props} />
+
+  _classNames: => classNames
+    "draft": @props.message.draft
+    "unread": @props.message.unread
+    "collapsed": @props.collapsed
+    "message-item-wrap": true
+    "before-reply-area": @props.isBeforeReplyArea
+
+  _onSendingStateChanged: (draftLocalId) =>
+    @setState(@_getStateFromStores()) if draftLocalId is @props.messageId
+
+  _getStateFromStores: ->
+    isSending: DraftStore.isSendingDraft(@props.messageId)
+
+module.exports = MessageItemContainer

internal_packages/message-list/lib/message-item.cjsx

@@ -92,12 +92,14 @@ class MessageItem extends React.Component
   _renderHeader: =>
     <header className="message-header" onClick={@_onClickHeader}>
 
+      {@_renderHeaderSideItems()}
+
       <div className="message-header-right">
         <MessageTimestamp className="message-time selectable"
                           isDetailed={@state.detailedHeaders}
                           date={@props.message.date} />
 
-        <MessageControls thread={@props.thread} message={@props.message}/>
+        {@_renderMessageControls()}
 
         <InjectedComponentSet className="message-indicator"
                               matching={role: "MessageIndicator"}
@@ -114,10 +116,13 @@ class MessageItem extends React.Component
                            message_participants={@props.message.participants()} />
 
       {@_renderFolder()}
-      {@_renderHeaderExpansionControl()}
+      {@_renderHeaderDetailToggle()}
 
     </header>
 
+  _renderMessageControls: ->
+    <MessageControls thread={@props.thread} message={@props.message}/>
+
   _renderFolder: =>
     return [] unless @state.detailedHeaders and @props.message.folder
     <div className="header-row">
@@ -125,7 +130,6 @@ class MessageItem extends React.Component
       <div className="header-name">{@props.message.folder.displayName}</div>
     </div>
 
-
   _onClickParticipants: (e) =>
     el = e.target
     while el isnt e.currentTarget
@@ -167,7 +171,11 @@ class MessageItem extends React.Component
     'no-quoted-text': not QuotedHTMLParser.hasQuotedHTML(@props.message.body)
     'show-quoted-text': @state.showQuotedText
 
-  _renderHeaderExpansionControl: =>
+  # This is used by subclasses of MessageItem.
+  # See {PendingMessageItem}
+  _renderHeaderSideItems: -> []
+
+  _renderHeaderDetailToggle: =>
     if @state.detailedHeaders
       <div className="header-toggle-control"
            style={top: "18px", left: "-14px"}

internal_packages/message-list/lib/message-list.cjsx

@@ -1,7 +1,8 @@
 _ = require 'underscore'
 React = require 'react'
 classNames = require 'classnames'
-MessageItem = require "./message-item"
+MessageItemContainer = require './message-item-container'
+
 {Utils,
  Actions,
  Message,
@@ -119,15 +120,15 @@ class MessageList extends React.Component
 
     newDraftIds = @_newDraftIds(prevState)
     if newDraftIds.length > 0
-      @_focusDraft(@_getMessageElement(newDraftIds[0]))
+      @_focusDraft(@_getMessageContainer(newDraftIds[0]))
 
   _newDraftIds: (prevState) =>
     oldDraftIds = _.map(_.filter((prevState.messages ? []), (m) -> m.draft), (m) -> m.id)
     newDraftIds = _.map(_.filter((@state.messages ? []), (m) -> m.draft), (m) -> m.id)
     return _.difference(newDraftIds, oldDraftIds) ? []
 
-  _getMessageElement: (id) =>
-    @refs["message-#{id}"]
+  _getMessageContainer: (id) =>
+    @refs["message-container-#{id}"]
 
   _focusDraft: (draftElement) =>
     draftElement.focus()
@@ -184,7 +185,7 @@ class MessageList extends React.Component
             updated[key].push(contact) unless _.findWhere(updated[key], {email: contact.email})
 
         session.changes.add(updated)
-        @_focusDraft(@_getMessageElement(last.id))
+        @_focusDraft(@_getMessageContainer(last.id))
 
     else
       if type is 'reply'
@@ -227,7 +228,7 @@ class MessageList extends React.Component
             matching={role:"MessageListHeaders"}
             exposedProps={thread: @state.currentThread}/>
         </div>
-        {@_messageComponents()}
+        {@_messageElements()}
       </ScrollRegion>
       <Spinner visible={@state.loading} />
     </div>
@@ -271,44 +272,39 @@ class MessageList extends React.Component
     return unless @state.currentThread
     @_createReplyOrUpdateExistingDraft(@_replyType())
 
-  _messageComponents: =>
-    components = []
+  _messageElements: =>
+    elements = []
 
     messages = @_messagesWithMinification(@state.messages)
     messages.forEach (message, idx) =>
 
       if message.type is "minifiedBundle"
-        components.push(@_renderMinifiedBundle(message))
+        elements.push(@_renderMinifiedBundle(message))
         return
 
       collapsed = !@state.messagesExpandedState[message.id]
+      isLastMsg = (messages.length - 1 is idx)
+      isBeforeReplyArea = isLastMsg and @_hasReplyArea()
 
-      className = classNames
-        "message-item-wrap": true
-        "before-reply-area": (messages.length - 1 is idx) and @_hasReplyArea()
-        "unread": message.unread
-        "draft": message.draft
-        "collapsed": collapsed
+      messageId = @state.messageLocalIds[message.id]
+      messageId ?= message.id
 
-      if message.draft
-        components.push <InjectedComponent matching={role:"Composer"}
-                         exposedProps={ mode:"inline", localId:@state.messageLocalIds[message.id], onRequestScrollTo:@_onChildScrollRequest, threadId:@state.currentThread.id }
-                         ref={"message-#{message.id}"}
-                         key={@state.messageLocalIds[message.id]}
-                         className={className} />
-      else
-        components.push <MessageItem key={message.id}
-                         thread={@state.currentThread}
-                         ref={"message-#{message.id}"}
-                         message={message}
-                         className={className}
-                         collapsed={collapsed}
-                         isLastMsg={(messages.length - 1 is idx)} />
+      elements.push(
+        <MessageItemContainer key={idx}
+                              ref={"message-container-#{message.id}"}
+                              thread={@state.currentThread}
+                              message={message}
+                              messageId={messageId}
+                              collapsed={collapsed}
+                              isLastMsg={isLastMsg}
+                              isBeforeReplyArea={isBeforeReplyArea}
+                              onRequestScrollTo={@_onChildScrollRequest} />
+      )
 
     if @_hasReplyArea()
       components.push @_renderReplyArea()
 
-    return components
+    return elements
 
   _renderMinifiedBundle: (bundle) ->
     BUNDLE_HEIGHT = 36
@@ -375,7 +371,7 @@ class MessageList extends React.Component
   # smoothly to the top of a particular message.
   _onChildScrollRequest: ({messageId, rect}={}) =>
     if messageId
-      @refs.messageWrap.scrollTo(@_getMessageElement(messageId), {
+      @refs.messageWrap.scrollTo(@_getMessageContainer(messageId), {
         position: ScrollRegion.ScrollPosition.Visible
       })
     else if rect

internal_packages/message-list/lib/pending-message-item.cjsx

@@ -0,0 +1,25 @@
+React = require 'react'
+{RetinaImg} = require 'nylas-component-kit'
+MessageItem = require './message-item'
+
+class PendingMessageItem extends MessageItem
+  @displayName = 'PendingMessageItem'
+
+  _renderMessageControls: -> null
+
+  _renderHeaderDetailToggle: -> null
+
+  _renderHeaderSideItems: ->
+    styles =
+      width: 24
+      float: "left"
+      marginTop: -2
+      marginRight: 10
+
+    <div style={styles}>
+      <RetinaImg ref="spinner"
+                 name="sending-spinner.gif"
+                 mode={RetinaImg.Mode.ContentPreserve}/>
+    </div>
+
+module.exports = PendingMessageItem

internal_packages/message-list/spec/message-item-container-spec.cjsx

@@ -0,0 +1,58 @@
+React = require "react/addons"
+proxyquire = require("proxyquire").noPreserveCache()
+ReactTestUtils = React.addons.TestUtils
+
+{Thread,
+ Message,
+ DraftStore} = require 'nylas-exports'
+
+class MessageItem extends React.Component
+  @displayName: "StubMessageItem"
+  render: -> <span></span>
+
+class PendingMessageItem extends MessageItem
+
+MessageItemContainer = proxyquire '../lib/message-item-container',
+  "./message-item": MessageItem
+  "./pending-message-item": PendingMessageItem
+
+{InjectedComponent} = require 'nylas-component-kit'
+
+testThread = new Thread(id: "t1")
+testMessageId = "m1"
+testMessage = new Message(id: "m1", draft: false, unread: true)
+testDraft = new Message(id: "d1", draft: true, unread: true)
+
+describe 'MessageItemContainer', ->
+
+  beforeEach ->
+    @isSendingDraft = false
+    spyOn(DraftStore, "isSendingDraft").andCallFake => @isSendingDraft
+
+  renderContainer = (message) ->
+    ReactTestUtils.renderIntoDocument(
+      <MessageItemContainer thread={testThread}
+                            message={message}
+                            messageId={testMessageId} />
+    )
+
+  it "shows composer if it's a draft", ->
+    @isSendingDraft = false
+    doc = renderContainer(testDraft)
+    items = ReactTestUtils.scryRenderedComponentsWithType(doc,
+            InjectedComponent)
+    expect(items.length).toBe 1
+
+  it "shows a pending message if it's a sending draft", ->
+    @isSendingDraft = true
+    doc = renderContainer(testDraft)
+    items = ReactTestUtils.scryRenderedComponentsWithType(doc,
+            PendingMessageItem)
+    expect(items.length).toBe 1
+
+  it "renders a message if it's not a draft", ->
+    @isSendingDraft = false
+    doc = renderContainer(testMessage)
+    items = ReactTestUtils.scryRenderedComponentsWithType(doc,
+            MessageItem)
+    expect(items.length).toBe 1

internal_packages/message-list/spec/message-list-spec.cjsx

@@ -1,6 +1,6 @@
 _ = require "underscore"
 moment = require "moment"
-proxyquire = require "proxyquire"
+proxyquire = require("proxyquire").noPreserveCache()
 
 CSON = require "season"
 React = require "react/addons"
@@ -19,15 +19,19 @@ TestUtils = React.addons.TestUtils
 
 {InjectedComponent} = require 'nylas-component-kit'
 
+MessageParticipants = require "../lib/message-participants"
+
 MessageItem = proxyquire("../lib/message-item", {
   "./email-frame": React.createClass({render: -> <div></div>})
 })
 
-MessageList = proxyquire("../lib/message-list", {
+MessageItemContainer = proxyquire("../lib/message-item-container", {
   "./message-item": MessageItem
+  "./pending-message-item": MessageItem
 })
 
-MessageParticipants = require "../lib/message-participants"
+MessageList = proxyquire '../lib/message-list',
+  "./message-item-container": MessageItemContainer
 
 me = new Namespace
   name: "User One",
@@ -180,7 +184,7 @@ describe "MessageList", ->
 
   it "by default has zero children", ->
     items = TestUtils.scryRenderedComponentsWithType(@messageList,
-            MessageItem)
+            MessageItemContainer)
 
     expect(items.length).toBe 0
 
@@ -195,11 +199,11 @@ describe "MessageList", ->
 
     it "renders all the correct number of messages", ->
       items = TestUtils.scryRenderedComponentsWithType(@messageList,
-              MessageItem)
+              MessageItemContainer)
       expect(items.length).toBe 5
 
     it "renders the correct number of expanded messages", ->
-      msgs = TestUtils.scryRenderedDOMComponentsWithClass(@messageList, "message-item-wrap collapsed")
+      msgs = TestUtils.scryRenderedDOMComponentsWithClass(@messageList, "collapsed message-item-wrap")
       expect(msgs.length).toBe 4
 
     it "displays lists of participants on the page", ->
@@ -220,7 +224,15 @@ describe "MessageList", ->
         messages: msgs.concat(draftMessages)
 
       expect(@messageList._focusDraft).toHaveBeenCalled()
-      expect(@messageList._focusDraft.mostRecentCall.args[0].props.exposedProps.localId).toEqual(draftMessages[0].id)
+      expect(@messageList._focusDraft.mostRecentCall.args[0].props.messageId).toEqual(draftMessages[0].id)
+
+    it "includes drafts as message item containers", ->
+      msgs = @messageList.state.messages
+      @messageList.setState
+        messages: msgs.concat(draftMessages)
+      items = TestUtils.scryRenderedComponentsWithType(@messageList,
+              MessageItemContainer)
+      expect(items.length).toBe 6
 
   describe "MessageList with draft", ->
     beforeEach ->

src/flux/stores/draft-store.coffee

@@ -397,6 +397,7 @@ class DraftStore
   # The user request to send the draft
   _onSendDraft: (draftLocalId) =>
     @_pendingEnqueue[draftLocalId] = true
+    @trigger(draftLocalId)
     @sessionForLocalId(draftLocalId).then (session) =>
       @_runExtensionsBeforeSend(session)
 
@@ -415,7 +416,7 @@ class DraftStore
             # the line, we'll make a new session and handle them later
             @_doneWithSession(session)
             @_pendingEnqueue[draftLocalId] = false
-            @trigger()
+            @trigger(draftLocalId)
 
         Actions.queueTask(task)
         @_doneWithSession(session)

src/flux/tasks/send-draft.coffee

@@ -39,6 +39,7 @@ class SendDraftTask extends Task
     # recent draft version
     DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) =>
       # The draft may have been deleted by another task. Nothing we can do.
+      NylasAPI.incrementOptimisticChangeCount(Message, draft.id)
       @draft = draft
       if not draft
         return Promise.reject(new Error("We couldn't find the saved draft."))
@@ -72,6 +73,7 @@ class SendDraftTask extends Task
         return Promise.resolve(Task.Status.Finished)
 
     .catch APIError, (err) =>
+      NylasAPI.decrementOptimisticChangeCount(Message, @draft.id)
       if err.message?.indexOf('Invalid message public id') is 0
         body.reply_to_message_id = null
         return @_send(body)