[4.1.0] Ext.ux.CheckColum with "editable: true" causes a JavaScript error on click

[Daniil]

REQUIRED INFORMATION

Ext version tested:

• Ext 4.1.0

Browser versions tested against:

• Any

DOCTYPE tested against:

• Any

Description:

• If a CheckColumn with "editable: true" is used within GridPanel, it causes a "mousedown is not defined" JavaScript error when any checkbox is clicked.
• If a CheckColumn with "editable: true" is used within TreePanel, it causes a "store.getAt is not a function" JavaScript error when any checkbox is clicked.

Steps to reproduce the problem:

• Run any GridPanel/TreePanel with CheckColumn with "editable: true" and click any checkbox.

The result that was expected:

• Clicking on a checkbox inverts its state.

The result that occurs instead:

• A JavaScript occurs.

Test Case:

Apologize, I am in haste. Please see the Steps to reproduce the problem, Debugging, Possible Fix sections.

HELPFUL INFORMATION

Debugging already done:

• There are three obvious problems with the CheckColumn processEvent function.

1. The problem #1.

This line

if (mousedown) {
    e.stopEvent();
}

but "mousedown" is not defined as a variable.

I guess it should look

if (type == 'mousedown') { 
    e.stopEvent();
}

2. The problem #2.

This line

if (!me.stopSelection)

but "me" is not defined as a variable.

I have add

me = this

in the "var" section

3. The problem #3.

This line

record = store.getAt(recordIndex),

It causes a JavaScript error in the case with a TreePanel, because there is no getAt method of its TreeStore.

It might be replaced with

record = store.getAt ? store.getAt(recordIndex) : view.getRecord(view.getNode(recordIndex)),

Possible fix:

• Here is the full code of the processEvent function with the suggested fixes.

Ext.ux.CheckColumn processEvent

Ext.ux.CheckColumn.override({
    processEvent: function (type, view, cell, recordIndex, cellIndex, e) {
        if (this.editable && (type == 'mousedown' || (type == 'keydown' && (e.getKey() == e.ENTER || e.getKey() == e.SPACE)))) {

            var me = this,
                store = view.panel.store,
                record = store.getAt ? store.getAt(recordIndex) : view.getRecord(view.getNode(recordIndex)), //store.getAt(recordIndex),
                dataIndex = this.dataIndex,
                checked = !record.get(dataIndex),
                eventTarget = view.panel.editingPlugin || view.panel;

            var ev = {
                grid: view.panel,
                record: record,
                field: dataIndex,
                value: record.get(this.dataIndex),
                row: view.getNode(recordIndex),
                column: this,
                rowIdx: recordIndex,
                colIdx: cellIndex,
                cancel: false
            };

            if (eventTarget.fireEvent("beforeedit", eventTarget, ev) === false || ev.cancel === true) {
                return;
            }

            ev.originalValue = ev.value;
            ev.value = checked;

            if (eventTarget.fireEvent("validateedit", eventTarget, ev) === false || ev.cancel === true) {
                return;
            }

            if (this.singleSelect) {
                store.suspendEvents();
                store.each(function (record, i) {
                    var value = (i == recordIndex);
                    if (value != record.get(dataIndex)) {
                        record.set(dataIndex, value);
                    }
                });
                store.resumeEvents();
                store.fireEvent("datachanged", store);
            } else {
                record.set(dataIndex, checked);
            }

            this.fireEvent('checkchange', this, recordIndex, record, checked);
            eventTarget.fireEvent('edit', eventTarget, ev);
                    
            if (type == 'mousedown') { //if (mousedown) {
                e.stopEvent();
            }

            // Selection will not proceed after this because of the DOM update caused by the record modification
            // Invoke the SelectionModel unless configured not to do so
            if (!me.stopSelection) {
                view.selModel.selectByPosition({
                    row: recordIndex,
                    column: cellIndex
                });
            }
            // cancel selection.
            return false;
        } else {
            return this.callParent(arguments);
        }
    }
});

[cellinger]

I am experiencing the second problem with Ext.tree.Panels. Here is a working example of the problem. Please indicate if this will be assigned a bug id and fixed in a future release.

[evant]

@OP Not sure where you got that code from, in the 4.1.0 release the code reads:

processEvent: function(type, view, cell, recordIndex, cellIndex, e) {
        var me = this,
            key = type === 'keydown' && e.getKey(),
            mousedown = type == 'mousedown';

        if (mousedown || (key == e.ENTER || key == e.SPACE)) {
            var record = view.panel.store.getAt(recordIndex),
                dataIndex = me.dataIndex,
                checked = !record.get(dataIndex);

            // Allow apps to hook beforecheckchange
            if (me.fireEvent('beforecheckchange', me, recordIndex, checked) !== false) {
                record.set(dataIndex, checked);
                me.fireEvent('checkchange', me, recordIndex, checked);

                // Mousedown on the now nonexistent cell causes the view to blur, so stop it continuing.
                if (mousedown) {
                    e.stopEvent();
                }

                // Selection will not proceed after this because of the DOM update caused by the record modification
                // Invoke the SelectionModel unless configured not to do so
                if (!me.stopSelection) {
                    view.selModel.selectByPosition({
                        row: recordIndex,
                        column: cellIndex
                    });
                }

                // Prevent the view from propagating the event to the selection model - we have done that job.
                return false;
            } else {
                // Prevent the view from propagating the event to the selection model if configured to do so.
                return !me.stopSelection;
            }
        } else {
            return me.callParent(arguments);
        }
    }

The "me" var is there, the mousedown variable is already defined.

@cellinger Currently it's only intended for use with the grid. I'll push it to the track as a new feature, however it will be fairly low priority. If you really need it you could open a support ticket to have them help you out.

[Daniil]

@OP Not sure where you got that code from, in the 4.1.0 release the code reads:

My fault, Evan. I forgot that we overrode CheckColumn to add some additional functionality. I was sure it was update to 4.1.0. So, the two first bugs are our bugs (or, maybe, came from some previous code, never mind).

Thanks a lot for the answer.