Remove the concept of focus from the model

spec/focusable-spec.coffee

@@ -1,36 +0,0 @@
-{Model} = require 'theorist'
-Focusable = require '../src/focusable'
-FocusContext = require '../src/focus-context'
-
-describe "Focusable mixin", ->
-  it "ensures that only a single model is focused for a given focus manager", ->
-    class Item extends Model
-      Focusable.includeInto(this)
-
-    focusContext = new FocusContext
-    item1 = new Item({focusContext})
-    item2 = new Item({focusContext})
-    item3 = new Item({focusContext})
-
-    expect(focusContext.focusedObject).toBe null
-    expect(item1.focused).toBe false
-    expect(item2.focused).toBe false
-    expect(item3.focused).toBe false
-
-    item1.focus()
-    expect(focusContext.focusedObject).toBe item1
-    expect(item1.focused).toBe true
-    expect(item2.focused).toBe false
-    expect(item3.focused).toBe false
-
-    item2.focus()
-    expect(focusContext.focusedObject).toBe item2
-    expect(item1.focused).toBe false
-    expect(item2.focused).toBe true
-    expect(item3.focused).toBe false
-
-    item2.blur()
-    expect(focusContext.focusedObject).toBe null
-    expect(item1.focused).toBe false
-    expect(item2.focused).toBe false
-    expect(item3.focused).toBe false

spec/pane-container-model-spec.coffee

@@ -16,11 +16,10 @@ describe "PaneContainerModel", ->
 
       containerB = containerA.testSerialization()
       [pane1B, pane2B, pane3B] = containerB.getPanes()
-      expect(pane3B.focusContext).toBe containerB.focusContext
       expect(pane3B.focused).toBe true
 
     it "preserves the active pane across serialization, independent of focus", ->
-      pane3A.blur()
+      pane3A.makeActive()
       expect(containerA.activePane).toBe pane3A
 
       containerB = containerA.testSerialization()
@@ -34,30 +33,30 @@ describe "PaneContainerModel", ->
       pane1 = new PaneModel
       container = new PaneContainerModel(root: pane1)
 
-    it "references the first pane if no pane has been focused", ->
+    it "references the first pane if no pane has been made active", ->
       expect(container.activePane).toBe pane1
       expect(pane1.active).toBe true
 
-    it "references the most recently focused pane", ->
+    it "references the most pane on which ::makeActive was most recently called", ->
       pane2 = pane1.splitRight()
+      pane2.makeActive()
       expect(container.activePane).toBe pane2
       expect(pane1.active).toBe false
       expect(pane2.active).toBe true
-      pane1.focus()
+      pane1.makeActive()
       expect(container.activePane).toBe pane1
       expect(pane1.active).toBe true
       expect(pane2.active).toBe false
 
-    it "is reassigned to the next pane if the current active pane is unfocused and destroyed", ->
+    it "is reassigned to the next pane if the current active pane is destroyed", ->
       pane2 = pane1.splitRight()
-      pane2.blur()
+      pane2.makeActive()
       pane2.destroy()
       expect(container.activePane).toBe pane1
       expect(pane1.active).toBe true
       pane1.destroy()
       expect(container.activePane).toBe null
 
-  # TODO: Remove this behavior when we have a proper workspace model we can explicitly focus
   describe "when the last pane is removed", ->
     [container, pane, surrenderedFocusHandler] = []
 
@@ -70,14 +69,3 @@ describe "PaneContainerModel", ->
       pane.destroy()
       expect(container.root).toBe null
       expect(container.activePane).toBe null
-
-    describe "if the pane is focused", ->
-      it "emits a 'surrendered-focus' event", ->
-        pane.focus()
-        pane.destroy()
-        expect(surrenderedFocusHandler).toHaveBeenCalled()
-
-    describe "if the pane is not focused", ->
-      it "does not emit an event", ->
-        expect(pane.focused).toBe false
-        expect(surrenderedFocusHandler).not.toHaveBeenCalled()

spec/pane-container-spec.coffee

@@ -42,7 +42,7 @@ describe "PaneContainer", ->
   describe ".focusPreviousPane()", ->
     it "focuses the pane preceding the focused pane or the last pane if no pane has focus", ->
       container.attachToDom()
