Skip to content

Commit cde7014

Browse files
rmbh-odooLucasLefevre
authored andcommitted
[IMP] selection: allow to de-select a zone
Before this commit: - Users could only add to the selection using Ctrl+Click; de-selection was not possible. - Ctrl+Click on an already selected cell had no effect. - There was no way to remove part of an existing selection. Issue Example: 1. Select range A1:C3. 2. Ctrl+Click on B2. 3. Expected: Selection splits into A1:C1, A2, C2, A3:C3. 4. Previously, Selection remained unchanged; B2 stayed selected. After this commit: - Ctrl+Click on a selected cell now removes it from the selection. - The selection is split into non-overlapping zones as needed. Technical Changes: - The commitSelection function is now triggered on mouse up to finalize zone updates. - If the active zone overlaps with existing selected zones, the overlapping part is removed by splitting the zone. closes #6125 Task: 4647187 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
1 parent a537d1f commit cde7014

File tree

12 files changed

+254
-17
lines changed

12 files changed

+254
-17
lines changed

packages/o-spreadsheet-engine/src/helpers/zones.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,3 +887,45 @@ export function splitIfAdjacent(zone: UnboundedZone, zoneToRemove: Zone): Unboun
887887
return newZones;
888888
}
889889
}
890+
891+
/**
892+
* Splits zone z2 by removing the overlapping zone z1 (fully inside z2).
893+
* Returns the remaining parts of z2 that don't overlap with z1.
894+
*
895+
* Diagram:
896+
* ┌──────────── z2 ─────────────┐
897+
* │ 1 │
898+
* │--------─────────────--------│
899+
* │ 2 | z1 | 3 │
900+
* │--------─────────────--------│
901+
* │ 4 │
902+
* └─────────────────────────────┘
903+
*
904+
* Input:
905+
* z1 = { top: 2, bottom: 3, left: 2, right: 3 }
906+
* z2 = { top: 1, bottom: 4, left: 1, right: 4 }
907+
*
908+
* Output:
909+
* [
910+
* { top: 4, bottom: 4, left: 1, right: 4 }, // bottom
911+
* { top: 2, bottom: 3, left: 4, right: 4 }, // right
912+
* { top: 2, bottom: 3, left: 1, right: 1 }, // left
913+
* { top: 1, bottom: 1, left: 1, right: 4 } // top
914+
* ]
915+
*/
916+
export function splitZone(z1: Zone, z2: Zone): Zone[] {
917+
const zones: Zone[] = [];
918+
if (z1.bottom < z2.bottom) {
919+
zones.push({ ...z2, top: z1.bottom + 1 });
920+
}
921+
if (z1.right < z2.right) {
922+
zones.push({ ...z2, left: z1.right + 1, top: z1.top, bottom: z1.bottom });
923+
}
924+
if (z1.left > z2.left) {
925+
zones.push({ ...z2, right: z1.left - 1, top: z1.top, bottom: z1.bottom });
926+
}
927+
if (z1.top > z2.top) {
928+
zones.push({ ...z2, bottom: z1.top - 1 });
929+
}
930+
return zones;
931+
}

