fix(message-list): Adds fade-in, fixes jerky scroll offset when viewing messages

Summary:
feat(debugging): Put db query tiemstamps in js timelines

Mark as read *after* you stop looking at a message, to avoid pointless repaint while looking

fix specs :'(

Make focus() do something smart if not provided a field name

Stop doing the partial load / full load from cache. Slower, but also means we can evaluate "correct" scroll offset in one pass

refactor message-list to fade in after load. simplifies scroll logic

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1306

exports/ui-components.coffee

@@ -3,6 +3,7 @@
 module.exports =
   # Models
   Menu: require '../src/components/menu'
+  Spinner: require '../src/components/spinner'
   Popover: require '../src/components/popover'
   Flexbox: require '../src/components/flexbox'
   RetinaImg: require '../src/components/retina-img'

internal_packages/composer/lib/composer-view.cjsx

@@ -114,7 +114,7 @@ ComposerView = React.createClass
       </div>
 
   _wrapClasses: ->
-    "composer-outer-wrap #{@props.containerClass ? ""}"
+    "composer-outer-wrap #{@props.className ? ""}"
 
   _renderComposer: ->
     <div className="composer-inner-wrap" onDragOver={@_onDragNoop} onDragLeave={@_onDragNoop} onDragEnd={@_onDragNoop} onDrop={@_onDrop}>
@@ -218,7 +218,18 @@ ComposerView = React.createClass
 
     </div>
 
-  focus: (field) -> @refs[field]?.focus?() if @isMounted()
+  # Focus the composer view. Chooses the appropriate field to start
+  # focused depending on the draft type, or you can pass a field as
+  # the first parameter.
+  focus: (field = null) ->
+    return unless @isMounted()
+
+    if component?.isForwardedMessage()
+      field ?= "textFieldTo"
+    else
+      field ?= "contentBody"
+
+    @refs[field]?.focus?()
 
   isForwardedMessage: ->
     draft = @_proxy.draft()

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

@@ -2,6 +2,7 @@ _ = require 'underscore-plus'
 React = require 'react'
 MessageItem = require "./message-item.cjsx"
 {Actions, ThreadStore, MessageStore, ComponentRegistry} = require("inbox-exports")
+{Spinner} = require('ui-components')
 
 module.exports =
 MessageList = React.createClass
@@ -16,51 +17,43 @@ MessageList = React.createClass
     @_unsubscribers = []
     @_unsubscribers.push MessageStore.listen @_onChange
     @_unsubscribers.push ThreadStore.listen @_onChange
-    @_lastHeight = -1
-    @_scrollToBottom()
+    if @state.messages.length > 0
+      @_prepareContentForDisplay()
 
   componentWillUnmount: ->
     unsubscribe() for unsubscribe in @_unsubscribers
 
-  componentWillUpdate: (nextProps, nextState) ->
-    newDraftIds = @_newDraftIds(nextState)
-    if newDraftIds.length >= 1
-      @_focusComposerId = newDraftIds[0]
-
   componentDidUpdate: (prevProps, prevState) ->
-    if @_shouldScroll(prevState)
-      @_lastHeight = -1
-      @_scrollToBottom()
-      if @_focusComposerId?
-        @_focusRef(@refs["composerItem-#{@_focusComposerId}"])
-        @_focusComposerId = null
+    didLoad = prevState.messages.length is 0 and @state.messages.length > 0
 
-  # Only scroll if the messages change and there are some message
-  _shouldScroll: (prevState) ->
-    return false if (@state.messages ? []).length is 0
-    prevMsg = (prevState.messages ? []).map((m) -> m.id)
-    curMsg = (@state.messages ? []).map((m) -> m.id)
-    return true if prevMsg.length isnt curMsg.length
-    iLength = _.intersection(prevMsg, curMsg).length
-    return true if iLength isnt prevMsg.length or iLength isnt curMsg.length
-    return false
+    oldDraftIds = _.map(_.filter((prevState.messages ? []), (m) -> m.draft), (m) -> m.id)
+    newDraftIds = _.map(_.filter((@state.messages ? []), (m) -> m.draft), (m) -> m.id)
+    addedDraftIds = _.difference(newDraftIds, oldDraftIds)
+    didAddDraft = addedDraftIds.length > 0
 
