Skip to content

Fix T1308137: repaintChangesOnly restores cell edits after cancel #31850

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Draft
nightskylark wants to merge 1 commit into
base: 25_2
Choose a base branch
from
Draft

Fix T1308137: repaintChangesOnly restores cell edits after cancel #31850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 35 additions & 5 deletions packages/devextreme/js/__internal/grids/grid_core/editing/m_editing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1937,6 +1937,7 @@ class EditingControllerImpl extends modules.ViewController {

protected _cancelEditDataCore() {
const rowIndex = this._getVisibleEditRowIndex();
const rowIndices = this._getRowIndicesToUpdateAfterCancel(rowIndex);

this._beforeCancelEditData();

Expand All @@ -1945,18 +1946,47 @@ class EditingControllerImpl extends modules.ViewController {
this._resetEditColumnName();
this._resetEditRowKey();

this._afterCancelEditData(rowIndex);
this._afterCancelEditData(rowIndex, rowIndices);
}

private _getRowIndicesToUpdateAfterCancel(rowIndex): number[] {
const dataController = this._dataController;
const changes = [...this.getChanges()];
const rowIndices: number[] = changes
.map(({ key }) => dataController.getRowIndexByKey(key))
.filter((index, position, array) => index >= 0 && array.indexOf(index) === position);

if (rowIndex >= 0 && !rowIndices.includes(rowIndex)) {
rowIndices.push(rowIndex);
}

return rowIndices;
}

/**
* @extended: filter_row
*/
protected _afterCancelEditData(rowIndex) {
protected _afterCancelEditData(rowIndex, rowIndices: number[] = []) {
const dataController = this._dataController;
const repaintChangesOnly = this.option('repaintChangesOnly');
const uniqueRowIndices = rowIndices
.filter((index, position, array) => index >= 0 && array.indexOf(index) === position);

dataController.updateItems({
repaintChangesOnly: this.option('repaintChangesOnly'),
});
if (rowIndex >= 0 && !uniqueRowIndices.includes(rowIndex)) {
uniqueRowIndices.push(rowIndex);
}

if (uniqueRowIndices.length) {
dataController.updateItems({
changeType: 'update',
rowIndices: uniqueRowIndices,
repaintChangesOnly,
});
} else {
dataController.updateItems({
repaintChangesOnly,
});
}
}

protected _hideEditPopup(): any {}
Expand Down
15 changes: 12 additions & 3 deletions packages/devextreme/js/__internal/grids/grid_core/editing/m_editing_row_based.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,25 @@ const editingControllerExtender = (Base: ModuleType<EditingController>) => class
return this.getEditMode() === EDIT_MODE_ROW;
}

protected _afterCancelEditData(rowIndex) {
protected _afterCancelEditData(rowIndex, rowIndices: number[] = []) {
const dataController = this._dataController;

if (this.isRowBasedEditMode() && rowIndex >= 0) {
const combinedRowIndices = rowIndices.slice();

[rowIndex, rowIndex + 1].forEach((index) => {
if (index >= 0 && !combinedRowIndices.includes(index)) {
combinedRowIndices.push(index);
}
});

dataController.updateItems({
changeType: 'update',
rowIndices: [rowIndex, rowIndex + 1],
rowIndices: combinedRowIndices,
repaintChangesOnly: this.option('repaintChangesOnly'),
});
} else {
super._afterCancelEditData(rowIndex);
super._afterCancelEditData(rowIndex, rowIndices);
}
}

Expand Down
84 changes: 54 additions & 30 deletions packages/devextreme/js/__internal/grids/grid_core/validating/m_validating.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,41 +844,65 @@ export const validatingEditingExtender = (Base: ModuleType<EditingController>) =
}

protected _beforeSaveEditData(change, editIndex?) {
let result: any = super._beforeSaveEditData.apply(this, arguments as any);
const validationData = this._validatingController._getValidationData(change?.key, true);
const baseResult: any = super._beforeSaveEditData.apply(this, arguments as any);

if (change) {
const isValid = change.type === 'remove' || validationData.isValid;
result = result || !isValid;
} else {
const disposeValidators = this._createInvisibleColumnValidators(this.getChanges());
if (change.type === EDIT_DATA_REMOVE_TYPE) {
return baseResult || false;
}

const disposeValidators = this._createInvisibleColumnValidators([change]);
// @ts-expect-error
result = new Deferred();
this.executeOperation(result, () => {
this._validatingController.validate(true).done((isFullValid) => {
disposeValidators();
this._updateRowAndPageIndices();

// eslint-disable-next-line default-case, @typescript-eslint/switch-exhaustiveness-check
switch (this.getEditMode()) {
case EDIT_MODE_CELL:
if (!isFullValid) {
this._focusEditingCell();
}
break;
case EDIT_MODE_BATCH:
if (!isFullValid) {
this._resetEditRowKey();
this._resetEditColumnName();
this._dataController.updateItems();
}
break;
}
result.resolve(!isFullValid);
});
const deferred = new Deferred();

this.executeOperation(deferred, () => {
this._validatingController.validate(true)
.done(() => {
disposeValidators();
const validationData = this._validatingController._getValidationData(change?.key, true);
const isValid = validationData?.isValid;
deferred.resolve(baseResult || !isValid);
})
.fail((error) => {
disposeValidators();
deferred.reject(error);
});
});

return deferred.promise();
}
return result.promise ? result.promise() : result;

const disposeValidators = this._createInvisibleColumnValidators(this.getChanges());
// @ts-expect-error
const result = new Deferred();
this.executeOperation(result, () => {
this._validatingController.validate(true).done((isFullValid) => {
disposeValidators();
this._updateRowAndPageIndices();

// eslint-disable-next-line default-case, @typescript-eslint/switch-exhaustiveness-check
switch (this.getEditMode()) {
case EDIT_MODE_CELL:
if (!isFullValid) {
this._focusEditingCell();
}
break;
case EDIT_MODE_BATCH:
if (!isFullValid) {
this._resetEditRowKey();
this._resetEditColumnName();
this._dataController.updateItems();
}
break;
}
result.resolve(baseResult || !isFullValid);
}).fail((error) => {
disposeValidators();
result.reject(error);
});
});

return result.promise();
}

/**
Expand Down
87 changes: 87 additions & 0 deletions ...ages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/editing.integration.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5757,6 +5757,93 @@ QUnit.module('API methods', baseModuleConfig, () => {
assert.equal(editor.getInputElement().length, 0, 'cell has no editor');
});

QUnit.testInActiveWindow('editing.changes should collect invalid showEditorAlways edits when repaintChangesOnly is true', function(assert) {
// arrange
const dataGrid = createDataGrid({
dataSource: [
{ id: 1, name: 'Job 1', article: 'Article A' },
{ id: 2, name: 'Job 2', article: 'Article B' },
],
keyExpr: 'id',
loadingTimeout: null,
repaintChangesOnly: true,
columns: [{
dataField: 'name',
showEditorAlways: true,
validationRules: [{ type: 'required' }],
}, 'article'],
editing: {
mode: 'cell',
allowUpdating: true,
},
});
this.clock.tick(10);

const getEditor = (rowIndex) => $(dataGrid.getCellElement(rowIndex, 0)).find('.dx-textbox').dxTextBox('instance');
const blurAnotherCell = (rowIndex) => $(dataGrid.getCellElement(rowIndex === 0 ? 1 : 0, 1)).trigger('dxclick');
const clearNameCell = (rowIndex) => {
dataGrid.editCell(rowIndex, 0);
this.clock.tick(10);
getEditor(rowIndex).option('value', '');
this.clock.tick(10);
blurAnotherCell(rowIndex);
this.clock.tick(10);
};

clearNameCell(0);
clearNameCell(1);

const changes = dataGrid.option('editing.changes');
assert.deepEqual(changes.map((change) => change.key), [1, 2], 'changes tracked for every invalid row');
assert.deepEqual(changes.map((change) => change.data), [{ name: '' }, { name: '' }], 'empty values stored per row');
});

QUnit.testInActiveWindow('cancelEditData should restore showEditorAlways editors when repaintChangesOnly is true', function(assert) {
// arrange
const dataGrid = createDataGrid({
dataSource: [
{ id: 1, name: 'Job 1', article: 'Article A' },
{ id: 2, name: 'Job 2', article: 'Article B' },
],
keyExpr: 'id',
loadingTimeout: null,
repaintChangesOnly: true,
columns: [{
dataField: 'name',
showEditorAlways: true,
validationRules: [{ type: 'required' }],
}, 'article'],
editing: {
mode: 'cell',
allowUpdating: true,
},
});
this.clock.tick(10);

const getEditor = (rowIndex) => $(dataGrid.getCellElement(rowIndex, 0)).find('.dx-textbox').dxTextBox('instance');
const blurAnotherCell = (rowIndex) => $(dataGrid.getCellElement(rowIndex === 0 ? 1 : 0, 1)).trigger('dxclick');
const clearNameCell = (rowIndex) => {
dataGrid.editCell(rowIndex, 0);
this.clock.tick(10);
getEditor(rowIndex).option('value', '');
this.clock.tick(10);
blurAnotherCell(rowIndex);
this.clock.tick(10);
};

clearNameCell(0);
clearNameCell(1);

// act
dataGrid.cancelEditData();
this.clock.tick(10);

// assert
assert.strictEqual(getEditor(0).option('value'), 'Job 1', 'first editor restored');
assert.strictEqual(getEditor(1).option('value'), 'Job 2', 'second editor restored');
assert.deepEqual(dataGrid.option('editing.changes'), [], 'no pending changes');
});

QUnit.test('Using watch in cellPrepared event for editor if repaintChangesOnly', function(assert) {
// arrange
const dataSource = new DataSource({
Expand Down
33 changes: 33 additions & 0 deletions packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/editing.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9140,6 +9140,39 @@ QUnit.module('Editing with real dataController', {
assert.equal(this.option('editing.editRowKey'), 3, 'editRowKey');
});

QUnit.test('Cell values should be restored for all changed rows after cancelEditData when repaintChangesOnly is enabled', function(assert) {
// arrange
const rowsView = this.rowsView;
const $testElement = $('#container');

this.options.repaintChangesOnly = true;
$.extend(this.options.editing, {
allowUpdating: true,
mode: 'cell'
});
rowsView.render($testElement);

// act
this.editCell(0, 0);
this.cellValue(0, 'name', 'updated name 1');
this.closeEditCell();
this.clock.tick(10);

this.editCell(1, 0);
this.cellValue(1, 'name', 'updated name 2');
this.closeEditCell();
this.clock.tick(10);

assert.deepEqual(this.option('editing.changes').map(function(change) { return change.key; }), ['test1', 'test2'], 'changes are tracked for both rows');

this.cancelEditData();
this.clock.tick(10);

// assert
assert.strictEqual($(rowsView.getCellElement(0, 0)).text(), 'test1', 'first row value is restored');
assert.strictEqual($(rowsView.getCellElement(1, 0)).text(), 'test2', 'second row value is restored');
});

['cell', 'batch'].forEach(editMode => {
QUnit.test(`editColumnName should be reset after cancelEditData (editing.mode = ${editMode})`, function(assert) {
// arrange
Expand Down
Loading