fix(composer): Fix several composer issues and refactor Contenteditable

Summary:
  - Fixes T5819 issues
  - Adds ContenteditbalePlugin mechanism to allow extension of Contenteditable
    functionality, and completely removes lifecycleCallbacks from Contenteditable
  - Refactors list functionality outside of Contenteditable and into a plugin
  - Updates ComposerView to apply DraftStoreExtensions through a ContentEditablePlugin
  - Moves spell checking logic outside of Contenteditable into the spellcheck package

Fixes T5824 (atom.assert)
Fixes T5951 (shift-tabbing) bullets

Test Plan: - Unit tests and manual

Reviewers: evan, bengotow

Reviewed By: bengotow

Maniphest Tasks: T5951, T5824, T5819

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

internal_packages/composer-spellcheck/lib/draft-extension.coffee

@@ -1,19 +1,47 @@
-{DraftStoreExtension, AccountStore} = require 'nylas-exports'
+{DraftStoreExtension, AccountStore, DOMUtils} = require 'nylas-exports'
 _ = require 'underscore'
+spellchecker = require('spellchecker')
+remote = require('remote')
+MenuItem = remote.require('menu-item')
 
 SpellcheckCache = {}
 
 class SpellcheckDraftStoreExtension extends DraftStoreExtension
 
   @isMisspelled: (word) ->
-    @spellchecker ?= require('spellchecker')
-    SpellcheckCache[word] ?= @spellchecker.isMisspelled(word)
+    SpellcheckCache[word] ?= spellchecker.isMisspelled(word)
     SpellcheckCache[word]
 
-  @onComponentDidUpdate: (editableNode) ->
+  @onInput: (editableNode) ->
     @walkTree(editableNode)
 
-  @onLearnSpelling: (editableNode, word) ->
+  @onShowContextMenu: (event, editableNode, selection, innerStateProxy, menu) =>
+    range = DOMUtils.Mutating.getRangeAtAndSelectWord(selection, 0)
+    word = range.toString()
+    if @isMisspelled(word)
+      corrections = spellchecker.getCorrectionsForMisspelling(word)
+      if corrections.length > 0
+        corrections.forEach (correction) =>
+          menu.append(new MenuItem({
+            label: correction,
+            click: @applyCorrection.bind(@, editableNode, range, selection, correction)
+          }))
+      else
+        menu.append(new MenuItem({ label: 'No Guesses Found', enabled: false}))
+
+      menu.append(new MenuItem({ type: 'separator' }))
+      menu.append(new MenuItem({
+        label: 'Learn Spelling',
+        click: @learnSpelling.bind(@, editableNode, word)
+      }))
+      menu.append(new MenuItem({ type: 'separator' }))
+
+  @applyCorrection: (editableNode, range, selection, correction) =>
+    DOMUtils.Mutating.applyTextInRange(range, selection, correction)
+    @walkTree(editableNode)
+
+  @learnSpelling: (editableNode, word) =>
+    spellchecker.add(word)
     delete SpellcheckCache[word]
     @walkTree(editableNode)
 

internal_packages/composer/lib/composer-extensions-plugin.coffee

@@ -0,0 +1,27 @@
+{DraftStore, DOMUtils, ContenteditablePlugin} = require 'nylas-exports'
+
+class ComposerExtensionsPlugin extends ContenteditablePlugin
+  @onInput: (event, editableNode, selection, innerStateProxy) ->
+    for extension in DraftStore.extensions()
+      extension.onInput?(editableNode, event)
+
+  @onKeyDown: (event, editableNode, selection, innerStateProxy) ->
+    if event.key is "Tab"
+      range = DOMUtils.getRangeInScope(editableNode)
+      for extension in DraftStore.extensions()
+        extension.onTabDown?(editableNode, range, event)
+
+  @onShowContextMenu: (args...) ->
+    for extension in DraftStore.extensions()
+      extension.onShowContextMenu?(args...)
+
+  @onClick: (event, editableNode, selection, innerStateProxy) ->
+    range = DOMUtils.getRangeInScope(editableNode)
+    return unless range
+    try
+      for extension in DraftStore.extensions()
+        extension.onMouseUp?(editableNode, range, event)
+    catch e
+      console.error('DraftStore extension raised an error: '+e.toString())
+
+module.exports = ComposerExtensionsPlugin

internal_packages/composer/lib/composer-view.cjsx

@@ -29,6 +29,8 @@ CollapsedParticipants = require './collapsed-participants'
 
 Fields = require './fields'
 
+ComposerExtensionsPlugin = require './composer-extensions-plugin'
+
 # The ComposerView is a unique React component because it (currently) is a
 # singleton. Normally, the React way to do things would be to re-render the
 # Composer with new props.
@@ -78,7 +80,7 @@ class ComposerView extends React.Component
     @_usubs = []
     @_usubs.push FileUploadStore.listen @_onFileUploadStoreChange
     @_usubs.push AccountStore.listen @_onAccountStoreChanged
-    @_applyFocusedField()
+    @_applyFieldFocus()
 
   componentWillUnmount: =>
     @_unmounted = true # rarf
