Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add editCell to gridPro #3354

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,6 @@ export declare class InlineEditingMixinClass {
protected _stopEdit(shouldCancel?: boolean, shouldRestoreFocus?: boolean): void;

protected _switchEditCell(e: KeyboardEvent): void;

public editCell(row: Number, col: Number, userOriginated: Boolean): void;
roastedcpu marked this conversation as resolved.
Show resolved Hide resolved
}
57 changes: 57 additions & 0 deletions packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,63 @@ export const InlineEditingMixin = (superClass) =>
super._updateItem(row, item);
}

/**
roastedcpu marked this conversation as resolved.
Show resolved Hide resolved
* @param {Number | String} row
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Examining the code, seems that row could also be an item?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not anymore, but I'd like this possibility, especially because of scrolling. Passing the index only works well if all items are being shown.

* @param {Number | String} col
* @param {Boolean} userOriginated
* @public
*/
editCell(row, col, userOriginated) {
if (userOriginated && this.hasAttribute('disabled')) {
throw new Error('Grid is disabled.');
}
roastedcpu marked this conversation as resolved.
Show resolved Hide resolved

const columns = this._getColumns().filter((col) => !col.hidden);

let colIdx = -1;
// If col is not a number, maybe it's because the id was passed instead
if (isNaN(col)) {
const matchingColumn = columns.filter((c) => c.id == col);
if (!matchingColumn || matchingColumn.length == 0) {
throw new Error(`col with id ${col} was not found`);
}
colIdx = columns.indexOf(matchingColumn[0]);
} else {
colIdx = col;
}

let rowIdx = -1;
// If row is not a number, maybe it's because the item was passed instead
if (isNaN(row)) {
if (!Object.hasOwnProperty.call(row, 'key') || isNaN(row.key)) {
throw new Error('Invalid object passed as row item.');
}
roastedcpu marked this conversation as resolved.
Show resolved Hide resolved
rowIdx = row.key - 1;
} else {
rowIdx = row;
}

// Make sure col[colIdx] exists and is editable:
if (colIdx > columns.length || colIdx < 0) {
throw new Error('Invalid colIdx (out of bounds)');
}

const column = columns[colIdx];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Column reordering isn't taken into account

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you can pass a columnId and that ensures reordering is not a problem. What do you suggest for situations where the index is directly passed ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the API design accepts "path" only

if (!this._isEditColumn(column)) {
throw new Error('Column is not editable');
}

// Get rows (excluding header)
const tRows = this.$.table.getElementsByTagName('tbody').items.rows;
roastedcpu marked this conversation as resolved.
Show resolved Hide resolved

// Make sure row[rowIdx] exists
if (rowIdx > tRows.length || rowIdx < 0) {
throw new Error('Invalid rowIdx (out of bounds)');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't sound right to throw an error depending on the scroll position of the user & how many items the virtual scrolling engine has decided to render in DOM?

}

this._startEdit(tRows[rowIdx].cells[colIdx], column);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rowIdx probably refers to a virtual index and not the index of the element in the DOM (which wouldn't be correct for the API due to virtual scrolling). If rowIdx was, let's say 1000, this would fail since there are not that many physical rows in the DOM.

}

/**
* Fired before exiting the cell edit mode, if the value has been changed.
* If the default is prevented, value change would not be applied.
Expand Down
115 changes: 115 additions & 0 deletions packages/grid-pro/test/edit-column-renderer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,4 +289,119 @@ describe('edit column renderer', () => {
expect(grid.items[0].name).to.equal('New');
});
});

describe('programatically enter edit mode', () => {
let grid;

beforeEach(() => {
grid = fixtureSync(`
<vaadin-grid-pro>
<vaadin-grid-pro-edit-column path="name" id="name"></vaadin-grid-pro-edit-column>
<vaadin-grid-pro-column path="age"></vaadin-grid-pro-column>
<vaadin-grid-column>
<template>[[item.name]]</template>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use the deprecated template API

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just copied from other examples. How should I do it ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case the column isn't essential for the test, it can be removed. If it is, see the documentation for how custom content is provided for grid column cells (renderer API)

</vaadin-grid-column>
</vaadin-grid-pro>
`);
const column = grid.firstElementChild;
grid.items = createItems();

column.renderer = function (root, owner, model) {
root.innerHTML = '';
const wrapper = document.createElement('div');
const text = document.createTextNode(model.index + ' ' + model.item.name);
wrapper.appendChild(text);
root.appendChild(wrapper);
};

flushGrid(grid);
});

it('enter edit mode programatically should fail for user originated call when grid is disabled', () => {
grid.setAttribute('disabled', true);
expect(() => {
grid.editCell(10, 10, true);
}).to.throw(Error, 'Grid is disabled.');

expect(() => {
grid.editCell(0, 0, false);
}).to.not.throw(Error, 'Grid is disabled.');
});

it('enter edit mode programatically should fail for non-existing indexes', () => {
expect(() => {
grid.editCell(0, 10, false);
}).to.throw(Error, /out of bounds/);

expect(() => {
grid.editCell(10, 0, false);
}).to.throw(Error, /out of bounds/);
});

it('enter edit mode programatically should fail for non-existing column (by id)', () => {
expect(() => {
grid.editCell(0, 'name', false);
}).to.not.throw(Error, 'col with id name was not found');

expect(() => {
grid.editCell(10, 'invalid', false);
}).to.throw(Error, 'col with id invalid was not found');
});

it('enter edit mode programatically should fail for non-existing row (by key)', () => {
expect(() => {
grid.editCell({ key: 1 }, 0, false);
}).to.not.throw(Error, 'Invalid object passed as row item.');

expect(() => {
grid.editCell('invalid', 0, false);
}).to.throw(Error, 'Invalid object passed as row item.');

expect(() => {
grid.editCell({ key: 'invalid' }, 0, false);
}).to.throw(Error, 'Invalid object passed as row item.');

expect(() => {
grid.editCell({ key: -1 }, 0, false);
}).to.throw(Error, 'Invalid rowIdx (out of bounds)');
});

it('enter edit mode programatically should fail for non-editable columns', () => {
expect(() => {
grid.editCell(0, 0, false);
}).to.not.throw(Error, 'Column is not editable');

expect(() => {
grid.editCell(0, 1, false);
}).to.throw(Error, 'Column is not editable');
});

it('enter edit mode programatically (x,y) should call _startEdit', () => {
const cell = getContainerCell(grid.$.items, 0, 0);
grid.editCell(0, 0, false);
const editor = getCellEditor(cell);
expect(editor).to.be.ok;
});

it('enter edit mode programatically (x,colId) should call _startEdit', () => {
const cell = getContainerCell(grid.$.items, 0, 0);
grid.editCell(0, 'name', false);
const editor = getCellEditor(cell);
expect(editor).to.be.ok;
});

it('enter edit mode programatically (rowKey,y) should call _startEdit', () => {
const cell = getContainerCell(grid.$.items, 0, 0);
grid.editCell({ key: 1 }, 0, false);
const editor = getCellEditor(cell);
expect(editor).to.be.ok;
});

it('enter edit mode programatically (rowKey,colId) should call _startEdit', () => {
const cell = getContainerCell(grid.$.items, 0, 0);
grid.editCell({ key: 1 }, 'name', false);
const editor = getCellEditor(cell);
expect(editor).to.be.ok;
});
});
});