Skip to content
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.

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

Improve editor's behavior while dataset altering #10963

Merged
merged 14 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changelogs/10963.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"issuesOrigin": "private",
"title": "Improved editor's behavior after dataset altering",
"type": "changed",
"issueOrPR": 10963,
"breaking": false,
"framework": "none"
}
240 changes: 69 additions & 171 deletions handsontable/src/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal

const onIndexMapperCacheUpdate = ({ hiddenIndexesChanged }) => {
if (hiddenIndexesChanged) {
this.selection.refresh();
this.selection.commit();
}
};

Expand Down Expand Up @@ -412,7 +412,12 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
removeClass(this.rootElement, ['ht__selection--rows', 'ht__selection--columns']);
}

this._refreshBorders(null);
if (selection.getSelectionSource() !== 'shift') {
editorManager.closeEditor(null);
}

instance.view.render();
editorManager.prepareEditor();
});

this.selection.addLocalHook('beforeSetFocus', (cellCoords) => {
Expand All @@ -428,7 +433,9 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
viewportScroller.scrollTo(cellCoords);
}

this._refreshBorders(null);
editorManager.closeEditor();
instance.view.render();
editorManager.prepareEditor();
});

this.selection.addLocalHook('afterSelectionFinished', (cellRanges) => {
Expand All @@ -450,9 +457,9 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
});

this.selection.addLocalHook('afterDeselect', () => {
editorManager.destroyEditor();
editorManager.closeEditor();
instance.view.render();

this._refreshBorders();
removeClass(this.rootElement, ['ht__selection--rows', 'ht__selection--columns']);

this.runHooks('afterDeselect');
Expand Down Expand Up @@ -552,45 +559,7 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
startPhysicalIndex: startRowPhysicalIndex,
} = datamap.createRow(index, amount, { source, mode: insertRowMode });

if (rowDelta) {
const currentSelectedRange = selection.selectedRange.current();
const currentFromRange = currentSelectedRange?.from;
const currentFromRow = currentFromRange?.row;
const startVisualRowIndex = instance.toVisualRow(startRowPhysicalIndex);

if (selection.isSelectedByCorner()) {
selection.selectAll(true, true, {
disableHeadersHighlight: true,
});

} else if (isDefined(currentFromRow) && currentFromRow >= startVisualRowIndex) {
// Moving the selection (if it exists) downward – it should be applied to the "old" row.
// TODO: The logic here should be handled by selection module.
const { row: currentToRow, col: currentToColumn } = currentSelectedRange.to;
let currentFromColumn = currentFromRange.col;

// Workaround: headers are not stored inside selection.
if (selection.isSelectedByRowHeader()) {
currentFromColumn = -1;
}

// Remove from the stack the last added selection as that selection below will be
// replaced by new transformed selection.
selection.getSelectedRange().pop();

instance.addHookOnce('afterSelection', (...[,,,, preventScrolling]) => {
preventScrolling.value = true;
});

// I can't use transforms as they don't work in negative indexes.
selection.setRangeStartOnly(instance
._createCellCoords(currentFromRow + rowDelta, currentFromColumn), true);
selection.setRangeEnd(instance
._createCellCoords(currentToRow + rowDelta, currentToColumn)); // will call render() internally
} else {
instance._refreshBorders(); // it will call render and prepare methods
}
}
selection.shiftRows(instance.toVisualRow(startRowPhysicalIndex), rowDelta);
break;

case 'insert_col_start':
Expand All @@ -615,39 +584,7 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
Array.prototype.splice.apply(tableMeta.colHeaders, spliceArray); // inserts empty (undefined) elements into the colHeader array
}

const currentSelectedRange = selection.selectedRange.current();
const currentFromRange = currentSelectedRange?.from;
const currentFromColumn = currentFromRange?.col;
const startVisualColumnIndex = instance.toVisualColumn(startColumnPhysicalIndex);

