From 27b22acd3789f716ccede8bf254028fe6350bfe3 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 2 Dec 2016 09:52:28 +1100 Subject: [PATCH] Fixing bug that caused references to stale DOM objects to be kept and used by Panels service Was causing panels to break when used in conjuction with ng-repeat + filters --- .../index_utils/services/panels.js.coffee | 12 ++++--- .../services/panels_spec.js.coffee | 35 +++++++++---------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/admin/index_utils/services/panels.js.coffee b/app/assets/javascripts/admin/index_utils/services/panels.js.coffee index d020b88264..ca6cd22710 100644 --- a/app/assets/javascripts/admin/index_utils/services/panels.js.coffee +++ b/app/assets/javascripts/admin/index_utils/services/panels.js.coffee @@ -1,14 +1,16 @@ angular.module("admin.indexUtils").factory 'Panels', -> new class Panels - panels: [] + all: [] register: (ctrl, object, selected=null) -> if ctrl? && object? - @panels.push { ctrl: ctrl, object: object, selected: selected } + existing = @panelFor(object) + newPanel = { ctrl: ctrl, object: object, selected: selected } + if existing then angular.extend(existing, newPanel) else @all.push(newPanel) ctrl.select(selected) if selected? toggle: (object, name, state=null) -> - panel = @findPanelByObject(object) + panel = @panelFor(object) if panel.selected == name @select(panel, null) unless state == "open" else @@ -18,5 +20,5 @@ angular.module("admin.indexUtils").factory 'Panels', -> panel.selected = name panel.ctrl.select(name) - findPanelByObject: (object) -> - (panel for panel in @panels when panel.object == object)[0] + panelFor: (object) -> + (@all.filter (panel) -> panel.object == object)[0] diff --git a/spec/javascripts/unit/admin/index_utils/services/panels_spec.js.coffee b/spec/javascripts/unit/admin/index_utils/services/panels_spec.js.coffee index 8a659df984..613784c30a 100644 --- a/spec/javascripts/unit/admin/index_utils/services/panels_spec.js.coffee +++ b/spec/javascripts/unit/admin/index_utils/services/panels_spec.js.coffee @@ -13,13 +13,11 @@ describe "Panels service", -> ctrl1 = jasmine.createSpyObj('ctrl', ['select']) ctrl2 = jasmine.createSpyObj('ctrl', ['select']) - it "adds the panels controller, object and selection to @panels", -> + it "adds the panels controller, object and selection to @byObjectID", -> Panels.register(ctrl1, { name: "obj1"}, "panel1") Panels.register(ctrl2, { name: "obj2"}) - expect(Panels.panels).toEqual [ - { ctrl: ctrl1, object: { name: "obj1"}, selected: "panel1" }, - { ctrl: ctrl2, object: { name: "obj2"}, selected: null } - ] + expect(Panels.all[0]).toEqual { ctrl: ctrl1, object: { name: "obj1"}, selected: "panel1" } + expect(Panels.all[1]).toEqual { ctrl: ctrl2, object: { name: "obj2"}, selected: null } it "call select on the controller if a selection is provided", -> Panels.register(ctrl1, { name: "obj1"}, "panel1") @@ -27,39 +25,40 @@ describe "Panels service", -> expect(ctrl1.select.calls.count()).toEqual 1 expect(ctrl2.select.calls.count()).toEqual 0 - it "ignores the input if object or ctrl are null", -> + it "ignores the input if ctrl, object are null", -> Panels.register(ctrl1, null) Panels.register(null, { name: "obj2"}) - expect(Panels.panels).toEqual [] + expect(Panels.all.length).toEqual 0 describe "toggling a panel", -> panelMock = ctrlMock = objMock = null beforeEach -> + objMock = {some: "object"} ctrlMock = jasmine.createSpyObj('ctrl', ['select']) - panelMock = { ctrl: ctrlMock } - spyOn(Panels, "findPanelByObject").and.returnValue(panelMock) + panelMock = { ctrl: ctrlMock, object: objMock } + Panels.all = [panelMock] describe "when no panel is currently selected", -> beforeEach -> panelMock.selected = null describe "when no state is provided", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name') + beforeEach -> Panels.toggle(objMock, 'panel_name') it "selects the named panel", -> expect(panelMock.selected).toEqual 'panel_name' expect(ctrlMock.select).toHaveBeenCalledWith('panel_name') describe "when the state given is 'open'", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name', "open") + beforeEach -> Panels.toggle(objMock, 'panel_name', "open") it "selects the named panel", -> expect(panelMock.selected).toEqual 'panel_name' expect(ctrlMock.select).toHaveBeenCalledWith('panel_name') describe "when the state given is 'closed'", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name', "closed") + beforeEach -> Panels.toggle(objMock, 'panel_name', "closed") it "does not select the named panel", -> expect(panelMock.selected).toEqual null @@ -70,21 +69,21 @@ describe "Panels service", -> panelMock.selected = 'panel_name' describe "when no state is provided", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name') + beforeEach -> Panels.toggle(objMock, 'panel_name') it "de-selects the named panel", -> expect(panelMock.selected).toEqual null expect(ctrlMock.select).toHaveBeenCalledWith(null) describe "when the state given is 'open'", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name', "open") + beforeEach -> Panels.toggle(objMock, 'panel_name', "open") it "keeps the the named panel selected, but does not call select on the controller", -> expect(panelMock.selected).toEqual 'panel_name' expect(ctrlMock.select).not.toHaveBeenCalledWith('panel_name') describe "when the state given is 'closed'", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name', "closed") + beforeEach -> Panels.toggle(objMock, 'panel_name', "closed") it "de-selects the named panel", -> expect(panelMock.selected).toEqual null @@ -95,21 +94,21 @@ describe "Panels service", -> panelMock.selected = 'some_other_panel' describe "when no state is provided", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name') + beforeEach -> Panels.toggle(objMock, 'panel_name') it "selects the named panel", -> expect(panelMock.selected).toEqual 'panel_name' expect(ctrlMock.select).toHaveBeenCalledWith('panel_name') describe "when the state given is 'open'", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name', "open") + beforeEach -> Panels.toggle(objMock, 'panel_name', "open") it "selects the named panel", -> expect(panelMock.selected).toEqual 'panel_name' expect(ctrlMock.select).toHaveBeenCalledWith('panel_name') describe "when the state given is 'closed'", -> - beforeEach -> Panels.toggle({some: "object"}, 'panel_name', "closed") + beforeEach -> Panels.toggle(objMock, 'panel_name', "closed") it "keeps the currently selected panel selected, ie. does nothing", -> expect(panelMock.selected).toEqual "some_other_panel"