Clean up some of the datagrid code.
Review Request #4173 — Created May 26, 2013 and submitted
Clean up some of the datagrid code. This is a minor cleanup that reduces all the "this" confusion by assigning real variables to things as quickly as possible, and reduces all the "var" statements everywhere. It also adds the "$" prefix to jQuery-wrapped elements.
Tested reordering columns. Tested adding/removing columns. Tested sorting columns.
| Description | From | Last Updated |
|---|---|---|
|
I'm kind of confused about "this" here. Does .each bind this to the elements? Also, we don't use 'i' so … |
david
|
|
|
Isn't this the same as $grid? |
david
|
|
|
We don't use 'i' in here. Also, same question about this inside $(selector).each() |
david
|
|
|
The second argument to the each iterator function is the element itself, which would simplify this a lot. |
david
|
-
-
I'm kind of confused about "this" here. Does .each bind this to the elements? Also, we don't use 'i' so might as well not include it in the parameters.
-
-
-
The second argument to the each iterator function is the element itself, which would simplify this a lot.
-
One cute syntactic trick I noticed in the pdf.js source was defining the length inline in here: for (i = 0, rowsLen = table.rows.length; i < rowsLen; i++) {
Review request changed
- Change Summary:
-
Used the second parameter to each() in various cases so we could use real variables instead of "this" in more places.
- Diff:
-
Revision 2 (+92 -78)
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: Ignored Files: djblets/media/js/datagrid.js