@@ -94,7 +96,7 @@ class ComposerView extends React.Component
     # re-rendering.
     @_recoveredSelection = null if @_recoveredSelection?
 
-    @_applyFocusedField()
+    @_applyFieldFocus()
 
   _keymapHandlers: ->
     'composer:send-message': => @_sendDraft()
@@ -109,14 +111,18 @@ class ComposerView extends React.Component
     "composer:undo": @undo
     "composer:redo": @redo
 
-  _applyFocusedField: ->
-    if @state.focusedField
+  _applyFieldFocus: ->
+    if @state.focusedField and @_lastFocusedField isnt @state.focusedField
+      @_lastFocusedField = @state.focusedField
       return unless @refs[@state.focusedField]
       if @refs[@state.focusedField].focus
         @refs[@state.focusedField].focus()
       else
         React.findDOMNode(@refs[@state.focusedField]).focus()
 
+      if @state.focusedField is Fields.Body
+        @refs[Fields.Body].selectEnd()
+
   componentWillReceiveProps: (newProps) =>
     @_ignoreNextTrigger = false
     if newProps.draftClientId isnt @props.draftClientId
@@ -223,7 +229,10 @@ class ComposerView extends React.Component
 
       {@_renderSubject()}
 
-      <div className="compose-body" ref="composeBody" onClick={@_onClickComposeBody}>
+      <div className="compose-body"
+           ref="composeBody"
+           onMouseUp={@_onMouseUpComposerBody}
+           onMouseDown={@_onMouseDownComposerBody}>
         {@_renderBody()}
         {@_renderFooterRegions()}
       </div>
@@ -291,39 +300,10 @@ class ComposerView extends React.Component
       onScrollTo={@props.onRequestScrollTo}
       onFilePaste={@_onFilePaste}
       onScrollToBottom={@_onScrollToBottom()}
-      lifecycleCallbacks={@_contenteditableLifecycleCallbacks()}
+      plugins={[ComposerExtensionsPlugin]}
       getComposerBoundingRect={@_getComposerBoundingRect}
       initialSelectionSnapshot={@_recoveredSelection} />
 
-  _contenteditableLifecycleCallbacks: ->
-    componentDidUpdate: (editableNode) =>
-      for extension in DraftStore.extensions()
-        extension.onComponentDidUpdate?(editableNode)
-
-    onInput: (editableNode, event) =>
-      for extension in DraftStore.extensions()
-        extension.onInput?(editableNode, event)
-
-    onTabDown: (editableNode, event, range) =>
-      for extension in DraftStore.extensions()
-        extension.onTabDown?(editableNode, range, event)
-
-    onSubstitutionPerformed: (editableNode) =>
-      for extension in DraftStore.extensions()
-        extension.onSubstitutionPerformed?(editableNode)
-
-    onLearnSpelling: (editableNode, text) =>
-      for extension in DraftStore.extensions()
-        extension.onLearnSpelling?(editableNode, text)
-
-    onMouseUp: (editableNode, event, range) =>
-      return unless range
-      try
-        for extension in DraftStore.extensions()
-          extension.onMouseUp?(editableNode, range, event)
-      catch e
-        console.error('DraftStore extension raised an error: '+e.toString())
-
   # The contenteditable decides when to request a scroll based on the
   # position of the cursor and its relative distance to this composer
   # component. We provide it our boundingClientRect so it can calculate
@@ -472,8 +452,23 @@ class ComposerView extends React.Component
   # This lets us click outside of the `contenteditable`'s `contentBody`
   # and simulate what happens when you click beneath the text *in* the
   # contentEditable.
-  _onClickComposeBody: (event) =>
-    @refs[Fields.Body].selectEnd()
+  #
+  # Unfortunately, we need to manually keep track of the "click" in
+  # separate mouseDown, mouseUp events because we need to ensure that the
+  # start and end target are both not in the contenteditable. This ensures
+  # that this behavior doesn't interfear with a click and drag selection.
+  _onMouseDownComposerBody: (event) =>
+    if React.findDOMNode(@refs[Fields.Body]).contains(event.target)
+      @_mouseDownTarget = null
+    else @_mouseDownTarget = event.target
+
+  _onMouseUpComposerBody: (event) =>
+    if event.target is @_mouseDownTarget
+      @refs[Fields.Body].selectEnd()
+    @_mouseDownTarget = null
+
+  _onMouseMoveComposeBody: (event) =>
+    if @_mouseComposeBody is "down" then @_mouseComposeBody = "move"
 
   _onDraftChanged: =>
     return if @_ignoreNextTrigger

spec/components/contenteditable/automatic-list-manager-spec.coffee

