HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

ExtJS Grid Plugin

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
extjsplugingrid

Problem

The code below is a plugin I wrote for Ext.grid.GridPanel, which basically allows you to have a bit more control over how rows are striped in the grid. By default you can only have every other row in alternate colour. With this plugin you an have for example 5 in basic color, 5 in alternate and so on.

What bothers me about it, is that it basically is 100+ lines copied from Ext.grid.GridView with no more than 10 lines of my modifications. I would love to know if there's a way to do it in a more elegant way.

My second concern is with that it's a plugin for Ext.grid.GridPanel, but it actually modifies the underlying Ext.grid.GridView. As far as I know, there is no way to attach a plugin to a GridView with current framework architecture, but perhaps you DO know the way.

Legal stuff: The code is largely based on ExtJS source code, and as such it's released under GNU GPL license v3 license

```
MyNamespace.grid.plugins.RowStriper = Ext.extend(Object, {
constructor : function(config) {
config = config || {};
Ext.apply(this, config);
},
init : function(parent) {
if (parent instanceof Ext.grid.GridPanel) {
this.parent = parent;
this.parent.stripeInterval = this.stripeInterval;
parent.on('destroy', this.onDestroy, this);
Ext.apply(parent.getView(), this.parentViewOverrides);
}
},
onDestroy : Ext.emptyFn,
parentViewOverrides : {
doRender : function(columns, records, store, startRow, colCount, stripe) {
var templates = this.templates,
cellTemplate = templates.cell,
rowTemplate = templates.row,
last = colCount - 1;

var tstyle = 'width:' + this.getTotalWidth() + ';';

// buffers
var rowBuffer = [],
colBuffer = [],
rowParams = {tstyle: tstyle},
meta = {},
column,
record;

//build up each row's HTML
for (var j = 0, len = records.length; j (this

Solution

I don't see how to improve the duplication much: you're extending code that unfortunately wasn't meant to be extended.

What you could do to help anyone finding your code in the wild is to highlight changes and maybe explain why you're doing the calculations you're doing. How would you generalize your code to alternate between three or more row styles?

//set up row striping and row dirtiness CSS classes
    var alt = [];

/// -> Comment about how you're making the stripe interval magic happen here
            if (stripe && (rowIndex % (this.grid.stripeInterval * 2)) > (this.grid.stripeInterval - 1)) {
/// -^ Consider breaking this up into assignment into a var and subsequent if, or 
///    even nesting ifs: it might be more readable by separating logical steps.
///    A helper to be used below would be an option, and if you're going to generalize
///    it could make extending easier.

      alt[0] = 'x-grid3-row-alt';
    }

/// [...]

     if (!skipStripe) {
                    r.className = r.className.replace(this.rowClsRe, ' ');
        if ((i % (stripeInterval * 2)) > (stripeInterval - 1)){
                        r.className += ' x-grid3-row-alt';
/// -^ Same comments apply here, specially about highlighting your changes.


You could improve modularity in your version then propose a patch to make upstream easier to work with, so we don't have to suffer the same fate you did ;)

Code Snippets

//set up row striping and row dirtiness CSS classes
    var alt = [];

/// -> Comment about how you're making the stripe interval magic happen here
            if (stripe && (rowIndex % (this.grid.stripeInterval * 2)) > (this.grid.stripeInterval - 1)) {
/// -^ Consider breaking this up into assignment into a var and subsequent if, or 
///    even nesting ifs: it might be more readable by separating logical steps.
///    A helper to be used below would be an option, and if you're going to generalize
///    it could make extending easier.

      alt[0] = 'x-grid3-row-alt';
    }

/// [...]

     if (!skipStripe) {
                    r.className = r.className.replace(this.rowClsRe, ' ');
        if ((i % (stripeInterval * 2)) > (stripeInterval - 1)){
                        r.className += ' x-grid3-row-alt';
/// -^ Same comments apply here, specially about highlighting your changes.

Context

StackExchange Code Review Q#174, answer score: 4

Revisions (0)

No revisions yet.