From ae95c04bbce05ce703dabfc71c6ab3e8704ded96 Mon Sep 17 00:00:00 2001
From: Corey Johnson & Nathan Sobo <cj+nathan@github.com>
Date: Wed, 27 Feb 2013 18:21:24 -0700
Subject: [PATCH] Focus next pane when removing the last pane item of a focused
 pane

Previously, removing the last pane item also ruined our ability to
determine if the pane had focus. Now, if we're removing the last item,
we instead just go ahead and remove the entire pane. Remove contains
logic to switch focus to the next pane if its active view is focused,
which works as intended if we leave the active view in place.
---
 spec/app/pane-spec.coffee | 16 +++++++++++++---
 src/app/pane.coffee       | 23 +++++++++++++++--------
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/spec/app/pane-spec.coffee b/spec/app/pane-spec.coffee
index 78ae84e22..987258ae7 100644
--- a/spec/app/pane-spec.coffee
+++ b/spec/app/pane-spec.coffee
@@ -121,9 +121,19 @@ describe "Pane", ->
       expect(itemRemovedHandler).toHaveBeenCalled()
       expect(itemRemovedHandler.argsForCall[0][1..2]).toEqual [editSession1, 1]
 
-    it "removes the pane when its last item is removed", ->
-      pane.removeItem(item) for item in pane.getItems()
-      expect(pane.hasParent()).toBeFalsy()
+    describe "when removing the last item", ->
+      it "removes the pane", ->
+        pane.removeItem(item) for item in pane.getItems()
+        expect(pane.hasParent()).toBeFalsy()
+
+      describe "when the pane is focused", ->
+        it "shifts focus to the next pane", ->
+          container.attachToDom()
+          pane2 = pane.splitRight($$ -> @div class: 'view-3', tabindex: -1, 'View 3')
+          pane.focus()
+          expect(pane).toMatchSelector(':has(:focus)')
+          pane.removeItem(item) for item in pane.getItems()
+          expect(pane2).toMatchSelector ':has(:focus)'
 
     describe "when the item is a view", ->
       it "removes the item from the 'item-views' div", ->
diff --git a/src/app/pane.coffee b/src/app/pane.coffee
index 0f5e9d96b..0c4886f76 100644
--- a/src/app/pane.coffee
+++ b/src/app/pane.coffee
@@ -105,10 +105,8 @@ class Pane extends View
 
     @showNextItem() if item is @activeItem and @items.length > 1
     _.remove(@items, item)
-
     @cleanupItemView(item)
     @trigger 'pane:item-removed', [item, index]
-    @remove() unless @items.length
 
   moveItem: (item, newIndex) ->
     oldIndex = @items.indexOf(item)
@@ -125,16 +123,20 @@ class Pane extends View
 
   cleanupItemView: (item) ->
     if item instanceof $
-      item.remove()
+      viewToRemove = item
     else
       viewClass = item.getViewClass()
       otherItemsForView = @items.filter (i) -> i.getViewClass?() is viewClass
       unless otherItemsForView.length
-        view = @viewsByClassName[viewClass.name]
-        view?.setModel(null)
-        view?.remove()
+        viewToRemove = @viewsByClassName[viewClass.name]
+        viewToRemove?.setModel(null)
         delete @viewsByClassName[viewClass.name]
 
+    if @items.length > 0
+      viewToRemove?.remove()
+    else
+      @remove()
+
   viewForItem: (item) ->
     if item instanceof $
       item
@@ -197,19 +199,24 @@ class Pane extends View
 
   remove: (selector, keepData) ->
     return super if keepData
+
     # find parent elements before removing from dom
     container = @getContainer()
     parentAxis = @parent('.row, .column')
+
     if @is(':has(:focus)')
-      rootView?.focus() unless container.focusNextPane()
+      container.focusNextPane() or rootView?.focus()
     else if @isActive()
       container.makeNextPaneActive()
 
     super
 
     if parentAxis.children().length == 1
-      sibling = parentAxis.children().detach()
+      sibling = parentAxis.children()
+      siblingFocused = sibling.is(':has(:focus)')
+      sibling.detach()
       parentAxis.replaceWith(sibling)
+      sibling.focus() if siblingFocused
     container.adjustPaneDimensions()
     container.trigger 'pane:removed', [this]