if (selection.isSelectedByCorner()) {
selection.selectAll(true, true, {
disableHeadersHighlight: true,
});

} else if (isDefined(currentFromColumn) && currentFromColumn >= startVisualColumnIndex) {
// Moving the selection (if it exists) rightward – it should be applied to the "old" column.
// TODO: The logic here should be handled by selection module.
const { row: currentToRow, col: currentToColumn } = currentSelectedRange.to;
let currentFromRow = currentFromRange.row;

// Workaround: headers are not stored inside selection.
if (selection.isSelectedByColumnHeader()) {
currentFromRow = -1;
}

// Remove from the stack the last added selection as that selection below will be
// replaced by new transformed selection.
selection.getSelectedRange().pop();

// I can't use transforms as they don't work in negative indexes.
selection.setRangeStartOnly(instance
._createCellCoords(currentFromRow, currentFromColumn + colDelta), true);
selection.setRangeEnd(instance
._createCellCoords(currentToRow, currentToColumn + colDelta)); // will call render() internally
} else {
instance._refreshBorders(); // it will call render and prepare methods
}
selection.shiftColumns(instance.toVisualColumn(startColumnPhysicalIndex), colDelta);
}
break;

Expand Down Expand Up @@ -675,7 +612,26 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
return;
}

if (selection.isSelected()) {
const { row } = instance.getSelectedRangeLast().highlight;

if (row >= groupIndex && row <= groupIndex + groupAmount - 1) {
editorManager.closeEditor(true);
}
}

const totalRows = instance.countRows();

if (totalRows === 0) {
selection.deselect();

} else if (source === 'ContextMenu.removeRow') {
selection.refresh();

} else {
selection.shiftRows(groupIndex, -groupAmount);
}

const fixedRowsTop = tableMeta.fixedRowsTop;

if (fixedRowsTop >= calcIndex + 1) {
Expand All @@ -697,9 +653,6 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
} else {
removeRow([[index, amount]]);
}

grid.adjustRowsAndCols();
instance._refreshBorders(); // it will call render and prepare methods
break;

case 'remove_col':
Expand Down Expand Up @@ -727,6 +680,26 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
return;
}

if (selection.isSelected()) {
const { col } = instance.getSelectedRangeLast().highlight;

if (col >= groupIndex && col <= groupIndex + groupAmount - 1) {
editorManager.closeEditor(true);
}
}

const totalColumns = instance.countCols();

if (totalColumns === 0) {
selection.deselect();

} else if (source === 'ContextMenu.removeColumn') {
selection.refresh();

} else {
selection.shiftColumns(groupIndex, -groupAmount);
}

const fixedColumnsStart = tableMeta.fixedColumnsStart;

if (fixedColumnsStart >= calcIndex + 1) {
Expand All @@ -749,15 +722,13 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
} else {
removeCol([[index, amount]]);
}

grid.adjustRowsAndCols();
instance._refreshBorders(); // it will call render and prepare methods

break;
default:
throw new Error(`There is no such action "${action}"`);
}

instance.view.render();

if (!keepEmptyRows) {
grid.adjustRowsAndCols(); // makes sure that we did not add rows that will be removed in next refresh
}
Expand All @@ -774,10 +745,6 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
const minCols = tableMeta.minCols;
const minSpareCols = tableMeta.minSpareCols;

if (instance.countRows() === 0 && instance.countCols() === 0) {
selection.deselect();
}

if (minRows) {
// should I add empty rows to data source to meet minRows?
const nrOfRows = instance.countRows();
Expand Down Expand Up @@ -834,64 +801,6 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
}
}