+    if didLoad
+      @_prepareContentForDisplay()
 
-  # We need a 100ms delay so the DOM can finish painting the elements on
-  # the page. The focus doesn't work for some reason while the paint is in
-  # process.
-  _focusRef: (component) -> _.delay ->
-    if component?.isForwardedMessage()
-      component?.focus("textFieldTo")
-    else
-      component?.focus("contentBody")
-  , 100
+    else if didAddDraft
+      @_focusDraft(@refs["composerItem-#{addedDraftIds[0]}"])
+
+  _focusDraft: (draftDOMNode) ->
+    # We need a 100ms delay so the DOM can finish painting the elements on
+    # the page. The focus doesn't work for some reason while the paint is in
+    # process.
+    _.delay =>
+      return unless @isMounted
+      draftDOMNode.focus()
+    ,100
 
   render: ->
     return <div></div> if not @state.currentThread?
+    wrapClass = React.addons.classSet
+      "messages-wrap": true
+      "ready": @state.ready
 
     <div className="message-list" id="message-list">
-      <div tabIndex=1 ref="messageWrap" className="messages-wrap">
+      <div tabIndex=1 className={wrapClass} ref="messageWrap">
         <div className="message-list-notification-bars">
           {@_messageListNotificationBars()}
         </div>
@@ -68,8 +61,35 @@ MessageList = React.createClass
         {@_messageListHeaders()}
         {@_messageComponents()}
       </div>
+      <Spinner visible={!@state.ready} />
     </div>
 
+  # 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
+  # the page and our height is changing, keep waiting. Then scroll to message.
+  scrollToMessage: (msgDOMNode, done) ->
+    return done() unless msgDOMNode?
+
+    messageWrap = @refs.messageWrap?.getDOMNode()
+    lastHeight = -1
+    stableCount = 0
+    scrollIfSettled = =>
+      return done() unless @isMounted()
+
+      messageWrapHeight = messageWrap.getBoundingClientRect().height
+      if messageWrapHeight isnt lastHeight
+        lastHeight = messageWrapHeight
+        stableCount = 0
+      else
+        stableCount += 1
+        if stableCount is 5
+          messageWrap.scrollTop = msgDOMNode.offsetTop
+          return done()
+
+      window.requestAnimationFrame -> scrollIfSettled(msgDOMNode, done)
+  
+    scrollIfSettled()
+
   _messageListNotificationBars: ->
     MLBars = ComponentRegistry.findAllViewsByRole('MessageListNotificationBar')
     <div className="message-list-notification-bar-wrap">
@@ -88,39 +108,34 @@ MessageList = React.createClass
       }
     </div>
 
-  _newDraftIds: (nextState) ->
-    currentMsgIds = _.map(_.filter((@state.messages ? []), (m) -> not m.draft), (m) -> m.id)
-    nextMsgIds = _.map(_.filter((nextState.messages ? []), (m) -> not m.draft), (m) -> m.id)
-
-    # Only return if all the non-draft messages are the same. If the
-    # non-draft messages aren't the same, that means we switched threads.
-    # Don't focus on new drafts if we just switched threads.
-    if nextMsgIds.length > 0 and _.difference(nextMsgIds, currentMsgIds).length is 0
-      nextDraftIds = _.map(_.filter((nextState.messages ? []), (m) -> m.draft), (m) -> m.id)
-      currentDraftIds = _.map(_.filter((@state.messages ? []), (m) -> m.draft), (m) -> m.id)
-      return (_.difference(nextDraftIds, currentDraftIds) ? [])
-    else return []
-
   _messageComponents: ->
     ComposerItem = @state.Composer
-    # containsUnread = _.any @state.messages, (m) -> m.unread
-    collapsed = false
+    appliedInitialFocus = false
     components = []
 
