From ddb08d0c46ccf6408c5e7618fedd6cfc93bbad50 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 5 Oct 2015 12:34:49 -0600 Subject: [PATCH] Avoid redundant onDidAddPaneItem notifications Refs #9012 --- spec/pane-spec.coffee | 16 +++++++++++----- src/pane-container.coffee | 8 ++++---- src/pane.coffee | 24 ++++++++++++------------ 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index 851ffe71d..d293a6d2c 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -103,7 +103,7 @@ describe "Pane", -> item = new Item("C") pane.addItem(item, 1) - expect(events).toEqual [{item, index: 1}] + expect(events).toEqual [{item, index: 1, moved: false}] it "throws an exception if the item is already present on a pane", -> item = new Item("A") @@ -223,13 +223,13 @@ describe "Pane", -> events = [] pane.onWillRemoveItem (event) -> events.push(event) pane.destroyItem(item2) - expect(events).toEqual [{item: item2, index: 1, destroyed: true}] + expect(events).toEqual [{item: item2, index: 1, moved: false, destroyed: true}] it "invokes ::onDidRemoveItem() observers", -> events = [] pane.onDidRemoveItem (event) -> events.push(event) pane.destroyItem(item2) - expect(events).toEqual [{item: item2, index: 1, destroyed: true}] + expect(events).toEqual [{item: item2, index: 1, moved: false, destroyed: true}] describe "when the destroyed item is the active item and is the first item", -> it "activates the next item", -> @@ -517,14 +517,20 @@ describe "Pane", -> pane1.onWillRemoveItem (event) -> events.push(event) pane1.moveItemToPane(item2, pane2, 1) - expect(events).toEqual [{item: item2, index: 1, destroyed: false}] + expect(events).toEqual [{item: item2, index: 1, moved: true, destroyed: false}] it "invokes ::onDidRemoveItem() observers", -> events = [] pane1.onDidRemoveItem (event) -> events.push(event) pane1.moveItemToPane(item2, pane2, 1) - expect(events).toEqual [{item: item2, index: 1, destroyed: false}] + expect(events).toEqual [{item: item2, index: 1, moved: true, destroyed: false}] + + it "does not invoke ::onDidAddPaneItem observers on the container", -> + addedItems = [] + container.onDidAddPaneItem (item) -> addedItems.push(item) + pane1.moveItemToPane(item2, pane2, 1) + expect(addedItems).toEqual [] describe "when the moved item the last item in the source pane", -> beforeEach -> diff --git a/src/pane-container.coffee b/src/pane-container.coffee index 9fbf14519..786c4b84f 100644 --- a/src/pane-container.coffee +++ b/src/pane-container.coffee @@ -210,11 +210,11 @@ class PaneContainer extends Model for item, index in pane.getItems() @addedPaneItem(item, pane, index) - pane.onDidAddItem ({item, index}) => - @addedPaneItem(item, pane, index) + pane.onDidAddItem ({item, index, moved}) => + @addedPaneItem(item, pane, index) unless moved - pane.onDidRemoveItem ({item}) => - @removedPaneItem(item) + pane.onDidRemoveItem ({item, moved}) => + @removedPaneItem(item) unless moved addedPaneItem: (item, pane, index) -> @itemRegistry.addItem(item) diff --git a/src/pane.coffee b/src/pane.coffee index 6cf505b54..4085edff2 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -339,7 +339,7 @@ class Pane extends Model # the pane's view. activateItem: (item) -> if item? - @addItem(item) + @addItem(item, @getActiveItemIndex() + 1, false) @setActiveItem(item) # Public: Add the given item to the pane. @@ -350,17 +350,17 @@ class Pane extends Model # If omitted, the item is added after the current active item. # # Returns the added item. - addItem: (item, index=@getActiveItemIndex() + 1) -> + addItem: (item, index=@getActiveItemIndex() + 1, moved=false) -> throw new Error("Pane items must be objects. Attempted to add item #{item}.") unless item? and typeof item is 'object' throw new Error("Adding a pane item with URI '#{item.getURI?()}' that has already been destroyed") if item.isDestroyed?() return if item in @items if typeof item.onDidDestroy is 'function' - @itemSubscriptions.set item, item.onDidDestroy => @removeItem(item, true) + @itemSubscriptions.set item, item.onDidDestroy => @removeItem(item, false) @items.splice(index, 0, item) - @emitter.emit 'did-add-item', {item, index} + @emitter.emit 'did-add-item', {item, index, moved} @setActiveItem(item) unless @getActiveItem()? item @@ -375,14 +375,14 @@ class Pane extends Model # Returns an {Array} of added items. addItems: (items, index=@getActiveItemIndex() + 1) -> items = items.filter (item) => not (item in @items) - @addItem(item, index + i) for item, i in items + @addItem(item, index + i, false) for item, i in items items - removeItem: (item, destroyed=false) -> + removeItem: (item, moved) -> index = @items.indexOf(item) return if index is -1 - @emitter.emit 'will-remove-item', {item, index, destroyed} + @emitter.emit 'will-remove-item', {item, index, destroyed: !moved, moved} @unsubscribeFromItem(item) if item is @activeItem @@ -393,8 +393,8 @@ class Pane extends Model else @activatePreviousItem() @items.splice(index, 1) - @emitter.emit 'did-remove-item', {item, index, destroyed} - @container?.didDestroyPaneItem({item, index, pane: this}) if destroyed + @emitter.emit 'did-remove-item', {item, index, destroyed: !moved, moved} + @container?.didDestroyPaneItem({item, index, pane: this}) unless moved @destroy() if @items.length is 0 and atom.config.get('core.destroyEmptyPanes') # Public: Move the given item to the given index. @@ -414,8 +414,8 @@ class Pane extends Model # * `index` {Number} indicating the index to which to move the item in the # given pane. moveItemToPane: (item, pane, index) -> - @removeItem(item) - pane.addItem(item, index) + @removeItem(item, true) + pane.addItem(item, index, true) # Public: Destroy the active item and activate the next item. destroyActiveItem: -> @@ -435,7 +435,7 @@ class Pane extends Model @emitter.emit 'will-destroy-item', {item, index} @container?.willDestroyPaneItem({item, index, pane: this}) if @promptToSaveItem(item) - @removeItem(item, true) + @removeItem(item, false) item.destroy?() true else