-      pane3.focusout()
+      $(document.body).focus() # clear focus
 
       container.focusPreviousPane()
       expect(pane3.activeItem).toMatchSelector ':focus'

spec/pane-model-spec.coffee

@@ -2,7 +2,6 @@
 PaneModel = require '../src/pane-model'
 PaneAxisModel = require '../src/pane-axis-model'
 PaneContainerModel = require '../src/pane-container-model'
-FocusContext = require '../src/focus-context'
 
 describe "PaneModel", ->
   describe "split methods", ->
@@ -80,25 +79,12 @@ describe "PaneModel", ->
           expect(column.orientation).toBe 'vertical'
           expect(column.children).toEqual [pane1, pane3, pane2]
 
-    it "focuses the new pane, even if the current pane isn't focused", ->
+    it "sets up the new pane to be focused", ->
       expect(pane1.focused).toBe false
       pane2 = pane1.splitRight()
       expect(pane2.focused).toBe true
 
   describe "::removeItemAtIndex(index)", ->
-    describe "when the removal of the item causes blur to be called on the pane model", ->
-      it "remains focused if it was before the item was removed", ->
-        pane = new PaneModel(items: ["A", "B", "C"])
-        container = new PaneContainerModel(root: pane)
-        pane.on 'item-removed', -> pane.blur()
-        pane.focus()
-        pane.removeItemAtIndex(0)
-        expect(pane.focused).toBe true
-
-        pane.blur()
-        pane.removeItemAtIndex(0)
-        expect(pane.focused).toBe false
-
     describe "when the last item is removed", ->
       it "destroys the pane", ->
         pane = new PaneModel(items: ["A", "B"])
@@ -146,13 +132,3 @@ describe "PaneModel", ->
         expect(container.root.children).toEqual [pane1, pane2]
         pane2.destroy()
         expect(container.root).toBe pane1
-
-    describe "if the pane is focused", ->
-      it "shifts focus to the next pane", ->
-        pane2 = pane1.splitRight()
-        pane3 = pane2.splitRight()
-        pane2.focus()
-        expect(pane2.focused).toBe true
-        expect(pane3.focused).toBe false
-        pane2.destroy()
-        expect(pane3.focused).toBe true

src/focus-context.coffee

@@ -1,15 +0,0 @@
-{Model} = require 'theorist'
-
-module.exports =
-class FocusContext extends Model
-  @property 'focusedObject', null
-
-  blurSuppressionCounter: 0
-
-  isBlurSuppressed: ->
-    @blurSuppressionCounter > 0
-
-  suppressBlur: (fn) ->
-    @blurSuppressionCounter++
-    fn()
-    @blurSuppressionCounter--

src/focusable.coffee

@@ -1,28 +0,0 @@
-Mixin = require 'mixto'
-
-module.exports =
-class Focusable extends Mixin
-  @included: ->
-    @property 'focusContext'
-    @behavior 'focused', ->
-      @$focusContext
-        .flatMapLatest((context) -> context?.$focusedObject)
-        .map((focusedObject) => focusedObject is this)
-        .distinctUntilChanged()
-
-  focus: ->
-    throw new Error("Object must be assigned a focusContext to be focus") unless @focusContext
-    unless @focused
-      @suppressBlur =>
-        @focusContext.focusedObject = this
-
-  blur: ->
-    throw new Error("Object must be assigned a focusContext to be blurred") unless @focusContext
-    if @focused and not @focusContext.isBlurSuppressed()
-      @focusContext.focusedObject = null
-
-  suppressBlur: (fn) ->
-    if @focusContext?
-      @focusContext.suppressBlur(fn)
-    else
-      fn()

src/pane-axis-model.coffee

@@ -12,8 +12,6 @@ class PaneAxisModel extends Model
   Serializable.includeInto(this)
   Delegator.includeInto(this)
 
-  @delegatesProperty 'focusContext', toProperty: 'container'
-
   constructor: ({@container, @orientation, children}) ->
     @children = Sequence.fromArray(children ? [])
 