-    @state.messages?.forEach (message) =>
+    @state.messages?.forEach (message, idx) =>
+      initialFocus = not appliedInitialFocus and
+                    ((message.draft) or
+                     (message.unread) or
+                     (idx is @state.messages.length - 1 and idx > 0))
+      appliedInitialFocus ||= initialFocus
+
+      className = React.addons.classSet
+        "message-item-wrap": true
+        "initial-focus": initialFocus
+        "unread": message.unread
+        "draft": message.draft
+
       if message.draft
         components.push <ComposerItem mode="inline"
                          ref="composerItem-#{message.id}"
                          key={@state.messageLocalIds[message.id]}
                          localId={@state.messageLocalIds[message.id]}
-                         containerClass="message-item-wrap draft-message"/>
+                         className={className} />
       else
-        className = "message-item-wrap"
-        if message.unread then className += " unread-message"
         components.push <MessageItem key={message.id}
                          thread={@state.currentThread}
                          message={message}
-                         collapsed={collapsed}
                          className={className}
                          thread_participants={@_threadParticipants()} />
 
@@ -133,6 +148,12 @@ MessageList = React.createClass
     messages: (MessageStore.items() ? [])
     messageLocalIds: MessageStore.itemLocalIds()
     currentThread: ThreadStore.selectedThread()
+    ready: if MessageStore.itemsLoading() then false else @state?.ready ? false
+
+  _prepareContentForDisplay: ->
+    focusedMessage = @getDOMNode().querySelector(".initial-focus")
+    @scrollToMessage focusedMessage, =>
+      @setState(ready: true)
 
   _threadParticipants: ->
     # We calculate the list of participants instead of grabbing it from
@@ -147,36 +168,5 @@ MessageList = React.createClass
           participants[contact.email] = contact
     return _.values(participants)
 
-  # 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
-  # the page, we keep trying to scroll to the bottom. We scroll to the top
-  # of the last message.
-  #
-  # We don't scroll if there's only 1 item.
-  # We don't scroll if you're actively focused somewhere in the message
-  # list.
-  _scrollToBottom: ->
-    _.defer =>
-      if @isMounted()
-        messageWrap = @refs?.messageWrap?.getDOMNode?()
-
-        return if not messageWrap?
-        items = messageWrap.querySelectorAll(".message-item-wrap")
-        return if items.length <= 1
-        return if @getDOMNode().contains document.activeElement
-
-        msgToScroll = messageWrap.querySelector(".draft-message, .unread-message")
-        if not msgToScroll?
-          msgToScroll = messageWrap.children[messageWrap.children.length - 1]
-
-        currentHeight = messageWrap.getBoundingClientRect().height
-
-        if currentHeight isnt @_lastHeight
-          @_lastHeight = currentHeight
-          @_scrollToBottom()
-        else
-          scrollTo = currentHeight - msgToScroll.getBoundingClientRect().height
-          @getDOMNode().scrollTop = scrollTo
-
 MessageList.minWidth = 680
 MessageList.maxWidth = 900

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

@@ -194,6 +194,8 @@ describe "MessageList", ->
     MessageStore._threadId = null
     spyOn(MessageStore, "itemLocalIds").andCallFake ->
       {"666": "666"}
+    spyOn(MessageStore, "itemsLoading").andCallFake ->
+      false
 
     @message_list = TestUtils.renderIntoDocument(<MessageList />)
     @message_list_node = @message_list.getDOMNode()
@@ -235,34 +237,27 @@ describe "MessageList", ->
     #   expect(items.length).toBe 1
 
     it "focuses new composers when a draft is added", ->
-      spyOn(@message_list, "_focusRef")
+      spyOn(@message_list, "_focusDraft")
       msgs = @message_list.state.messages
-      msgs = msgs.concat(draftMessages)
-      @message_list.setState messages: msgs
+
+      @message_list.setState
+        messages: msgs.concat(draftMessages)
+
       items = TestUtils.scryRenderedComponentsWithType(@message_list,
               ComposerItem)
+
       expect(items.length).toBe 1
-      composer = items[0]
-      expect(@message_list._focusRef).toHaveBeenCalledWith(composer)
+      expect(@message_list._focusDraft).toHaveBeenCalledWith(items[0])
 
     it "doesn't focus if we're just navigating through messages", ->
