[FIX] selection: allow to de-select a zone

[rmbh-odoo]

Description:

Before this pr:

• Ctrl+Click on a selected cell had no effect.
• Users could add to the selection, but couldn't remove cells or zones.

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, Ctrl+Click did not deselect B2.

After this pr:

• Ctrl+Click on a selected cell now removes it.
• The selection splits into separate zones as needed.

Technical Changes:

• UpdateSelection is now triggered on mouseup, allowing precise detection of Ctrl+Click behavior.
• The selection logic removes the zone from any existing selection zones.
• Splits overlapping zones into non-overlapping parts.

Task: 4647187

review checklist

• feature is organized in plugin, or UI components
• support of duplicate sheet (deep copy)
• in model/core: ranges are Range object, and can be adapted (adaptRanges)
• in model/UI: ranges are strings (to show the user)
• undo-able commands (uses this.history.update)
• multiuser-able commands (has inverse commands and transformations where needed)
• new/updated/removed commands are documented
• exportable in excel
• translations (_t("qmsdf %s", abc))
• unit tested
• clean commented code
• track breaking changes
• doc is rebuild (npm run doc)
• status is correct in Odoo

[robodoo]

[rrahir]

we already have an algorithm that should theoretically take care of this. Did you have a look at. recomputeZones?

[rmbh-odoo]

I tried using recomputeZones, but IMO it doesn’t fit our use case. It splits zones vertically, while GSheet and our task spec require horizontal splits.
Additionally, it depends on zoneToRemove, which may not be relevant here since we’d still need to identify the overlapping zone first.
It also reorders zones internally, which makes it harder to set the anchor.zone at the end. For de-selection, we only need to split the overlapping zone.

[rrahir]

the new handler is not clear, it has no payload, I don't have a single clue of what we want it to do.

• Could you give me some details on why was it introduced in the first place?
• What limitations did you meet with the existing methods?
• Consider that the selection processor is modeled such that we can declare clearly what the method should do. UpdateSelection, espectially with no payload,... i'm expecting it to do exactly nothing.

[rmbh-odoo]

updateSelection was added to handle de-selection on mouse up, which isn't covered by existing methods like overrideSelection, newAnchor, or updateAnchor, as those are used during click and drag.
De-selection needs to check for overlap between anchor.zone and gridSelection.zones only when the mouse is released.
This method has no payload because it's solely meant to check and update gridSelection.zones.
Open to better approaches if you have suggestions.

[rrahir]

Thanks for the explanation :) I think it will definitely deserve an extended explanation on the method docstring with this kind of rationale. I don't have a better idea about the naming right now but I don't really feel confortable with a selectionEvent that will transport useless information that we never use (like the previousAnchor and anchor). Would it be possible to consider using previousAnchor and anchor more extensively (rather than relying on zones.at(-1) for instance?

[rrahir]

both the call in this file and HeaderOverlay seem untested.

[rrahir]

so you make a change on the fly and use the "updateSelection" after a previous event to 'fix' things?

[rmbh-odoo]

In the current flow, all events directly update gridSelection.zones on mouse down and drag. updateSelection is triggered after the mouse up event to correct the selection by checking if the zone already exists or overlaps.

In GSheet, zones are updated only on mouse up to avoid redundant updates during drag. On mouse up, updateSelection compares anchor.zone with existing gridSelection.zones: if present, it removes it; if overlapping, it splits; otherwise, it adds the new zone.
Let me know which approach we should go with.