patternjavascriptMinor
ExtJS Grid Plugin
Viewed 0 times
extjsplugingrid
Problem
The code below is a plugin I wrote for
What bothers me about it, is that it basically is 100+ lines copied from
My second concern is with that it's a plugin for
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
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?
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 ;)
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.