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
|
-
-
djblets/media/js/datagrid.js (Diff revision 1) 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.
-
-
djblets/media/js/datagrid.js (Diff revision 1) We don't use 'i' in here. Also, same question about this inside $(selector).each()
-
djblets/media/js/datagrid.js (Diff revision 1) The second argument to the each iterator function is the element itself, which would simplify this a lot.
-
djblets/media/js/datagrid.js (Diff revision 1) 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++) {
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