packages/o-spreadsheet-engine/src/plugins/ui_stateful/selection.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import { getClipboardDataPositions } from "../../helpers/clipboard/clipboard_hel
44
import { clip, deepCopy, range } from "../../helpers/misc";
55
import {
66
isEqual,
7+
isZoneInside,
78
positionToZone,
9+
splitZone,
810
uniqueZones,
911
updateSelectionOnDeletion,
1012
updateSelectionOnInsertion,
@@ -118,8 +120,8 @@ export class GridSelectionPlugin extends UIPlugin {
118120
}
119121

120122
private handleEvent(event: SelectionEvent) {
121-
const anchor = event.anchor;
122-
let zones: Zone[] = [];
123+
let anchor = event.anchor;
124+
let zones: Zone[] = [...this.gridSelection.zones];
123125

124126
this.isUnbounded = event.options?.unbounded || false;
125127

@@ -128,17 +130,36 @@ export class GridSelectionPlugin extends UIPlugin {
128130
zones = [anchor.zone];
129131
break;
130132
case "updateAnchor":
131-
zones = [...this.gridSelection.zones];
132133
const index = zones.findIndex((z: Zone) => isEqual(z, event.previousAnchor.zone));
133134
if (index >= 0) {
134135
zones[index] = anchor.zone;
135136
}
136137
break;
137138
case "newAnchor":
138-
zones = [...this.gridSelection.zones, anchor.zone];
139+
zones.push(anchor.zone);
140+
break;
141+
case "commitSelection":
142+
const zoneToSplit = zones.find(
143+
(zone) => isZoneInside(anchor.zone, zone) && !isEqual(anchor.zone, zone)
144+
);
145+
const removeFullAnchor = zones.filter((zone) => isEqual(anchor.zone, zone)).length > 1;
146+
if (removeFullAnchor && zones.length > 2) {
147+
zones = zones.filter((zone) => !isEqual(zone, anchor.zone));
148+
} else if (zoneToSplit) {
149+
const splittedZones = splitZone(anchor.zone, zoneToSplit);
150+
zones = zones
151+
.filter((z) => !isEqual(z, anchor.zone) && !isEqual(z, zoneToSplit))
152+
.concat(splittedZones);
153+
}
154+
zones = uniqueZones(zones);
155+
const lastZone = zones[zones.length - 1];
156+
anchor = {
157+
cell: { col: lastZone.left, row: lastZone.top },
158+
zone: lastZone,
159+
};
139160
break;
140161
}
141-
this.setSelectionMixin(event.anchor, zones);
162+
this.setSelectionMixin(anchor, zones);
142163
/** Any change to the selection has to be reflected in the selection processor. */
143164
this.selection.resetDefaultAnchor(this, deepCopy(this.gridSelection.anchor));
144165
const { col, row } = this.gridSelection.anchor.cell;
@@ -455,7 +476,7 @@ export class GridSelectionPlugin extends UIPlugin {
455476
{ anchor, zones }
456477
);
457478
this.gridSelection.anchor = clippedAnchor;
458-
this.gridSelection.zones = uniqueZones(clippedZones);
479+
this.gridSelection.zones = clippedZones;
459480
}
460481
/**
461482
* Change the anchor of the selection active cell to an absolute col and row index.

packages/o-spreadsheet-engine/src/selection_stream/selection_stream_processor.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ export class SelectionStreamProcessorImpl implements SelectionStreamProcessor {
152152
bottom: Math.max(anchorRow, row),
153153
};
154154
const expandedZone = this.getters.expandZone(sheetId, zone);
155+
if (isEqual(this.anchor.zone, expandedZone)) {
156+
return new DispatchResult(CommandResult.NoChanges);
157+
}
155158
const anchor = { zone: expandedZone, cell: { col: anchorCol, row: anchorRow } };
156159
return this.processEvent({
157160
mode: "updateAnchor",
@@ -174,6 +177,23 @@ export class SelectionStreamProcessorImpl implements SelectionStreamProcessor {
174177
});
175178
}
176179

180+
/**
181+
* Triggered on mouse up to finalize the selection.
182+
* - If the current anchor zone already exists in the selection, it will be removed.
183+
* - If the anchor zone overlaps with existing zones, it will be split into
184+
* multiple non-overlapping parts.
185+
*/
186+
commitSelection(): DispatchResult {
187+
return this.processEvent({
188+
options: {
189+
scrollIntoView: false,
190+
unbounded: true,
191+
},
192+
anchor: this.anchor,
193+
mode: "commitSelection",
194+
});
195+
}
196+
177197
/**
178198
* Increase or decrease the size of the current anchor zone.
179199
* The anchor cell remains where it is. It's the opposite side
@@ -257,7 +277,10 @@ export class SelectionStreamProcessorImpl implements SelectionStreamProcessor {
257277
});
258278
}
259279

260-
selectColumn(index: HeaderIndex, mode: SelectionEvent["mode"]): DispatchResult {
280+
selectColumn(
281+
index: HeaderIndex,
282+
mode: Exclude<SelectionEvent["mode"], "commitSelection">
283+
): DispatchResult {
261284
const sheetId = this.getters.getActiveSheetId();
262285
const bottom = this.getters.getNumberRows(sheetId) - 1;
263286
let zone = { left: index, right: index, top: 0, bottom };
@@ -284,7 +307,10 @@ export class SelectionStreamProcessorImpl implements SelectionStreamProcessor {
284307
});
285308
}
286309

287-
selectRow(index: HeaderIndex, mode: SelectionEvent["mode"]): DispatchResult {
310+
selectRow(
311+
index: HeaderIndex,
312+
mode: Exclude<SelectionEvent["mode"], "commitSelection">
313+
): DispatchResult {
288314
const sheetId = this.getters.getActiveSheetId();
289315
const right = this.getters.getNumberCols(sheetId) - 1;
290316
let zone = { top: index, bottom: index, left: 0, right };

packages/o-spreadsheet-engine/src/types/event_stream/selection_events.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ export type SelectionEventOptions = {
88
export interface SelectionEvent {
99
anchor: AnchorZone;
1010
previousAnchor: AnchorZone;
11-
mode: "newAnchor" | "overrideSelection" | "updateAnchor";
11+
mode: "newAnchor" | "overrideSelection" | "updateAnchor" | "commitSelection";
1212
options: SelectionEventOptions;
1313
}

packages/o-spreadsheet-engine/src/types/selection_stream_processor.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ interface SelectionProcessor {
4040

4141
selectTableAroundSelection(): DispatchResult;
4242

43+
commitSelection(): DispatchResult;
44+
4345
isListening(owner: unknown): boolean;
4446
}
4547

src/components/grid/grid.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,7 @@ export class Grid extends Component<Props, SpreadsheetChildEnv> {
553553
}
554554
};
555555
const onMouseUp = () => {
556+
this.env.model.selection.commitSelection();
556557
if (this.paintFormatStore.isActive) {
557558
this.paintFormatStore.pasteFormat(this.env.model.getters.getSelectedZones());
558559
}

src/components/headers_overlay/headers_overlay.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ abstract class AbstractResizer extends Component<ResizerProps, SpreadsheetChildE
137137
}
138138

139139
_computeGrabDisplay(zoomedMouseEvent: ZoomedMouseEvent<MouseEvent>) {
140+
if (isCtrlKey(zoomedMouseEvent.ev)) {
141+
this.state.waitingForMove = false;
142+
return;
143+
}
140144
const index = this._getElementIndex(this._getEvOffset(zoomedMouseEvent));
141145
const activeElements = this._getActiveElements();
142146
const selectedZoneStart = this._getSelectedZoneStart();
@@ -239,7 +243,7 @@ abstract class AbstractResizer extends Component<ResizerProps, SpreadsheetChildE
239243
this._selectElement(index, false);
240244
return;
241245
}
242-
if (this.state.waitingForMove) {
246+
if (!isCtrlKey(ev) && this.state.waitingForMove) {
243247
if (!this.env.model.getters.isGridSelectionActive()) {
244248
this._selectElement(index, false);
245249
} else {
@@ -320,17 +324,14 @@ abstract class AbstractResizer extends Component<ResizerProps, SpreadsheetChildE
320324
}
321325
};
322326
const mouseUpSelect = () => {
327+
this.env.model.selection.commitSelection();
323328
this.state.isSelecting = false;
324329
this.lastSelectedElementIndex = null;
325330
this._computeGrabDisplay(zoomedMouseEvent);
326331
};
327332
this.dragNDropGrid.start(zoomedMouseEvent, mouseMoveSelect, mouseUpSelect);
328333
}
329334

330-
onMouseUp(ev: MouseEvent) {
331-
this.lastSelectedElementIndex = null;
332-
}
333-
334335
onContextMenu(ev: MouseEvent) {
335336
ev.preventDefault();
336337
const index = this._getElementIndex(this._getEvOffset(withZoom(this.env, ev)));

src/components/headers_overlay/headers_overlay.xml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
t-on-mouseleave="onMouseLeave"
1616
t-on-pointerdown.self.prevent="select"
1717
t-on-click="onClick"
18-
t-on-pointerup.self="onMouseUp"
1918
t-on-contextmenu.self="onContextMenu"
2019
t-att-class="{'o-grab': state.waitingForMove, 'o-dragging': state.isMoving}">
2120
<div
@@ -59,7 +58,6 @@
5958
t-on-mouseleave="onMouseLeave"
6059
t-on-pointerdown.self.prevent="select"
6160
t-on-click="onClick"
62-
t-on-pointerup.self="onMouseUp"
6361
t-on-contextmenu.self="onContextMenu"
6462
t-att-class="{'o-grab': state.waitingForMove, 'o-dragging': state.isMoving, }">
6563
<div

tests/grid/grid_component.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2280,3 +2280,48 @@ describe("Header grouping shortcuts", () => {
22802280
});
22812281
});
22822282
});
2283+
2284+
describe("Can select de-select zones", () => {
2285+
beforeEach(async () => {
2286+
({ parent, model, fixture } = await mountSpreadsheet());
2287+
});
2288+
test("Can select a zone", () => {
2289+
gridMouseEvent(model, "pointerdown", "A1");
2290+
gridMouseEvent(model, "pointermove", "C3");
2291+
gridMouseEvent(model, "pointerup", "C3");
2292+
expect(model.getters.getSelectedZones()).toEqual([toZone("A1:C3")]);
2293+
});
2294+
2295+
test("Can de-select cell from selection", () => {
2296+
gridMouseEvent(model, "pointerdown", "A1");
2297+
gridMouseEvent(model, "pointermove", "C3");
2298+
gridMouseEvent(model, "pointerup", "C3");
2299+
expect(model.getters.getSelectedZones()).toEqual([{ left: 0, right: 2, top: 0, bottom: 2 }]);
2300+
2301+
gridMouseEvent(model, "pointerdown", "B2", { ctrlKey: true });
2302+
gridMouseEvent(model, "pointerup", "B2", { ctrlKey: true });
2303+
expect(model.getters.getSelectedZones()).toEqual([
2304+
toZone("A3:C3"),
2305+
toZone("C2"),
2306+
toZone("A2"),
2307+
toZone("A1:C1"),
2308+
]);
2309+
});
2310+
2311+
test("Can select a zone and de-select a overlap zone", () => {
2312+
gridMouseEvent(model, "pointerdown", "A1");
2313+
gridMouseEvent(model, "pointermove", "D4");
2314+
gridMouseEvent(model, "pointerup", "D4");
2315+
expect(model.getters.getSelectedZones()).toEqual([{ left: 0, right: 3, top: 0, bottom: 3 }]);
2316+
2317+
gridMouseEvent(model, "pointerdown", "B2", { ctrlKey: true });
2318+
gridMouseEvent(model, "pointermove", "C3", { ctrlKey: true });
2319+
gridMouseEvent(model, "pointerup", "C3", { ctrlKey: true });
2320+
expect(model.getters.getSelectedZones()).toEqual([
2321+
toZone("A4:D4"),
2322+
toZone("D2:D3"),
2323+
toZone("A2:A3"),
2324+
toZone("A1:D1"),
2325+
]);
2326+
});
2327+
});

tests/grid/grid_overlay_component.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import {
3535
triggerMouseEvent,
3636
} from "../test_helpers/dom_helper";
3737
import { getEvaluatedCell, getSelectionAnchorCellXc } from "../test_helpers/getters_helpers";
38-
import { mountSpreadsheet, nextTick, typeInComposerGrid } from "../test_helpers/helpers";
38+
import { mountSpreadsheet, nextTick, target, typeInComposerGrid } from "../test_helpers/helpers";
3939

4040
let fixture: HTMLElement;
4141
let model: Model;
@@ -1289,3 +1289,52 @@ describe("move selected element(s)", () => {
12891289
expect(model.getters.getActiveRows()).toEqual(new Set([0, 1, 2]));
12901290
});
12911291
});
1292+
1293+
describe("Can select de-select headers", () => {
1294+
beforeEach(async () => {
1295+
const data = {
1296+
sheets: [
1297+
{
1298+
colNumber: 5,
1299+
rowNumber: 5,
1300+
},
1301+
],
1302+
};
1303+
model = new Model(data);
1304+
({ fixture, env } = await mountSpreadsheet({ model }));
1305+
});
1306+
1307+
test("cannot deselect a single column", async () => {
1308+
await selectColumn("A");
1309+
expect(model.getters.getSelectedZones()).toEqual(target("A1:A5"));
1310+
await selectColumn("A", { ctrlKey: true });
1311+
expect(model.getters.getSelectedZones()).toEqual(target("A1:A5"));
1312+
});
1313+
1314+
test("can deselect a single column when there are more than one", async () => {
1315+
await selectColumn("A");
1316+
expect(model.getters.getSelectedZones()).toEqual(target("A1:A5"));
1317+
await selectColumn("C", { ctrlKey: true });
1318+
expect(model.getters.getSelectedZones()).toEqual(target("A1:A5,C1:C5"));
1319+
await selectColumn("C", { ctrlKey: true });
1320+
expect(model.getters.getSelectedZones()).toEqual(target("A1:A5"));
1321+
});
1322+
1323+
test("Can select already selected column remove it from selection", async () => {
1324+
await selectColumn("A");
1325+
await selectColumn("D", { shiftKey: true });
1326+
await selectColumn("E", { ctrlKey: true });
1327+
expect(model.getters.getActiveCols()).toEqual(new Set([0, 1, 2, 3, 4]));
1328+
await selectColumn("C", { ctrlKey: true });
1329+
expect(model.getters.getActiveCols()).toEqual(new Set([0, 1, 3, 4]));
1330+
});
1331+
1332+
test("Can select already selected row remove it from selection", async () => {
1333+
await selectRow(0);
1334+
await selectRow(3, { shiftKey: true });
1335+
await selectRow(4, { ctrlKey: true });
1336+
expect(model.getters.getActiveRows()).toEqual(new Set([0, 1, 2, 3, 4]));
1337+
await selectRow(2, { ctrlKey: true });
1338+
expect(model.getters.getActiveRows()).toEqual(new Set([0, 1, 3, 4]));
1339+
});
1340+
});

0 commit comments

Comments
 (0)