@@ -67,5 +65,4 @@ class PaneAxisModel extends Model
     @children.splice(index + 1, 0, newChild)
 
   reparentLastChild: ->
-    @focusContext.suppressBlur =>
-      @parent.replaceChild(this, @children[0])
+    @parent.replaceChild(this, @children[0])

src/pane-axis.coffee

@@ -7,10 +7,8 @@ Pane = null
 module.exports =
 class PaneAxis extends View
   initialize: (@model) ->
-    @subscribe @model.children.onRemoval @onChildRemoved
-    @subscribe @model.children.onEach @onChildAdded
-
-    @onChildAdded(child) for child in children ? []
+    @onChildAdded(child) for child in @model.children
+    @subscribe @model.children, 'changed', @onChildrenChanged
 
   viewForModel: (model) ->
     viewClass = model.getViewClass()
@@ -22,6 +20,12 @@ class PaneAxis extends View
   removeChild: (child) ->
     @model.removeChild(child.model)
 
+  onChildrenChanged:  ({index, removedValues, insertedValues}) =>
+    focusedElement = document.activeElement if @hasFocus()
+    @onChildRemoved(child, index) for child in removedValues
+    @onChildAdded(child, index + i) for child, i in insertedValues
+    focusedElement?.focus() if document.activeElement is document.body
+
   onChildAdded: (child, index) =>
     view = @viewForModel(child)
     @insertAt(index, view)

src/pane-container-model.coffee

@@ -1,7 +1,6 @@
 {Model} = require 'theorist'
 Serializable = require 'serializable'
 {find} = require 'underscore-plus'
-FocusContext = require './focus-context'
 PaneModel = require './pane-model'
 
 module.exports =
@@ -11,7 +10,6 @@ class PaneContainerModel extends Model
 
   @properties
     root: null
-    focusContext: null
     activePane: null
 
   previousRoot: null
@@ -21,13 +19,9 @@ class PaneContainerModel extends Model
 
   constructor: ->
     super
-
-    @focusContext ?= new FocusContext
-
     @subscribe @$root, @onRootChanged
 
   deserializeParams: (params) ->
-    @focusContext ?= params.focusContext ? new FocusContext
     params.root = atom.deserializers.deserialize(params.root, container: this)
     params
 
@@ -41,31 +35,6 @@ class PaneContainerModel extends Model
   getPanes: ->
     @root?.getPanes() ? []
 
-  getFocusedPane: ->
-    find @getPanes(), (pane) -> pane.focused
-
-  focusNextPane: ->
-    panes = @getPanes()
-    if panes.length > 1
-      currentIndex = panes.indexOf(@getFocusedPane())
-      nextIndex = (currentIndex + 1) % panes.length
-      panes[nextIndex].focus()
-      true
-    else
-      @emit 'surrendered-focus'
-      false
-
-  focusPreviousPane: ->
-    panes = @getPanes()
-    if panes.length > 1
-      currentIndex = panes.indexOf(@getFocusedPane())
-      previousIndex = currentIndex - 1
-      previousIndex = panes.length - 1 if previousIndex < 0
-      panes[previousIndex].focus()
-      true
-    else
-      false
-
   makeNextPaneActive: ->
     panes = @getPanes()
     if panes.length > 1
@@ -78,7 +47,10 @@ class PaneContainerModel extends Model
   onRootChanged: (root) =>
     @unsubscribe(@previousRoot) if @previousRoot?
     @previousRoot = root
-    return unless root?
+
+    unless root?
+      @activePane = null
+      return
 
     root.parent = this
     root.container = this

src/pane-container.coffee

@@ -17,8 +17,6 @@ class PaneContainer extends View
   @content: ->
     @div class: 'panes'
 
-  @delegatesMethods 'focusNextPane', 'focusPreviousPane', toProperty: 'model'
-
   initialize: (params) ->
     if params instanceof PaneContainerModel
       @model = params
@@ -27,7 +25,6 @@ class PaneContainer extends View
 
     @subscribe @model.$root, 'value', @onRootChanged
     @subscribe @model.$activePaneItem.changes, 'value', @onActivePaneItemChanged
-    @subscribe @model, 'surrendered-focus', @onSurrenderedFocus
 
   viewForModel: (model) ->
     if model?