-      spyOn(@message_list, "_focusRef")
+      spyOn(@message_list, "scrollToMessage")
       @message_list.setState messages: draftMessages
       items = TestUtils.scryRenderedComponentsWithType(@message_list,
               ComposerItem)
       expect(items.length).toBe 1
       composer = items[0]
-      expect(@message_list._focusRef).not.toHaveBeenCalled()
+      expect(@message_list.scrollToMessage).not.toHaveBeenCalled()
 
-    describe "Message", ->
-      beforeEach ->
-        items = TestUtils.scryRenderedComponentsWithType(@message_list,
-                MessageItem)
-        item = items.filter (message) -> message.props.message.id is "111"
-        @message_item = item[0]
-
-      it "finds the message by id", ->
-        expect(@message_item.props.message.id).toBe "111"
 
   describe "MessageList with draft", ->
     beforeEach ->

internal_packages/message-list/stylesheets/message-list.less

@@ -29,8 +29,9 @@
   position: relative;
 
   width: 100%;
+  height: 100%;
+  min-height: 100%;
   padding: 0;
-  overflow-y: auto;
   order: 2;
 
   .message-list-headers {
@@ -47,6 +48,18 @@
     }
   }
 
+  .messages-wrap {
+    overflow-y: auto;
+    position: absolute;
+    top:0; left:0; right:0; bottom:0;
+    opacity:0;
+    transition: opacity 0s;
+  }
+  .messages-wrap.ready {
+    opacity:1;
+    transition: opacity .1s linear;
+  }
+
   .message-item-wrap {
     position: relative;
     width: 100%;

src/components/spinner.cjsx

@@ -0,0 +1,44 @@
+React = require 'react'
+_ = require 'underscore-plus'
+
+module.exports =
+Spinner = React.createClass
+  propTypes:
+    visible: React.PropTypes.bool
+    style: React.PropTypes.object
+
+  getInitialState: ->
+    hidden: false
+    paused: false
+
+  componentWillReceiveProps: (nextProps) ->
+    hidden = if nextProps.visible? then !nextProps.visible else false
+
+    if @state.hidden is false and hidden is true
+      @setState({hidden: true})
+      setTimeout =>
+        return unless @isMounted()
+        @setState({paused: true})
+      ,250
+    else if @state.hidden is true and hidden is false
+      @setState({paused: false, hidden: false})
+
+  render: ->
+    spinnerClass = React.addons.classSet
+      'spinner': true
+      'hidden': @state.hidden
+      'paused': @state.paused
+
+    style = _.extend @props.style ? {},
+      'position':'absolute'
+      'left': '50%'
+      'top': '50%'
+      'transform':'translate(-50%,-50%);'
+
+    otherProps = _.omit(@props, _.keys(@constructor.propTypes))
+
+    <div className={spinnerClass} {...otherProps} style={style}>
+      <div className="bounce1"></div>
+      <div className="bounce2"></div>
+      <div className="bounce3"></div>
+    </div>

src/flux/models/utils.coffee

@@ -159,6 +159,7 @@ Utils =
   # shown. Needs to be limited to first 250 to prevent replies to
   # forwarded messages from also being expanded.
   isForwardedMessage: (body="", subject="") ->
+    body = body.toLowerCase()
     indexForwarded = body.indexOf('forwarded')
     indexFwd = body.indexOf('fwd')
 

src/flux/stores/database-store.coffee

@@ -26,6 +26,7 @@ class DatabaseProxy
 
     ipc.on 'database-result', ({queryKey, err, result}) =>
       @queryCallbacks[queryKey](err, result) if @queryCallbacks[queryKey]
+      console.timeStamp("DB END #{queryKey}. #{result?.length} chars")
       delete @queryCallbacks[queryKey]
 
     @
@@ -34,6 +35,7 @@ class DatabaseProxy
     @queryId += 1
     queryKey = "#{@windowId}-#{@queryId}"
     @queryCallbacks[queryKey] = callback if callback
+    console.timeStamp("DB SEND #{queryKey}: #{query}")
     ipc.send('database-query', {@databasePath, queryKey, query, values})
 
 # DatabasePromiseTransaction converts the callback syntax of the Database

src/flux/stores/message-store.coffee

@@ -14,8 +14,14 @@ MessageStore = Reflux.createStore
 
   ########### PUBLIC #####################################################
 
-  items: -> @_items
-  itemLocalIds: -> @_itemsLocalIds
+  items: ->
+    @_items
+  
+  itemLocalIds: ->
+    @_itemsLocalIds
+  
+  itemsLoading: ->
+    @_threadId? and @_items.length is 0
 
   ########### PRIVATE ####################################################
 
@@ -45,8 +51,7 @@ MessageStore = Reflux.createStore
 
     # Fetch messages from cache. Fetch a few now,
     # and debounce loading all of them
-    @_fetchFromCache({preview: true})
-    @_fetchFromCacheDebounced()
+    @_fetchFromCache()
 
     # Fetch messages from API, only if the user
     # sits on this message for a moment
@@ -56,7 +61,6 @@ MessageStore = Reflux.createStore
     loadedThreadId = @_threadId
 
     query = DatabaseStore.findAll(Message, threadId: loadedThreadId)
-    query.limit(2) if options.preview
     query.then (items) =>
       localIds = {}
       async.each items, (item, callback) ->
@@ -80,10 +84,6 @@ MessageStore = Reflux.createStore
 
         @trigger(@)
 
-  _fetchFromCacheDebounced: _.debounce ->
-    @_fetchFromCache()
-  , 100
-
   _fetchFromAPIDebounced: _.debounce ->
     return unless @_threadId?
     # Fetch messages from API, which triggers an update to the database,

src/flux/stores/thread-store.coffee

@@ -103,14 +103,16 @@ ThreadStore = Reflux.createStore
     @fetchFromAPI()
 
   _onSelectThreadId: (id) ->
-    return if @_selectedId == id
-    @_selectedId = id
-    @trigger()
-
+    # Mark the *previously* selected thread as read,
+    # before we bring in the next thread
     thread = @selectedThread()
     if thread && thread.isUnread()
       thread.markAsRead()
 
+    return if @_selectedId == id
+    @_selectedId = id
+    @trigger()
+
   # Accessing Data
 
   selectedTagId: ->

static/components/spinner.less

@@ -0,0 +1,61 @@
+@import "ui-variables";
+
+.spinner {
+  margin: 0;
+  width: 94px;
+  text-align: center;
+  opacity: 1;
+  -webkit-transition: opacity 0.2s linear 0.3s; //transition in
+}
+.spinner.hidden {
+  opacity: 0;
+  -webkit-transition: opacity 0.2s linear; //transition out
+}
+.spinner.paused {
+  > div {
+    // important. animating with opacity 0 chews up about 5% cpu
+    -webkit-animation-play-state: paused;
+    animation-play-state: paused;
+  }
+}
+
+.spinner > div {
+  width: 18px;
+  height: 18px;
+  background-color: @gray-light;
+
+  border-radius: 100%;
+  display: inline-block;
+  -webkit-animation: bouncedelay 1.2s infinite ease-in-out;
+  animation: bouncedelay 1.2s infinite ease-in-out;
+  /* Prevent first frame from flickering when animation starts */
+  -webkit-animation-fill-mode: both;
+  animation-fill-mode: both;
+  margin-right:4px;
+  margin-left:4px;
+}
+
+.spinner .bounce1 {
+  -webkit-animation-delay: -0.32s;
+  animation-delay: -0.32s;
+}
+
+.spinner .bounce2 {
+  -webkit-animation-delay: -0.16s;
+  animation-delay: -0.16s;
+}
+
+@-webkit-keyframes bouncedelay {
+  0%, 80%, 100% { -webkit-transform: scale(0.0) }
+  40% { -webkit-transform: scale(1.0) }
+}
+
+@keyframes bouncedelay {
+  0%, 80%, 100% { 
+    transform: scale(0.0);
+    -webkit-transform: scale(0.0);
+  } 40% { 
+    transform: scale(1.0);
+    -webkit-transform: scale(1.0);
+  }
+}

static/index.less

@@ -17,3 +17,4 @@
 @import "components/tokenizing-text-field";
 @import "components/extra";
 @import "components/list-tabular";
+@import "components/spinner";