ColumnReorder is broken in dgrid 0.3.7 #525

lixopmstp · 2013-04-04T20:29:49Z

When I try to swap columns the grid headers disappear. Observed in Firefox 20 (Firebug shows a "too much recursion" error) and MSIE 9 with dojo 1.8.1. Works fine in dgrid 0.3.6.
Note I'm using the following extensions and plugins:

    "dgrid/Grid", 
    "dgrid/selector", 
    "dgrid/Keyboard", 
    "dgrid/Selection", 
    "dgrid/extensions/Pagination",
    "dgrid/extensions/ColumnReorder",
    "dgrid/extensions/ColumnResizer", 
    "dgrid/extensions/ColumnHider",

The text was updated successfully, but these errors were encountered:

lixopmstp · 2013-04-04T20:38:35Z

Chrome 26.0.1410.43 m (Windows) also has trouble with the recursion. It's developer tools show a bit more info:

Uncaught RangeError: Maximum call stack size exceeded
declare.row
column.disabled
grid.allowSelect
column.disabled
grid.allowSelect
column.disabled
... (repeats manytimes)

lixopmstp · 2013-04-04T20:51:31Z

This may not matter, but some columns have formatters and some renderCells, a couple of them also have renderCellHeaders.

kfranqueiro · 2013-04-10T20:11:58Z

Looking at that stack trace, it looks like this issue might have nothing to do with extensions, and everything to do with how you are using Selection and selector. Can you provide code indicating how your columns are set up, and if you have specified a custom allowSelect method implementation?

lixopmstp · 2013-04-12T14:42:49Z

Hi,

I cannot easily show how the columns are created (they’re dynamically created in a JSP based on a user query and preferences).
The grid is initially created with the following selection options:

    selectionMode: "none",
    deselectOnRefresh: false,
    allowSelectAll: true,
    cellNavigation: true,
    allowTextSelection: true,

I also have an on("dgrid-select, dgrid-deselect") vent handler that simply enables/disables a few buttons and links (not on the grid)

Hope this helps,
Pedro