@@ -49,6 +46,8 @@ class PaneContainer extends View
     @model.root = root?.model
 
   onRootChanged: (root) =>
+    focusedElement = document.activeElement if @hasFocus()
+
     oldRoot = @getRoot()
     if oldRoot instanceof Pane and oldRoot.model.isDestroyed()
       @trigger 'pane:removed', [oldRoot]
@@ -56,14 +55,11 @@ class PaneContainer extends View
     if root?
       view = @viewForModel(root)
       @append(view)
-      view.makeActive?()
+      focusedElement?.focus()
 
   onActivePaneItemChanged: (activeItem) =>
     @trigger 'pane-container:active-pane-item-changed', [activeItem]
 
-  onSurrenderedFocus: =>
-    atom?.workspaceView?.focus()
-
   removeChild: (child) ->
     throw new Error("Removing non-existant child") unless @getRoot() is child
     @setRoot(null)
@@ -117,3 +113,25 @@ class PaneContainer extends View
   removeEmptyPanes: ->
     for pane in @getPanes() when pane.getItems().length == 0
       pane.remove()
+
+  focusNextPane: ->
+    panes = @getPanes()
+    if panes.length > 1
+      currentIndex = panes.indexOf(@getFocusedPane())
+      nextIndex = (currentIndex + 1) % panes.length
+      panes[nextIndex].focus()
+      true
+    else
+      atom.workspaceView?.focus()
+      false
+
+  focusPreviousPane: ->
+    panes = @getPanes()
+    if panes.length > 1
+      currentIndex = panes.indexOf(@getFocusedPane())
+      previousIndex = currentIndex - 1
+      previousIndex = panes.length - 1 if previousIndex < 0
+      panes[previousIndex].focus()
+      true
+    else
+      false

src/pane-model.coffee

@@ -3,18 +3,17 @@
 {Model, Sequence} = require 'theorist'
 Serializable = require 'serializable'
 PaneAxisModel = require './pane-axis-model'
-Focusable = require './focusable'
 Pane = null
 
 module.exports =
 class PaneModel extends Model
   atom.deserializers.add(this)
   Serializable.includeInto(this)
-  Focusable.includeInto(this)
 
   @properties
     container: null
     activeItem: null
+    focused: false
 
   @behavior 'active', ->
     @$container
@@ -28,9 +27,6 @@ class PaneModel extends Model
     @items = Sequence.fromArray(params?.items ? [])
     @activeItem ?= @items[0]
 
-    @subscribe @$container, (container) =>
-      @focusContext = container?.focusContext
-
     @subscribe @items.onEach (item) =>
       if typeof item.on is 'function'
         @subscribe item, 'destroyed', => @removeItem(item)
@@ -38,9 +34,6 @@ class PaneModel extends Model
     @subscribe @items.onRemoval (item, index) =>
       @unsubscribe item if typeof item.on is 'function'
 
-    @when @$focused, => @makeActive()
-
-    @focus() if params?.focused
     @makeActive() if params?.active
 
   serializeParams: ->
@@ -59,6 +52,13 @@ class PaneModel extends Model
 
   isActive: -> @active
 
+  focus: ->
+    @focused = true
+    @makeActive()
+
+  blur: ->
+    @focused = false
+
   makeActive: -> @container?.activePane = this
 
   getPanes: -> [this]
@@ -119,7 +119,7 @@ class PaneModel extends Model
     item = @items[index]
     @showNextItem() if item is @activeItem and @items.length > 1
     @items.splice(index, 1)
-    @suppressBlur => @emit 'item-removed', item, index, destroying
+    @emit 'item-removed', item, index, destroying
     @destroy() if @items.length is 0
 
   # Public: Moves the given item to a the new index.
@@ -160,11 +160,8 @@ class PaneModel extends Model
 
   # Private: Called by model superclass
   destroyed: ->
+    @container.makeNextPaneActive() if @isActive()
     item.destroy?() for item in @items.slice()
-    if @focused
-      @container.focusNextPane()
-    else if @isActive()
-      @container.makeNextPaneActive()
 
   # Public: Prompt the user to save the given item.
   promptToSaveItem: (item) ->
