From 561e31c0c53ff0478778532bf9403242ffbf6c8d Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 12 Jan 2014 17:25:51 -0700 Subject: [PATCH] Remove ::removeItemAtIndex and make ::removeItem private Call ::destroyItem or just destroy the item directly (it should emit the 'destroyed' event) --- spec/pane-container-spec.coffee | 28 ++++++++++++++-------------- spec/pane-model-spec.coffee | 8 ++++---- spec/pane-spec.coffee | 27 +++++++++++++-------------- src/pane-model.coffee | 6 ++---- src/pane.coffee | 10 +++++----- 5 files changed, 38 insertions(+), 41 deletions(-) diff --git a/spec/pane-container-spec.coffee b/spec/pane-container-spec.coffee index 30a89328d..b9cb64d7d 100644 --- a/spec/pane-container-spec.coffee +++ b/spec/pane-container-spec.coffee @@ -183,27 +183,27 @@ describe "PaneContainer", -> expect(activeItemChangedHandler.callCount).toBe 1 expect(activeItemChangedHandler.argsForCall[0][1]).toEqual item1a - it "is triggered when the active pane item is removed", -> + it "is triggered when the active pane item is destroyed", -> pane1.showItem(item1b) activeItemChangedHandler.reset() - pane1.removeItem(item1b) + pane1.destroyItem(item1b) expect(activeItemChangedHandler.callCount).toBe 1 expect(activeItemChangedHandler.argsForCall[0][1]).toEqual item1a - it "is not triggered when an inactive pane item is removed", -> + it "is not triggered when an inactive pane item is destroyed", -> pane1.showItem(item1b) activeItemChangedHandler.reset() - pane1.removeItem(item1a) + pane1.destroyItem(item1a) expect(activeItemChangedHandler).not.toHaveBeenCalled() - it "is triggered when all pane items are removed", -> - pane1.removeItem(item1a) + it "is triggered when all pane items are destroyed", -> + pane1.destroyItem(item1a) expect(activeItemChangedHandler.callCount).toBe 1 expect(activeItemChangedHandler.argsForCall[0][1]).toBe undefined - it "is triggered when the pane is removed", -> + it "is triggered when the pane is destroyed", -> pane1.remove() expect(activeItemChangedHandler.callCount).toBe 1 expect(activeItemChangedHandler.argsForCall[0][1]).toBe undefined @@ -224,27 +224,27 @@ describe "PaneContainer", -> pane1.showItem(item1b) expect(activeItemChangedHandler).not.toHaveBeenCalled() - it "is triggered when the active pane item removed from the active pane", -> + it "is triggered when the active pane's active item is destroyed", -> pane2.showItem(item2b) activeItemChangedHandler.reset() - pane2.removeItem(item2b) + pane2.destroyItem(item2b) expect(activeItemChangedHandler.callCount).toBe 1 expect(activeItemChangedHandler.argsForCall[0][1]).toEqual item2a - it "is not triggered when the active pane item removed from an inactive pane", -> + it "is not triggered when an inactive pane's active item is destroyed", -> pane1.showItem(item1b) activeItemChangedHandler.reset() - pane1.removeItem(item1b) + pane1.destroyItem(item1b) expect(activeItemChangedHandler).not.toHaveBeenCalled() - it "is triggered when the active pane is removed", -> + it "is triggered when the active pane is destroyed", -> pane2.remove() expect(activeItemChangedHandler.callCount).toBe 1 expect(activeItemChangedHandler.argsForCall[0][1]).toEqual item1a - it "is not triggered when an inactive pane is removed", -> + it "is not triggered when an inactive pane is destroyed", -> pane1.remove() expect(activeItemChangedHandler).not.toHaveBeenCalled() @@ -263,7 +263,7 @@ describe "PaneContainer", -> expect(activeItemChangedHandler.callCount).toBe 1 expect(activeItemChangedHandler.argsForCall[0][1]).toEqual item3a - it "is not triggered when the non active pane is removed", -> + it "is not triggered when an inactive pane is destroyed", -> pane3 = pane2.splitDown(item3a) activeItemChangedHandler.reset() diff --git a/spec/pane-model-spec.coffee b/spec/pane-model-spec.coffee index 77f1db022..6eb897b06 100644 --- a/spec/pane-model-spec.coffee +++ b/spec/pane-model-spec.coffee @@ -84,12 +84,12 @@ describe "PaneModel", -> pane2 = pane1.splitRight() expect(pane2.focused).toBe true - describe "::removeItemAtIndex(index)", -> - describe "when the last item is removed", -> + describe "::destroyItem(item)", -> + describe "when the last item is destroyed", -> it "destroys the pane", -> pane = new PaneModel(items: ["A", "B"]) - pane.removeItemAtIndex(0) - pane.removeItemAtIndex(0) + pane.destroyItem("A") + pane.destroyItem("B") expect(pane.isDestroyed()).toBe true describe "when an item emits a destroyed event", -> diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index cf85d9828..c24d10447 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -125,10 +125,10 @@ describe "Pane", -> pane.showPreviousItem() expect(pane.itemViews.find('.test-view').length).toBe initialViewCount + 2 - pane.removeItem(model2) + pane.destroyItem(model2) expect(pane.itemViews.find('.test-view').length).toBe initialViewCount + 1 - pane.removeItem(model1) + pane.destroyItem(model1) expect(pane.itemViews.find('.test-view').length).toBe initialViewCount describe "when showing a view item", -> @@ -201,27 +201,26 @@ describe "Pane", -> pane.destroyItem(view1) expect(view1.wasRemoved).toBe true - describe "::removeItem(item)", -> it "removes the item from the items list and shows the next item if it was showing", -> - pane.removeItem(view1) + pane.destroyItem(view1) expect(pane.getItems()).toEqual [editor1, view2, editor2] expect(pane.activeItem).toBe editor1 pane.showItem(editor2) - pane.removeItem(editor2) + pane.destroyItem(editor2) expect(pane.getItems()).toEqual [editor1, view2] expect(pane.activeItem).toBe editor1 it "triggers 'pane:item-removed' with the item and its former index", -> itemRemovedHandler = jasmine.createSpy("itemRemovedHandler") pane.on 'pane:item-removed', itemRemovedHandler - pane.removeItem(editor1) + pane.destroyItem(editor1) expect(itemRemovedHandler).toHaveBeenCalled() expect(itemRemovedHandler.argsForCall[0][1..2]).toEqual [editor1, 1] describe "when removing the last item", -> it "removes the pane", -> - pane.removeItem(item) for item in pane.getItems() + pane.destroyItem(item) for item in pane.getItems() expect(pane.hasParent()).toBeFalsy() describe "when the pane is focused", -> @@ -231,22 +230,22 @@ describe "Pane", -> pane2 = pane.splitRight(new TestView(id: 'view-3', text: 'View 3')) pane.focus() expect(pane).toMatchSelector(':has(:focus)') - pane.removeItem(item) for item in pane.getItems() + pane.destroyItem(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", -> expect(view1.parent()).toMatchSelector pane.itemViews - pane.removeItem(view1) + pane.destroyItem(view1) expect(view1.parent()).not.toMatchSelector pane.itemViews describe "when the item is a model", -> it "removes the associated view only when all items that require it have been removed", -> pane.showItem(editor1) pane.showItem(editor2) - pane.removeItem(editor2) + pane.destroyItem(editor2) expect(pane.itemViews.find('.editor')).toExist() - pane.removeItem(editor1) + pane.destroyItem(editor1) expect(pane.itemViews.find('.editor')).not.toExist() describe "::moveItem(item, index)", -> @@ -286,9 +285,9 @@ describe "Pane", -> describe "when it is the last item on the source pane", -> it "removes the source pane, but does not destroy the item", -> - pane.removeItem(view1) - pane.removeItem(view2) - pane.removeItem(editor2) + pane.destroyItem(view1) + pane.destroyItem(view2) + pane.destroyItem(editor2) expect(pane.getItems()).toEqual [editor1] pane.moveItemToPane(editor1, pane2, 1) diff --git a/src/pane-model.coffee b/src/pane-model.coffee index 8ccef977e..15db07345 100644 --- a/src/pane-model.coffee +++ b/src/pane-model.coffee @@ -135,12 +135,10 @@ class PaneModel extends Model @emit 'item-added', item, index item + # Private: removeItem: (item, destroying) -> index = @items.indexOf(item) - @removeItemAtIndex(index, destroying) if index >= 0 - - removeItemAtIndex: (index, destroying) -> - item = @items[index] + return if index is -1 @showNextItem() if item is @activeItem and @items.length > 1 @items.splice(index, 1) @emit 'item-removed', item, index, destroying diff --git a/src/pane.coffee b/src/pane.coffee index 37c15add7..b52329127 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -26,11 +26,11 @@ class Pane extends View @delegatesProperties 'items', 'activeItem', toProperty: 'model' @delegatesMethods 'getItems', 'showNextItem', 'showPreviousItem', 'getActiveItemIndex', - 'showItemAtIndex', 'showItem', 'addItem', 'itemAtIndex', 'removeItem', 'removeItemAtIndex', - 'moveItem', 'moveItemToPane', 'destroyItem', 'destroyItems', 'destroyActiveItem', - 'destroyInactiveItems', 'saveActiveItem', 'saveActiveItemAs', 'saveItem', 'saveItemAs', - 'saveItems', 'itemForUri', 'showItemForUri', 'promptToSaveItem', 'copyActiveItem', - 'isActive', 'activate', toProperty: 'model' + 'showItemAtIndex', 'showItem', 'addItem', 'itemAtIndex', 'moveItem', 'moveItemToPane', + 'destroyItem', 'destroyItems', 'destroyActiveItem', 'destroyInactiveItems', + 'saveActiveItem', 'saveActiveItemAs', 'saveItem', 'saveItemAs', 'saveItems', + 'itemForUri', 'showItemForUri', 'promptToSaveItem', 'copyActiveItem', 'isActive', + 'activate', toProperty: 'model' previousActiveItem: null