Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) Fix cell selection bugs

Browse Source 
Summary:
Cell selection would sometimes get out of sync, causing
unexpected results when pasting. The UI would also incorrectly
indicate that rows/columns were still selected if you clicked the
selected cell (outlined in green) after doing a drag selection of
multiple rows/columns. Finally, canceling a copy operation would
fail to remove the "scissors" outline  around the copied cells if the
cursor was not on the copied selection.

This resolves all of these bugs.

Test Plan: Browser tests.

Reviewers: cyprien

Reviewed By: cyprien

Subscribers: cyprien

Differential Revision: https://phab.getgrist.com/D3517
This commit is contained in:
George Gevoian 2022-07-09 22:20:52 -07:00
parent 77775401fc
commit c0852761b9
1 changed files with 17 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
app/client/components/GridView.js
View File

@ -234,7 +234,8 @@ function GridView(gristDoc, viewSectionModel, isPreview = false) {
this.autoDispose(commands.createGroup(GridView.gridCommands, this, this.viewSection.hasFocus)); this.autoDispose(commands.createGroup(GridView.gridCommands, this, this.viewSection.hasFocus));
// Cancel command is registered conditionally, only when there is an active // Cancel command is registered conditionally, only when there is an active
// cell selection. This command is also used by Raw Data Views, to close the Grid popup. // cell selection. This command is also used by Raw Data Views, to close the Grid popup.
const hasSelection = this.autoDispose(ko.pureComputed(() => !this.cellSelector.isCurrentSelectType(''))); const hasSelection = this.autoDispose(ko.pureComputed(() =>
!this.cellSelector.isCurrentSelectType('') || this.copySelection()));
this.autoDispose(commands.createGroup(GridView.selectionCommands, this, hasSelection)); this.autoDispose(commands.createGroup(GridView.selectionCommands, this, hasSelection));
// Timer to allow short, otherwise non-actionable clicks on column names to trigger renaming. // Timer to allow short, otherwise non-actionable clicks on column names to trigger renaming.
@ -614,8 +615,21 @@ GridView.prototype.assignCursor = function(elem, elemType) {
console.error("GridView.assignCursor expects a row/col header, or cell as an input."); console.error("GridView.assignCursor expects a row/col header, or cell as an input.");
} }
/* CellSelector already updates the selection whenever rowIndex/fieldIndex is changed, but
this.cellSelector.currentSelectType(elemType); * since those observables don't currently notify subscribers when an unchanged value is
* written, there are cases where the selection doesn't get updated. For example, when doing
* a click and drag to select cells and then clicking the "selected" cell that's outlined in
* green, the row/column numbers remain highlighted as if they are still selected, while
* GridView indicates the cells are not selected. This causes bugs that range from the
* aformentioned visual discrepancy to incorrect copy/paste behavior due to out-of-date
* selection ranges.
*
* We address this by calling setToCursor here unconditionally, but another possible approach
* might be to extend rowIndex/fieldIndex to always notify their subscribers. Always notifying
* currently introduces some bugs, and we'd also need to check that it doesn't cause too
* much unnecessary UI recomputation elsewhere, so in the interest of time we use the first
* approach. */
this.cellSelector.setToCursor(elemType);
}; };
/** /**