@@ -245,10 +242,10 @@ class PaneModel extends Model
     if @parent.orientation isnt orientation
       @parent.replaceChild(this, new PaneAxisModel({@container, orientation, children: [this]}))
 
-    newPane = new @constructor(params)
+    newPane = new @constructor(extend({focused: true}, params))
     switch side
       when 'before' then @parent.insertChildBefore(this, newPane)
       when 'after' then @parent.insertChildAfter(this, newPane)
 
-    newPane.focus()
+    newPane.makeActive()
     newPane

src/pane.coffee

@@ -49,9 +49,6 @@ class Pane extends View
     @viewsByItem = new WeakMap()
     @handleEvents()
 
-  hasFocus: ->
-    @is(':focus') or @is(':has(:focus)')
-
   handleEvents: ->
     @subscribe @model, 'destroyed', => @remove()
 
@@ -63,16 +60,10 @@ class Pane extends View
     @subscribe @model, 'item-destroyed', @onItemDestroyed
     @subscribe @model.$active, 'value', @onActiveStatusChanged
 
-    @subscribe @model.$focused, 'value', (focused) =>
-      if focused
-        @focus() unless @hasFocus()
-      else
-        @blur() if @hasFocus()
-
     @subscribe this, 'focusin', => @model.focus()
     @subscribe this, 'focusout', => @model.blur()
     @subscribe this, 'focus', =>
-      @model.suppressBlur => @activeView?.focus()
+      @activeView?.focus()
       false
 
     @command 'pane:save-items', => @saveItems()
@@ -141,8 +132,8 @@ class Pane extends View
     @itemViews.children().not(view).hide()
     @itemViews.append(view) unless view.parent().is(@itemViews)
     view.show() if @attached
-    if isFocused
-      @model.suppressBlur -> view.focus()
+    view.focus() if isFocused
+
     @activeView = view
     @trigger 'pane:active-item-changed', [item]
 
@@ -150,7 +141,25 @@ class Pane extends View
     @trigger 'pane:item-added', [item, index]
 
   onItemRemoved: (item, index, destroyed) =>
-    @cleanupItemView(item, destroyed)
+    if item instanceof $
+      viewToRemove = item
+    else if viewToRemove = @viewsByItem.get(item)
+      @viewsByItem.delete(item)
+
+    removingLastItem = @model.items.length is 0
+    hasFocus = @hasFocus()
+
+    @getContainer().focusNextPane() if hasFocus and removingLastItem
+
+    if viewToRemove?
+      viewToRemove.setModel?(null)
+      if destroyed
+        viewToRemove.remove()
+      else
+        viewToRemove.detach()
+
+    # @focus() if hasFocus and not removingLastItem
+
     @trigger 'pane:item-removed', [item, index]
 
   onItemMoved: (item, newIndex) =>
@@ -167,20 +176,6 @@ class Pane extends View
   activeItemTitleChanged: =>
     @trigger 'pane:active-item-title-changed'
 
-  # Private:
-  cleanupItemView: (item, destroyed) ->
-    if item instanceof $
-      viewToRemove = item
-    else if viewToRemove = @viewsByItem.get(item)
-      @viewsByItem.delete(item)
-
-    if viewToRemove?
-      viewToRemove.setModel?(null)
-      if destroyed
-        viewToRemove.remove()
-      else
-        viewToRemove.detach()
-
   # Private:
   viewForItem: (item) ->
     if item instanceof $
@@ -210,6 +205,7 @@ class Pane extends View
     @closest('.panes').view()
 
   beforeRemove: ->
+    @getContainer()?.focusNextPane() if @hasFocus()
     @model.destroy() unless @model.isDestroyed()
 
   # Private:

src/space-pen-extensions.coffee

@@ -59,6 +59,9 @@ jQuery.fn.destroyTooltip = ->
   @hideTooltip()
   @tooltip('destroy')
 
+jQuery.fn.hasFocus = ->
+  @is(':focus') or @is(':has(:focus)')
+
 jQuery.fn.setTooltip.getKeystroke = getKeystroke
 jQuery.fn.setTooltip.humanizeKeystrokes = humanizeKeystrokes