From fc7a27171ddf07c0d8e55d6b080c8766738468d6 Mon Sep 17 00:00:00 2001 From: Alex Ma Date: Thu, 9 Jun 2022 10:51:03 -0700 Subject: [PATCH] Fix for editData tab switch back when adding new row. (#19634) * added filter for AsyncDataProvider Filter crash * fixed cell switch back for editdata * Added new setData check * disable selecting of all columns when new row is being added * Changed comment * removed try catch check due to Aasim's PR * added previous dataset to handlechange * added feedback changes * added fix for infinite loop * changes made as suggested by Alan * following Charles' feedback * removed onGridRendered as its pointless jumping --- .../editData/browser/editDataGridPanel.ts | 66 +++++++++++-------- src/typings/slickgrid.d.ts | 3 +- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts b/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts index ff17c9c23d..28c7ab60d6 100644 --- a/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts +++ b/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts @@ -61,6 +61,7 @@ export class EditDataGridPanel extends GridParentComponent { private enableEditing = true; // Current selected cell state private currentCell: { row: number, column: number, isEditable: boolean, isDirty: boolean }; + private lastClickedCell: { row: number, column: number }; private currentEditCellValue: string; private newRowVisible: boolean; private removingNewRow: boolean; @@ -78,7 +79,6 @@ export class EditDataGridPanel extends GridParentComponent { public overrideCellFn: (rowNumber, columnId, value?, data?) => string; public loadDataFunction: (offset: number, count: number) => Promise<{}[]>; public onBeforeAppendCell: (row: number, column: number) => string; - public onGridRendered: (event: Slick.OnRenderedEventArgs) => void; public onRefreshComplete: Promise; private savedViewState: { @@ -217,14 +217,6 @@ export class EditDataGridPanel extends GridParentComponent { return cellClass; }; - this.onGridRendered = (args: Slick.OnRenderedEventArgs): void => { - // After rendering move the focus back to the previous active cell - if (this.currentCell.column !== undefined && this.currentCell.row !== undefined - && this.isCellOnScreen(this.currentCell.row, this.currentCell.column)) { - this.focusCell(this.currentCell.row, this.currentCell.column, false); - } - }; - // Setup a function for generating a promise to lookup result subsets this.loadDataFunction = (offset: number, count: number): Promise<{}[]> => { return self.dataService.getEditRows(offset, count).then(result => { @@ -303,11 +295,17 @@ export class EditDataGridPanel extends GridParentComponent { return; } + // get the cell we have just immediately clicked (to set as the new active cell in handleChanges). + this.lastClickedCell = { row, column }; + // Skip processing if the cell hasn't moved (eg, we reset focus to the previous cell after a failed update) if (this.currentCell.row === row && this.currentCell.column === column && this.currentCell.isDirty === false) { return; } + // disable editing the grid temporarily as any text entered while the grid is being refreshed will be lost upon completion. + this.updateEnabledState(false); + let cellSelectTasks: Promise = this.submitCurrentCellChange( (result: EditUpdateCellResult) => { // Cell update was successful, update the flags @@ -317,6 +315,7 @@ export class EditDataGridPanel extends GridParentComponent { }, (error) => { // Cell update failed, jump back to the last cell we were on + this.updateEnabledState(true); self.focusCell(self.currentCell.row, self.currentCell.column, true); return Promise.reject(null); }); @@ -332,6 +331,7 @@ export class EditDataGridPanel extends GridParentComponent { return Promise.resolve(); }, error => { // Committing failed, jump back to the last selected cell + this.updateEnabledState(true); self.focusCell(self.currentCell.row, self.currentCell.column); return Promise.reject(null); }); @@ -340,11 +340,27 @@ export class EditDataGridPanel extends GridParentComponent { // At the end of a successful cell select, update the currently selected cell cellSelectTasks = cellSelectTasks.then(() => { + this.updateEnabledState(true); self.setCurrentCell(row, column); + self.focusCell(row, column); }); // Cap off any failed promises, since they'll be handled - cellSelectTasks.catch(() => { }); + cellSelectTasks.catch(() => { + }); + } + + /** + * Disables editing the grid temporarily when clicking on a cell (to allow for any processing tasks to be finished first such as adding a new row). + * @param state The variable telling whether to enable selection of the table cells or not. + */ + private updateEnabledState(state: boolean): void { + let newOptions = this.table.grid.getOptions(); + newOptions.editable = state; + // Need to suppress rerendering to avoid infinite loop when changing new row. + // When setOptions is called with rerendering, it triggers an onCellSelect in our code (which is by design currently), + // and thus an infinite loop is caused. + this.table.grid.setOptions(newOptions, true); } handleComplete(self: EditDataGridPanel, event: any): void { @@ -497,7 +513,6 @@ export class EditDataGridPanel extends GridParentComponent { this.firstLoad = false; } else { - this.table.setData(this.gridDataProvider); this.handleChanges({ ['dataRows']: { currentValue: this.dataSet.dataRows, firstChange: this.firstLoad, previousValue: this.oldDataRows } @@ -537,7 +552,7 @@ export class EditDataGridPanel extends GridParentComponent { this.renderedDataSets = tempRenderedDataSets; this.handleChanges({ ['dataRows']: { currentValue: this.renderedDataSets[0].dataRows, firstChange: this.firstLoad, previousValue: undefined }, - ['columnDefinitions']: { currentValue: this.renderedDataSets[0].columnDefinitions, firstChange: this.firstLoad, previousValue: undefined } + ['columnDefinitions']: { currentValue: this.renderedDataSets[0].columnDefinitions, firstChange: this.firstLoad, previousValue: this.dataSet.columnDefinitions } }); } @@ -791,16 +806,6 @@ export class EditDataGridPanel extends GridParentComponent { return this.currentCell.row === row && this.dirtyCells.indexOf(column) !== -1; } - private isCellOnScreen(row: number, column: number): boolean { - let slick: any = this.table; - let grid = slick._grid; - let viewport = grid.getViewport(); - let cellBox = grid.getCellNodeBox(row, column); - return viewport && cellBox - && viewport.leftPx <= cellBox.left && viewport.rightPx >= cellBox.right - && viewport.top < row + 1 && viewport.bottom > row + 1; - } - private resetCurrentCell() { this.currentCell = { row: undefined, @@ -962,10 +967,22 @@ export class EditDataGridPanel extends GridParentComponent { handleChanges(changes: { [propName: string]: any }): void { let columnDefinitionChanges = changes['columnDefinitions']; - let activeCell = this.table ? this.table.grid.getActiveCell() : undefined; + let activeCell: Slick.Cell | undefined = undefined; let hasGridStructureChanges = false; let wasEditing = this.table ? !!this.table.grid.getCellEditor() : false; + if (this.table) { + // Get the active cell we have just clicked to be the new active cell (cell needs to be manually set as active in slickgrid). + if (this.lastClickedCell) { + activeCell = { row: this.lastClickedCell.row, cell: this.lastClickedCell.column }; + this.lastClickedCell = undefined; + } + else { + // Get the last selected cell as the active cell as a backup. + activeCell = this.table.grid.getActiveCell(); + } + } + if (columnDefinitionChanges && !equals(columnDefinitionChanges.previousValue, columnDefinitionChanges.currentValue)) { if (!this.table) { this.createNewTable(); @@ -1065,9 +1082,6 @@ export class EditDataGridPanel extends GridParentComponent { // Since we need to return a string here, we are using calling a function instead of event emitter like other events handlers return this.onBeforeAppendCell ? this.onBeforeAppendCell(args.row, args.cell) : undefined; }); - this.table.grid.onRendered.subscribe((e, args) => { - this.onGridRendered(args); - }); } onBeforeEditCell(event: Slick.OnBeforeEditCellEventArgs): void { diff --git a/src/typings/slickgrid.d.ts b/src/typings/slickgrid.d.ts index 87f1a0aac9..627bdd1a3f 100644 --- a/src/typings/slickgrid.d.ts +++ b/src/typings/slickgrid.d.ts @@ -876,8 +876,9 @@ declare namespace Slick { /** * Extends grid options with a given hash. If an there is an active edit, the grid will attempt to commit the changes and only continue if the attempt succeeds. * @options An object with configuration options. + * @suppressRender A boolean telling setOptions to not rerender the grid upon options being changed. **/ - public setOptions(options: GridOptions): void; + public setOptions(options: GridOptions, suppressRender?: boolean): void; /** * Accepts an array of row indices and applies the current selectedCellCssClass to the cells in the row, respecting whether cells have been flagged as selectable.