LinkedInAttic / Backbone.TableView

Backbone View to render collections as tables
Apache License 2.0
71 stars 17 forks source link

Remove dependence on Backbone.History #5

Closed ghost closed 12 years ago

ghost commented 12 years ago

Unless I'm mistaken, it seems that TableView in its current form is dependent upon the use of Backbone.History. Is this necessary?

For example, on line 113 in JS, in TableView.prototype.initialize():

TableView.prototype.initialize = function() {
//...
this.data = $.extend({}, this.initialData, this.parseQueryString(Backbone.history.fragment));

this code block is called without checking to see if Backbone.history exists.

Yet elsewhere in the code, in TableView.prototype.updateUrl() (line 193 in JS):

TableView.prototype.updateUrl = function(replace) {
//...
if (this.router) {
    uri = Backbone.history.fragment;
}

You check for the existence of this.router before attempting to access Backbone.history.fragment.

Is the use of Backbone.History necessary for TableView, and if not, can this issue be corrected?

I "fixed" this by performing another check in TableView.prototype.initialize for this.router before merging data from this.parseQueryString(Backbone.history.fragment) into this.data. ex:

this.data = $.extend({}, this.initialData);
if (this.router) {
    this.data = $.extend(this.data, this.parseQueryString(Backbone.history.fragment));
}
jpbottaro commented 12 years ago

Thanks for this fragnemesis! I did the fix you described in coffeescript.