Parameters passed to the "editable" callback of a Backgrid.Column do not match the parameters given in the documentation

[floriandammeyer]

I am trying to write custom logic to determine wether a cell should be editable or not depending on some of the model's attributes. Following this issue https://github.com/wyuenho/backgrid/issues/500 I extended the basic Backgrid.Column object and passed it to the Grid constructor alongside the other columns.

var SophisticatedColumn = Backgrid.Column.extend({
    defaults: {
        editable: function(column, model) {
            return model.fancyEditabilityCalculation();
        }
    }
});

new Backgrid.Grid({
    columns: [
        // other columns
        new SophisticatedColumn({
            name: 'foobar',
            label: 'Foobar'
        }),
        // other columns
    ],
    // other settings
});

This conforms to how the documentation states the editable() function's signature should be. See http://wyuenho.github.io/backgrid/api/index.html#!/api/Backgrid.Column:

The method's signature must be function (Backgrid.Column, Backbone.Model): boolean

However, the parameters passed to that function (using any version upwards from 0.3.5), do not comply with that signature. Instead, only one parameter is given to the function and this parameter differs between the value of the attribute that is assigned to the column or the complete model object (maybe dependent on whether the attribute is set on the model or not).

I also tried defining my logic as a separate method and passing its name as a string, but that results in the same issue.

// ...
Backgrid.Column.extend({
    // ...
        editable: 'checkIfEditable'
    },
    checkIfEditable: function(column, model) {
    // ...
});

Is this a bug or am I approaching the problem in the wrong way?

[wyuenho]

It's probably a documentation error. At some point all these *able properties were changed to require functions that return functions.

https://github.com/wyuenho/backgrid/blob/master/src/column.js#L197-L205 https://github.com/wyuenho/backgrid/blob/master/src/cell.js#L258

If it makes things easier, you can override Column::editable() directly instead of putting it into a model property.

[floriandammeyer]

I figured out now that the documentation is indeed wrong. column is not passed as the first parameter, but is used as the function's execution context. Therefore this inside the editable callback refers to the Column model and the first argument passed to the function refers to the model of the current row. I don't even have to extend the basic Column model to implement my logic for editing, because I can simply pass a function as the editable attribute:

new Backgrid.Grid({
    columns: [
        // ...
        {
            name: 'foobar',
            label: 'Foobar',
            sortable: false,
            editable: function(model) {
                console.debug(this); // -> this refers to the Column model
                return model.fancyEditabilityCalculation();
            }
        },
        // ...
    ],
    // ...
});

I recommend updating the documentation accordingly so others don't have to do some useless hours of debugging ;)

There is another quirk however. The editable callback is called with different parameters from different places. The Cell.prototype.updateStateClassesMaybe method calls the editable callback passing the model of the current row, as seen in https://github.com/wyuenho/backgrid/blob/master/src/cell.js#L258, whereas HeaderCell.prototype.initialize passes the whole collection instead of a single model to the editable function: https://github.com/wyuenho/backgrid/blob/master/src/header.js#L55. This makes it a little hard to come up with a concise implementation of the editable callback, because I have to react to the different parameters. A little like implementing polymorphism.

I wonder why the HeaderCell needs to set the 'editable' class on the header element at all. Is it used for styling or for events or something? In my use case, the editability can't be determined for the entire column in general, but has to be deteremined per row. Hence setting a global editability for that specific column can't be done. I have worked around that issue now by simply returning false for the HeaderCell, but that solution is a thorn in my side, it just doesn't feel right.

// ...
editable: function(model) 
{
    if(model instanceof Backbone.Collection)
    {
        return false;
    }
    return model.fancyEditabilityCalculation();
}
// ...

[wyuenho]

Thanks for this very thorough investigation. The issue in the Header is probably a bug. Hardly ever anyone uses it so it hasn't been tested in the battle much.

This convoluted piece of shit is probably done for backward compatibility. I'd appreciate a PR if you can send one over. I'll fix the tests if they are broken

[wyuenho]

Ah I remember why the *able functions are passed a collection in the header. The reason was there's no access to a model inside the header. I'll update the docs.