if (selection.isSelected()) {
const rowCount = instance.countRows();
const colCount = instance.countCols();

arrayEach(selection.selectedRange, (range) => {
let selectionChanged = false;
let fromRow = range.from.row;
let fromCol = range.from.col;
let toRow = range.to.row;
let toCol = range.to.col;

// if selection is outside, move selection to last row
if (fromRow > rowCount - 1) {
fromRow = rowCount - 1;
selectionChanged = true;

if (toRow > fromRow) {
toRow = fromRow;
}
} else if (toRow > rowCount - 1) {
toRow = rowCount - 1;
selectionChanged = true;

if (fromRow > toRow) {
fromRow = toRow;
}
}
// if selection is outside, move selection to last row
if (fromCol > colCount - 1) {
fromCol = colCount - 1;
selectionChanged = true;

if (toCol > fromCol) {
toCol = fromCol;
}
} else if (toCol > colCount - 1) {
toCol = colCount - 1;
selectionChanged = true;

if (fromCol > toCol) {
fromCol = toCol;
}
}

if (selectionChanged) {
if (fromCol < 0) {
instance.selectRows(fromRow, toRow, fromCol);

} else if (fromRow < 0) {
instance.selectColumns(fromCol, toCol, fromRow);

} else {
instance.selectCell(fromRow, fromCol, toRow, toCol);
}
}
});
}

if (instance.view) {
instance.view.adjustElementsSize();
}
Expand Down Expand Up @@ -1422,7 +1331,9 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
instance.forceFullRender = true; // used when data was changed
grid.adjustRowsAndCols();
instance.runHooks('beforeChangeRender', changes, source);
instance._refreshBorders(null);
editorManager.closeEditor();
instance.view.render();
editorManager.prepareEditor();
instance.view.adjustElementsSize();
instance.runHooks('afterChange', changes, source || 'edit');

Expand Down Expand Up @@ -1737,7 +1648,12 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
* @param {boolean} [prepareEditorIfNeeded=true] If `true` the editor under the selected cell will be prepared to open.
*/
this.destroyEditor = function(revertOriginal = false, prepareEditorIfNeeded = true) {
instance._refreshBorders(revertOriginal, prepareEditorIfNeeded);
editorManager.closeEditor(revertOriginal);
instance.view.render();

if (prepareEditorIfNeeded && selection.isSelected()) {
editorManager.prepareEditor();
}
};

/**
Expand Down Expand Up @@ -1999,7 +1915,7 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
if (this.renderCall) {
this.render();
} else {
this._refreshBorders(null);
instance.view.render();
}
}
};
Expand All @@ -2020,9 +1936,7 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
this.forceFullRender = true; // used when data was changed

if (!this.isRenderSuspended()) {
editorManager.lockEditor();
this._refreshBorders(null);
editorManager.unlockEditor();
instance.view.render();
}
}
};
Expand Down Expand Up @@ -2299,6 +2213,7 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
instance.rowIndexMapper.fitToLength(this.countSourceRows());

grid.adjustRowsAndCols();
selection.refresh();
}, {
hotInstance: instance,
dataMap: datamap,
Expand Down Expand Up @@ -2342,6 +2257,7 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
metaManager.clearCellsCache();
instance.initIndexMappers();
grid.adjustRowsAndCols();
selection.refresh();

if (firstRun) {
firstRun = [null, 'loadData'];
Expand Down Expand Up @@ -2667,10 +2583,8 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal

if (instance.view && !firstRun) {
instance.forceFullRender = true; // used when data was changed
editorManager.lockEditor();
instance._refreshBorders(null);
instance.view.render();
instance.view._wt.wtOverlays.adjustElementsSize();
editorManager.unlockEditor();
}

if (!init && instance.view && (currentHeight === '' || height === '' || height === undefined) &&
Expand Down Expand Up @@ -4901,22 +4815,6 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
});
};

/**
* Refresh selection borders. This is temporary method relic after selection rewrite.
*
* @private
* @param {boolean} [revertOriginal=false] If `true`, the previous value will be restored. Otherwise, the edited value will be saved.
* @param {boolean} [prepareEditorIfNeeded=true] If `true` the editor under the selected cell will be prepared to open.
*/
this._refreshBorders = function(revertOriginal = false, prepareEditorIfNeeded = true) {
editorManager.destroyEditor(revertOriginal);
instance.view.render();

if (prepareEditorIfNeeded && selection.isSelected()) {
editorManager.prepareEditor();
}
};

/**
* Gets the instance of the EditorManager.
*
Expand Down