kendo-labs / knockout-kendo

A project to create a robust set of Knockout.js bindings for the Kendo UI widgets.
http://kendo-labs.github.com/knockout-kendo/
273 stars 144 forks source link

Grid header click events are bound twice, breaking sorting #79

Open NickFitz opened 11 years ago

NickFitz commented 11 years ago

Versions: knockout.js v2.2.0, knockout-kendo.js: v0.6.2; kendo.web.min.js: v2012.2.710.

The following occurs when passing a kendo.data.DataSource instance representing a remote data source as the data property. I haven't tested passing anything else.

The column header cells have click event handlers bound to them when they are initialised (and sorting is enabled). This initialisation is occurring twice during the execution of binding.setup().

First, when self.getWidget() is invoked at line 86, it calls directly into kendo.web.js. Then, when self.watchValues() is invoked at line 92, it leads to the creation of a ko.computed to watch the data property; the read() method, when evaluated during its creation, ultimately invokes setDataSource(), which invokes the setDataSource() method of the widget, which causes the initialisation of the column headers to be repeated. The result is that each click on a column header cell is processed twice before any action is taken.

The usual cycle for sorting when sortable.allowUnsort is true is: asc -> desc -> unsorted. As the click event is processed twice (which happens before the remote data is retrieved), every other mode is skipped. Thus the sorting goes: (asc skipped) desc -> (unsorted skipped) asc -> (desc skipped) unsorted. For this case it means no state is unreachable, but the first state on a previously-unsorted column is desc when it should be asc. In the case of a column where sortable.allowUnsort is false, it goes to desc and stays there, as it skips asc each time. Thus it becomes impossible to sort a column in ascending order.

CraigHead commented 11 years ago

Is this related to the binding error when used with Knockout 2.3.0?

NickFitz commented 11 years ago

Whoops, forgot to include Knockout version; added.

No, we are seeing this issue on KO v2.2.0.

CraigHead commented 11 years ago

Ah, well in KO v2.3.0 I get multiple binding errors when using the 'value' binding leaving me stuck with KO 2.2.1 :-)