@@ -0,0 +1,105 @@
+
+xdescribe "ListManager", ->
+  beforeEach ->
+    @ce = new ContenteditableTestHarness
+
+  it "Creates ordered lists", ->
+    @ce.type ['1', '.', ' ']
+    @ce.expectHTML "<ol><li></li></ol>"
+    @ce.expectSelection (dom) ->
+      dom.querySelectorAll("li")[0]
+
+  it "Undoes ordered list creation with backspace", ->
+    @ce.type ['1', '.', ' ', 'backspace']
+    @ce.expectHTML "1.&nbsp;"
+    @ce.expectSelection (dom) ->
+      node: dom.childNodes[0]
+      offset: 3
+
+  it "Creates unordered lists with star", ->
+    @ce.type ['*', ' ']
+    @ce.expectHTML "<ul><li></li></ul>"
+    @ce.expectSelection (dom) ->
+      dom.querySelectorAll("li")[0]
+
+  it "Undoes unordered list creation with backspace", ->
+    @ce.type ['*', ' ', 'backspace']
+    @ce.expectHTML "*&nbsp;"
+    @ce.expectSelection (dom) ->
+      node: dom.childNodes[0]
+      offset: 2
+
+  it "Creates unordered lists with dash", ->
+    @ce.type ['-', ' ']
+    @ce.expectHTML "<ul><li></li></ul>"
+    @ce.expectSelection (dom) ->
+      dom.querySelectorAll("li")[0]
+
+  it "Undoes unordered list creation with backspace", ->
+    @ce.type ['-', ' ', 'backspace']
+    @ce.expectHTML "-&nbsp;"
+    @ce.expectSelection (dom) ->
+      node: dom.childNodes[0]
+      offset: 2
+
+  it "create a single item then delete it with backspace", ->
+    @ce.type ['-', ' ', 'a', 'left', 'backspace']
+    @ce.expectHTML "a"
+    @ce.expectSelection (dom) ->
+      node: dom.childNodes[0]
+      offset: 0
+
+  it "create a single item then delete it with tab", ->
+    @ce.type ['-', ' ', 'a', 'shift-tab']
+    @ce.expectHTML "a"
+    @ce.expectSelection (dom) -> dom.childNodes[0]
+      node: dom.childNodes[0]
+      offset: 1
+
+  describe "when creating two items in a list", ->
+    beforeEach ->
+      @twoItemKeys = ['-', ' ', 'a', 'enter', 'b']
+
+    it "creates two items with enter at end", ->
+      @ce.type @twoItemKeys
+      @ce.expectHTML "<ul><li>a</li><li>b</li></ul>"
+      @ce.expectSelection (dom) ->
+        node: dom.querySelectorAll('li')[1].childNodes[0]
+        offset: 1
+
+    it "backspace from the start of the 1st item outdents", ->
+      @ce.type @twoItemKeys.concat ['left', 'up', 'backspace']
+
+    it "backspace from the start of the 2nd item outdents", ->
+      @ce.type @twoItemKeys.concat ['left', 'backspace']
+
+    it "shift-tab from the start of the 1st item outdents", ->
+      @ce.type @twoItemKeys.concat ['left', 'up', 'shift-tab']
+
+    it "shift-tab from the start of the 2nd item outdents", ->
+      @ce.type @twoItemKeys.concat ['left', 'shift-tab']
+
+    it "shift-tab from the end of the 1st item outdents", ->
+      @ce.type @twoItemKeys.concat ['up', 'shift-tab']
+
+    it "shift-tab from the end of the 2nd item outdents", ->
+      @ce.type @twoItemKeys.concat ['shift-tab']
+
+    it "backspace from the end of the 1st item doesn't outdent", ->
+      @ce.type @twoItemKeys.concat ['up', 'backspace']
+
+    it "backspace from the end of the 2nd item doesn't outdent", ->
+      @ce.type @twoItemKeys.concat ['backspace']
+
+  describe "multi-depth bullets", ->
+    it "creates multi level bullet when tabbed in", ->
+      @ce.type ['-', ' ', 'a', 'tab']
+
+    it "creates multi level bullet when tabbed in", ->
+      @ce.type ['-', ' ', 'tab', 'a']
+
+    it "returns to single level bullet on backspace", ->
+      @ce.type ['-', ' ', 'a', 'tab', 'left', 'backspace']
+
+    it "returns to single level bullet on shift-tab", ->
+      @ce.type ['-', ' ', 'a', 'tab', 'shift-tab']

src/components/contenteditable/contenteditable-plugin.coffee

@@ -0,0 +1,26 @@
+###
+ContenteditablePlugin is an abstract base class. Implementations of this
+are used to make additional changes to a <Contenteditable /> component
+beyond a user's input intents.
+
+While some ContenteditablePlugins are included with the core
+<Contenteditable /> component, others may be added via the `plugins`
+prop.
+###
+class ContenteditablePlugin
+
+  # The onInput event can be triggered by a variety of events, some of
+  # which could have been already been looked at by a callback.
+  # Pretty much any DOM mutation will fire this.
+  # Sometimes those mutations are the cause of callbacks.
+  @onInput: (event, editableNode, selection, innerStateProxy) ->
+
+  @onBlur: (event, editableNode, selection, innerStateProxy) ->
+
+  @onFocus: (event, editableNode, selection, innerStateProxy) ->
+
+  @onClick: (event, editableNode, selection, innerStateProxy) ->
+
+  @onKeyDown: (event, editableNode, selection, innerStateProxy) ->
+
+  @onShowContextMenu: (event, editableNode, selection, innerStateProxy, menu) ->