SitePen / dgrid

A lightweight, mobile-ready, data-driven, modular grid widget designed for use with dstore
http://dgrid.io/
Other
628 stars 298 forks source link

Removed row handing in Editor onblur #1460

Closed claudeG closed 4 years ago

claudeG commented 4 years ago

During many years our framework has been working with Dgrid-0.3 plus an integration layer. I am currently preparing the upgrade to Dgrid-1.3.1. We have the following usecase: our Dgrid instances may show an "actions" column where each cell contains a drop-down with a "delete this row" button. This was implemented as an editor.

I am facing the following error : When the user clicks the "delete this row" button, the row is well... deleted. But when she clicks the delete button on another row, a null-pointer error occurs in:

In Editor#_createSharedEditor, function onblur

var parentNode = node.parentNode, // null because node is no more in the DOM options = ... cell = self.cell(node); // null because node is no more in the grid ... on.emit(cell.element, ...) // no "element" property in null

You might think that our implementation is probably just incorrect. For sure it is not perfect. Nevertheless, few lines below, the following comment indicates that you also admit that the row may be present or may have been deleted :

"// If the row is still present (i.e. we didn't blur due to removal),"

So in order to complete the handling of nulls, a simple guard could solve the problem:

if(cell && parentNode) { <<<<<<<<<<<<<<<<<<<<

// emit an event immediately prior to removing an editOn editor
on.emit(cell.element, 'dgrid-editor-hide', {
    grid: self,
    cell: cell,
    column: column,
    editor: cmp,
    bubbles: true,
    cancelable: false
});
column._editorBlurHandle.pause();
// Remove the editor from the cell, to be reused later.
parentNode.removeChild(node);

if (cell.row) {
    // If the row is still present (i.e. we didn't blur due to removal),
    // clear out the rest of the cell's contents, then re-render with new value.
    domClass.remove(cell.element, 'dgrid-cell-editing');
    domConstruct.empty(parentNode);
    Grid.appendIfNode(parentNode, column.renderCell(cell.row.data, self._activeValue, parentNode,
        self._activeOptions ? lang.delegate(options, self._activeOptions) : options));
}

} <<<<<<<<<<<<<<<<<<<<

Thanks.

Claude

msssk commented 4 years ago

Can you provide a test case for this? For starters, this is not what dgrid/Editor was designed for. It would be better to provide a renderCell function to the column definition or use a tooltip.

However, with a simple test that does what you have described I don't run into any errors:

require([
    'dijit/form/DropDownButton',
    'dijit/DropDownMenu',
    'dijit/MenuItem',
    'dgrid/Editor',
    'dgrid/OnDemandGrid',
    'dstore/Memory',
    'dstore/Trackable'
], function (DropDownButton, DropDownMenu, MenuItem, Editor, OnDemandGrid, MemoryStore, Trackable) {
    var TOTAL_ROW_COUNT = 15;

    var deleteRowItem = new MenuItem({
        label: 'delete',
        onClick: function () {
            store.remove(grid._focusedEditorCell.row.id);
        }
    });
    var menu = new DropDownMenu();
    menu.addChild(deleteRowItem);

    var store = new (MemoryStore.createSubclass([ Trackable ]))({
        data: createData()
    });

    var grid = new (OnDemandGrid.createSubclass([ Editor ]))({
        collection: store,
        columns: {
            firstName: {
                label: 'First Name'
            },
            lastName: {
                label: 'Last Name'
            },
            col3: {
                label: 'action',
                get: function () {
                    return '';
                },
                editOn: 'click',
                editor: DropDownButton,
                editorArgs: {
                    label: 'action',
                    dropDown: menu
                }
            }
        }
    }, 'grid');

    grid.startup();

    function createData() {
        var data = [];
        var column;
        var i;
        var item;

        for (i = 0; i < TOTAL_ROW_COUNT; i++) {
            item = {};
            for (column in { firstName: 1, lastName: 1 }) {
                item.id = i;
                item[column] = column + '_' + (i + 1);
            }
            data.push(item);
        }

        return data;
    }
});
msssk commented 4 years ago

dgrid's Editor received some blur-related fixes in https://github.com/SitePen/dgrid/commit/a4783e82aae03a5574df553731a45d048118b32c#diff-a42b26912b6ab9debddd736cf956cc85 which will be in the 1.3.2 release.

If you continue to experience problems please provide a test case.