From 5ef821c92a0037b8b0cf4fd9546fe05e8d5f8cf8 Mon Sep 17 00:00:00 2001 From: Dan Wilson Date: Tue, 14 May 2013 12:43:22 +0100 Subject: [PATCH 1/3] Use listenTo() rather than bind() in the views so that the events get unbound upon removal. --- src/view.flot.js | 13 ++++------ src/view.grid.js | 6 ++--- src/view.map.js | 18 ++++++------- src/view.multiview.js | 52 +++++++++++++++++++------------------- src/view.slickgrid.js | 8 +++--- src/view.timeline.js | 4 +-- src/widget.facetviewer.js | 4 +-- src/widget.fields.js | 2 +- src/widget.filtereditor.js | 5 ++-- src/widget.pager.js | 2 +- src/widget.queryeditor.js | 2 +- src/widget.valuefilter.js | 5 ++-- 12 files changed, 56 insertions(+), 65 deletions(-) diff --git a/src/view.flot.js b/src/view.flot.js index 0bda4d18..7ec07b51 100644 --- a/src/view.flot.js +++ b/src/view.flot.js @@ -40,11 +40,9 @@ my.Flot = Backbone.View.extend({ _.bindAll(this, 'render', 'redraw', '_toolTip', '_xaxisLabel'); this.needToRedraw = false; - this.model.bind('change', this.render); - this.model.fields.bind('reset', this.render); - this.model.fields.bind('add', this.render); - this.model.records.bind('add', this.redraw); - this.model.records.bind('reset', this.redraw); + this.listenTo(this.model, 'change', this.render); + this.listenTo(this.model.fields, 'reset add', this.render); + this.listenTo(this.model.records, 'reset add', this.redraw); var stateData = _.extend({ group: null, // so that at least one series chooser box shows up @@ -59,7 +57,7 @@ my.Flot = Backbone.View.extend({ model: this.model, state: this.state.toJSON() }); - this.editor.state.bind('change', function() { + this.listenTo(this.editor.state, 'change', function() { self.state.set(self.editor.state.toJSON()); self.redraw(); }); @@ -403,8 +401,7 @@ my.FlotControls = Backbone.View.extend({ initialize: function(options) { var self = this; _.bindAll(this, 'render'); - this.model.fields.bind('reset', this.render); - this.model.fields.bind('add', this.render); + this.listenTo(this.model.fields, 'reset add', this.render); this.state = new recline.Model.ObjectState(options.state); this.render(); }, diff --git a/src/view.grid.js b/src/view.grid.js index 394b205d..6ab83454 100644 --- a/src/view.grid.js +++ b/src/view.grid.js @@ -17,9 +17,7 @@ my.Grid = Backbone.View.extend({ initialize: function(modelEtc) { var self = this; _.bindAll(this, 'render', 'onHorizontalScroll'); - this.model.records.bind('add', this.render); - this.model.records.bind('reset', this.render); - this.model.records.bind('remove', this.render); + this.listenTo(this.model.records, 'add reset remove', this.render); this.tempState = {}; var state = _.extend({ hiddenFields: [] @@ -168,7 +166,7 @@ my.GridRow = Backbone.View.extend({ initialize: function(initData) { _.bindAll(this, 'render'); this._fields = initData.fields; - this.model.bind('change', this.render); + this.listenTo(this.model, 'change', this.render); }, template: ' \ diff --git a/src/view.map.js b/src/view.map.js index c0979a2a..8015256b 100644 --- a/src/view.map.js +++ b/src/view.map.js @@ -77,29 +77,29 @@ my.Map = Backbone.View.extend({ }; // Listen to changes in the fields - this.model.fields.bind('change', function() { + this.listenTo(this.model.fields, 'change', function() { self._setupGeometryField(); self.render(); }); // Listen to changes in the records - this.model.records.bind('add', function(doc){self.redraw('add',doc);}); - this.model.records.bind('change', function(doc){ + this.listenTo(this.model.records, 'add', function(doc){self.redraw('add',doc);}); + this.listenTo(this.model.records, 'change', function(doc){ self.redraw('remove',doc); self.redraw('add',doc); }); - this.model.records.bind('remove', function(doc){self.redraw('remove',doc);}); - this.model.records.bind('reset', function(){self.redraw('reset');}); + this.listenTo(this.model.records, 'remove', function(doc){self.redraw('remove',doc);}); + this.listenTo(this.model.records, 'reset', function(){self.redraw('reset');}); this.menu = new my.MapMenu({ model: this.model, state: this.state.toJSON() }); - this.menu.state.bind('change', function() { + this.listenTo(this.menu.state, 'change', function() { self.state.set(self.menu.state.toJSON()); self.redraw(); }); - this.state.bind('change', function() { + this.listenTo(this.state, 'change', function() { self.redraw(); }); this.elSidebar = this.menu.$el; @@ -546,9 +546,9 @@ my.MapMenu = Backbone.View.extend({ initialize: function(options) { var self = this; _.bindAll(this, 'render'); - this.model.fields.bind('change', this.render); + this.listenTo(this.model.fields, 'change', this.render); this.state = new recline.Model.ObjectState(options.state); - this.state.bind('change', this.render); + this.listenTo(this.state, 'change', this.render); this.render(); }, diff --git a/src/view.multiview.js b/src/view.multiview.js index 691634ff..bce2b106 100644 --- a/src/view.multiview.js +++ b/src/view.multiview.js @@ -199,30 +199,30 @@ my.MultiView = Backbone.View.extend({ } this._showHideSidebar(); - this.model.bind('query:start', function() { - self.notify({loader: true, persist: true}); - }); - this.model.bind('query:done', function() { - self.clearNotifications(); - self.$el.find('.doc-count').text(self.model.recordCount || 'Unknown'); - }); - this.model.bind('query:fail', function(error) { - self.clearNotifications(); - var msg = ''; - if (typeof(error) == 'string') { - msg = error; - } else if (typeof(error) == 'object') { - if (error.title) { - msg = error.title + ': '; - } - if (error.message) { - msg += error.message; - } - } else { - msg = 'There was an error querying the backend'; + this.listenTo(this.model, 'query:start', function() { + self.notify({loader: true, persist: true}); + }); + this.listenTo(this.model, 'query:done', function() { + self.clearNotifications(); + self.$el.find('.doc-count').text(self.model.recordCount || 'Unknown'); + }); + this.listenTo(this.model, 'query:fail', function(error) { + self.clearNotifications(); + var msg = ''; + if (typeof(error) == 'string') { + msg = error; + } else if (typeof(error) == 'object') { + if (error.title) { + msg = error.title + ': '; } - self.notify({message: msg, category: 'error', persist: true}); - }); + if (error.message) { + msg += error.message; + } + } else { + msg = 'There was an error querying the backend'; + } + self.notify({message: msg, category: 'error', persist: true}); + }); // retrieve basic data like fields etc // note this.model and dataset returned are the same @@ -371,7 +371,7 @@ my.MultiView = Backbone.View.extend({ _bindStateChanges: function() { var self = this; // finally ensure we update our state object when state of sub-object changes so that state is always up to date - this.model.queryState.bind('change', function() { + this.listenTo(this.model.queryState, 'change', function() { self.state.set({query: self.model.queryState.toJSON()}); }); _.each(this.pageViews, function(pageView) { @@ -379,7 +379,7 @@ my.MultiView = Backbone.View.extend({ var update = {}; update['view-' + pageView.id] = pageView.view.state.toJSON(); self.state.set(update); - pageView.view.state.bind('change', function() { + self.listenTo(pageView.view.state, 'change', function() { var update = {}; update['view-' + pageView.id] = pageView.view.state.toJSON(); // had problems where change not being triggered for e.g. grid view so let's do it explicitly @@ -393,7 +393,7 @@ my.MultiView = Backbone.View.extend({ _bindFlashNotifications: function() { var self = this; _.each(this.pageViews, function(pageView) { - pageView.view.bind('recline:flash', function(flash) { + self.listenTo(pageView.view, 'recline:flash', function(flash) { self.notify(flash); }); }); diff --git a/src/view.slickgrid.js b/src/view.slickgrid.js index d1f1f206..f7250fc9 100644 --- a/src/view.slickgrid.js +++ b/src/view.slickgrid.js @@ -35,11 +35,9 @@ my.SlickGrid = Backbone.View.extend({ initialize: function(modelEtc) { var self = this; this.$el.addClass('recline-slickgrid'); - _.bindAll(this, 'render'); - this.model.records.bind('add', this.render); - this.model.records.bind('reset', this.render); - this.model.records.bind('remove', this.render); - this.model.records.bind('change', this.onRecordChanged, this); + _.bindAll(this, 'render', 'onRecordChanged'); + this.listenTo(this.model.records, 'add remove reset', this.render); + this.listenTo(this.model.records, 'change', this.onRecordChanged); var state = _.extend({ hiddenColumns: [], diff --git a/src/view.timeline.js b/src/view.timeline.js index 40aa7563..3a35c595 100644 --- a/src/view.timeline.js +++ b/src/view.timeline.js @@ -30,10 +30,10 @@ my.Timeline = Backbone.View.extend({ var self = this; this.timeline = new VMM.Timeline(); this._timelineIsInitialized = false; - this.model.fields.bind('reset', function() { + this.listenTo(this.model.fields, 'reset', function() { self._setupTemporalField(); }); - this.model.records.bind('all', function() { + this.listenTo(this.model.records, 'all', function() { self.reloadData(); }); var stateData = _.extend({ diff --git a/src/widget.facetviewer.js b/src/widget.facetviewer.js index 45f9a40e..4e140af3 100644 --- a/src/widget.facetviewer.js +++ b/src/widget.facetviewer.js @@ -42,8 +42,8 @@ my.FacetViewer = Backbone.View.extend({ }, initialize: function(model) { _.bindAll(this, 'render'); - this.model.facets.bind('all', this.render); - this.model.fields.bind('all', this.render); + this.listenTo(this.model.facets, 'all', this.render); + this.listenTo(this.model.fields, 'all', this.render); this.render(); }, render: function() { diff --git a/src/widget.fields.js b/src/widget.fields.js index bb950286..aba67557 100644 --- a/src/widget.fields.js +++ b/src/widget.fields.js @@ -65,7 +65,7 @@ my.Fields = Backbone.View.extend({ // TODO: this is quite restrictive in terms of when it is re-run // e.g. a change in type will not trigger a re-run atm. // being more liberal (e.g. binding to all) can lead to being called a lot (e.g. for change:width) - this.model.fields.bind('reset', function(action) { + this.listenTo(this.model.fields, 'reset', function(action) { self.model.fields.each(function(field) { field.facets.unbind('all', self.render); field.facets.bind('all', self.render); diff --git a/src/widget.filtereditor.js b/src/widget.filtereditor.js index d7ceefbf..ff92af87 100644 --- a/src/widget.filtereditor.js +++ b/src/widget.filtereditor.js @@ -90,9 +90,8 @@ my.FilterEditor = Backbone.View.extend({ }, initialize: function() { _.bindAll(this, 'render'); - this.model.fields.bind('all', this.render); - this.model.queryState.bind('change', this.render); - this.model.queryState.bind('change:filters:new-blank', this.render); + this.listenTo(this.model.fields, 'all', this.render); + this.listenTo(this.model.queryState, 'change change:filters:new-blank', this.render); this.render(); }, render: function() { diff --git a/src/widget.pager.js b/src/widget.pager.js index 59b308db..b2b5d03d 100644 --- a/src/widget.pager.js +++ b/src/widget.pager.js @@ -25,7 +25,7 @@ my.Pager = Backbone.View.extend({ initialize: function() { _.bindAll(this, 'render'); - this.model.bind('change', this.render); + this.listenTo(this.model, 'change', this.render); this.render(); }, onFormSubmit: function(e) { diff --git a/src/widget.queryeditor.js b/src/widget.queryeditor.js index 1229053b..87defe0d 100644 --- a/src/widget.queryeditor.js +++ b/src/widget.queryeditor.js @@ -24,7 +24,7 @@ my.QueryEditor = Backbone.View.extend({ initialize: function() { _.bindAll(this, 'render'); - this.model.bind('change', this.render); + this.listenTo(this.model, 'change', this.render); this.render(); }, onFormSubmit: function(e) { diff --git a/src/widget.valuefilter.js b/src/widget.valuefilter.js index b176ff40..29de9bab 100644 --- a/src/widget.valuefilter.js +++ b/src/widget.valuefilter.js @@ -52,9 +52,8 @@ my.ValueFilter = Backbone.View.extend({ }, initialize: function() { _.bindAll(this, 'render'); - this.model.fields.bind('all', this.render); - this.model.queryState.bind('change', this.render); - this.model.queryState.bind('change:filters:new-blank', this.render); + this.listenTo(this.model.fields, 'all', this.render); + this.listenTo(this.model.queryState, 'change change:filters:new-blank', this.render); this.render(); }, render: function() { From 93637a7b0b0101fb5ab25b168347bfa1465d5f3e Mon Sep 17 00:00:00 2001 From: Dan Wilson Date: Tue, 14 May 2013 12:45:48 +0100 Subject: [PATCH 2/3] Where necessary, override remove() to also remove subviews. --- src/view.flot.js | 5 +++++ src/view.multiview.js | 20 ++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/view.flot.js b/src/view.flot.js index 7ec07b51..ff1862d9 100644 --- a/src/view.flot.js +++ b/src/view.flot.js @@ -74,6 +74,11 @@ my.Flot = Backbone.View.extend({ return this; }, + remove: function () { + this.editor.remove(); + Backbone.View.prototype.remove.apply(this, arguments); + }, + redraw: function() { // There are issues generating a Flot graph if either: // * The relevant div that graph attaches to his hidden at the moment of creating the plot -- Flot will complain with diff --git a/src/view.multiview.js b/src/view.multiview.js index bce2b106..46f027b1 100644 --- a/src/view.multiview.js +++ b/src/view.multiview.js @@ -263,18 +263,30 @@ my.MultiView = Backbone.View.extend({ $dataSidebar.append(view.view.el); }, this); - var pager = new recline.View.Pager({ + this.pager = new recline.View.Pager({ model: this.model.queryState }); - this.$el.find('.recline-results-info').after(pager.el); + this.$el.find('.recline-results-info').after(this.pager.el); - var queryEditor = new recline.View.QueryEditor({ + this.queryEditor = new recline.View.QueryEditor({ model: this.model.queryState }); - this.$el.find('.query-editor-here').append(queryEditor.el); + this.$el.find('.query-editor-here').append(this.queryEditor.el); }, + remove: function () { + _.each(this.pageViews, function (view) { + view.view.remove(); + }); + _.each(this.sidebarViews, function (view) { + view.view.remove(); + }); + this.pager.remove(); + this.queryEditor.remove(); + Backbone.View.prototype.remove.apply(this, arguments); + }, + // hide the sidebar if empty _showHideSidebar: function() { var $dataSidebar = this.$el.find('.data-view-sidebar'); From ea8c8f1a5bd6c5f1ad7da6add99c354e29627951 Mon Sep 17 00:00:00 2001 From: Dan Wilson Date: Tue, 14 May 2013 13:11:21 +0100 Subject: [PATCH 3/3] Clean up slickgrid events on removal --- src/view.slickgrid.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/view.slickgrid.js b/src/view.slickgrid.js index f7250fc9..d1860d34 100644 --- a/src/view.slickgrid.js +++ b/src/view.slickgrid.js @@ -51,6 +51,8 @@ my.SlickGrid = Backbone.View.extend({ ); this.state = new recline.Model.ObjectState(state); + + this._slickHandler = new Slick.EventHandler(); }, events: { @@ -185,7 +187,7 @@ my.SlickGrid = Backbone.View.extend({ this.grid.setSortColumn(column, sortAsc); } - this.grid.onSort.subscribe(function(e, args){ + this._slickHandler.subscribe(this.grid.onSort, function(e, args){ var order = (args.sortAsc) ? 'asc':'desc'; var sort = [{ field: args.sortCol.field, @@ -194,7 +196,7 @@ my.SlickGrid = Backbone.View.extend({ self.model.query({sort: sort}); }); - this.grid.onColumnsReordered.subscribe(function(e, args){ + this._slickHandler.subscribe(this.grid.onColumnsReordered, function(e, args){ self.state.set({columnsOrder: _.pluck(self.grid.getColumns(),'id')}); }); @@ -210,7 +212,7 @@ my.SlickGrid = Backbone.View.extend({ self.state.set({columnsWidth:columnsWidth}); }); - this.grid.onCellChange.subscribe(function (e, args) { + this._slickHandler.subscribe(this.grid.onCellChange, function (e, args) { // We need to change the model associated value // var grid = args.grid; @@ -233,7 +235,12 @@ my.SlickGrid = Backbone.View.extend({ } return this; - }, + }, + + remove: function () { + this._slickHandler.unsubscribeAll(); + Backbone.View.prototype.remove.apply(this, arguments); + }, show: function() { // If the div is hidden, SlickGrid will calculate wrongly some