From: Kenneth G. Franqueiro [mailto:[email protected]]
Sent: Wednesday, April 10, 2013 4:12 PM
To: SitePen/dgrid
Cc: Pedro Proenca
Subject: Re: [dgrid] ColumnReorder is broken in dgrid 0.3.7 (#525)

Looking at that stack trace, it looks like this issue might have nothing to do with extensions, and everything to do with how you are using Selection and selector. Can you provide code indicating how your columns are set up, and if you have specified a custom allowSelect method implementation?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/525#issuecomment-16198852.

APGarchev · 2013-04-26T14:54:58Z

Hi, I get the same problem and traced the code. Here is what I found.

The problem is caused by a endless recursion between column.disabled and grid.allowSelect methos.

The selector method defines column.renderHeaderCell. When the selector column type is radio or grid.allowSelectAll is true it calls a method setupSelectionEvents defined in selector.js, else the renderInput calls the setupSelectionEvents method.
When the selector column hasn't defined disabled property and it's not a method, the setupSelectionEvents attaches to the selector column a default disabled method which calls the grid.allowSelect method (by default it returns always true), but else the grid.allowSelect is updated to call the original grid.allowSelect and the origianl column.default method and the column.disabled is updated to call the updated grid.allowSelect method.

The reorder action caused call of the grid._setColumns (or _setColumnSets/_setSubRows) method, all existing columns are destroyed (grid._destroyColumns) and as a result the flag (grid._hasSelectorInputListener) that prevents the execution of setupSelectionEvents is set to false. The column.renderHeaderCell is called again and that caused again update of the grid.allowSelect and the column.disabled.

Then the renderInput method is executed and it executes the column.disabled method, in result a recursion between the column.disabled method and the grid.allowSelect happens.

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <link rel="stylesheet" href="dojo/resources/dojo.css" />
  <link rel="stylesheet" href="dijit/themes/claro/claro.css" />
  <style>
    .grid {
      width: 700px;
      margin: 10px;
    }
  </style>
  <title>Tutorial: Hello dgrid!</title>
  <script>
    var dojoConfig = { async: true, parseOnLoad: true };
  </script>
</head>
<body class="claro">
  <script src="dojo/dojo.js"></script>
  <script>
  require([
      "dojo/_base/declare", "dojo/request", "dojo/store/Memory",
      "dgrid/Grid", "dgrid/ColumnSet", "dgrid/selector", "dgrid/Selection",
      "dgrid/extensions/Pagination", "dgrid/extensions/ColumnReorder"
  ], function (declare, request, Memory, Grid, ColumnSet, selector, Selection, 
      Pagination, ColumnReorder) {
    request("hof-batting.json", { handleAs: "json" }).then(function (response) {
      var store = new Memory({ "idProperty": "id", data: response });
      var CustomGrid = declare([Grid, Selection, ColumnReorder, Pagination]);
      var grid = new CustomGrid({
        store: store,
        columns: [
          selector({ field: "id", "selectorType": "checkbox", "reorderable": false,
              disabled: function() { return false; }}),
          { field: "first", label: "First Name" },
          { field: "last", label: "Last Name" },
          { field: "age", label: "Age" }
        ],
        allowSelectAll: true
      }, "grid");
      grid.startup();
    });
  });
  </script>
  <div id="grid"></div>
</body>
</html>

ghost · 2013-04-27T12:32:01Z

IIRC, I posted a fix for this as a pull request. When Ken gets a. Chance to review it, it will likely be merged into master.

Sent from my iPhone

On Apr 26, 2013, at 9:55 AM, APGarchev [email protected] wrote:

Hi, I get the same problem and traced the code. Here is what I found.

The problem is caused by a endless recursion between column.disabled and grid.allowSelect methos.

The selector method defines column.renderHeaderCell. When the selector column type is radio or grid.allowSelectAll is true it calls a method setupSelectionEvents defined in selector.js, else the renderInput calls the setupSelectionEvents method.
When the selector column hasn't defined disabled property and it's not a method, the setupSelectionEvents attaches to the selector column a default disabled method which calls the grid.allowSelect method (by default it returns always true), but else the grid.allowSelect is updated to call the original grid.allowSelect and the origianl column.default method and the column.disabled is updated to call the updated grid.allowSelect method.

The reorder action caused call of the grid._setColumns (or _setColumnSets/_setSubRows) method, all existing columns are destroyed (grid._destroyColumns) and as a result the flag (grid._hasSelectorInputListener) that prevents the execution of setupSelectionEvents is set to false. The column.renderHeaderCell is called again and that caused again update of the grid.allowSelect and the column.disabled.

Then the renderInput method is executed and it executes the column.disabled method, in result a recursion between the column.disabled method and the grid.allowSelect happens.
<style> .grid { width: 700px; margin: 10px; } </style> <title>Tutorial: Hello dgrid!</title> <script> var dojoConfig = { async: true, parseOnLoad: true }; </script> <script src="dojo/dojo.js"></script> <script> require([ "dojo/_base/declare", "dojo/request", "dojo/store/Memory", "dgrid/Grid", "dgrid/ColumnSet", "dgrid/selector", "dgrid/Selection", "dgrid/extensions/Pagination", "dgrid/extensions/ColumnReorder" ], function (declare, request, Memory, Grid, ColumnSet, selector, Selection, Pagination, ColumnReorder) { request("hof-batting.json", { handleAs: "json" }).then(function (response) { var store = new Memory({ "idProperty": "id", data: response }); var CustomGrid = declare([Grid, Selection, ColumnReorder, Pagination]); var grid = new CustomGrid({ store: store, columns: [ selector({ field: "id", "selectorType": "checkbox", "reorderable": false, disabled: function() { return false; }}), { field: "first", label: "First Name" }, { field: "last", label: "Last Name" }, { field: "age", label: "Age" } ], allowSelectAll: true }, "grid"); grid.startup(); }); }); </script>
— Reply to this email directly or view it on GitHub.

lixopmstp · 2013-04-29T14:27:19Z

I had a change to try it out with a local patched 0.3.7 version and it seems fine.

Thanks

kfranqueiro · 2015-01-29T19:34:26Z

Sorry this issue ended up left open. Yes, I believe this was fixed in 0.3.8. Furthermore, in 0.4, there is no longer a disabled property for selector columns since it is redundant of Selection#allowSelect.

kfranqueiro closed this as completed Jan 29, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ColumnReorder is broken in dgrid 0.3.7 #525

ColumnReorder is broken in dgrid 0.3.7 #525

lixopmstp commented Apr 4, 2013

lixopmstp commented Apr 4, 2013

lixopmstp commented Apr 4, 2013

kfranqueiro commented Apr 10, 2013

lixopmstp commented Apr 12, 2013

APGarchev commented Apr 26, 2013

ghost commented Apr 27, 2013

lixopmstp commented Apr 29, 2013

kfranqueiro commented Jan 29, 2015

ColumnReorder is broken in dgrid 0.3.7 #525

ColumnReorder is broken in dgrid 0.3.7 #525

Comments

lixopmstp commented Apr 4, 2013

lixopmstp commented Apr 4, 2013

lixopmstp commented Apr 4, 2013

kfranqueiro commented Apr 10, 2013

lixopmstp commented Apr 12, 2013

APGarchev commented Apr 26, 2013

ghost commented Apr 27, 2013

lixopmstp commented Apr 29, 2013

kfranqueiro commented Jan 29, 2015