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

[FW][FIX] edition: do not reset selection when range selecting on differe… #1372

Conversation

fw-bot
Copy link
Collaborator

@fw-bot fw-bot commented May 20, 2022

…nt sheet

When starting a ranger selection in the composer, we would
systematically reset the selection anchor to the cell that we were
editing in the first place. This behaviour is required when a user edits
a formula and wants to select some ranges by moving the position with
their keyboard.

e.g. User wants to sum several range together -
they start editing cell A1
select a first cell anywhere on the grid,
input a comma to add another zone,
hit ArrowDown,
it should select A2.

Unfortunately, when selecting a range on another sheet, this behaviour
does not make sense anymore since there is no guarantee that the anchor
coordinates of the edited cell existes on another sheet.

e.g. Edit on A1000 sheet 1 with 1000 rows, then move to sheet 2 with 10
rows, When selecting several ranges for a formula, you cannot be 'moved'
to A1000 since it doesn't exist.

To solve this, we choose to prevent the reset of the anchor when the
current sheet differs from the sheet of the edited cell.

task 2859417

Description:

description of this task, what is implemented and why it is implemented that way.

Odoo task ID : TASK_ID

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 (_lt("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

Forward-Port-Of: #1369

@robodoo
Copy link
Collaborator

robodoo commented May 20, 2022

@fw-bot
Copy link
Collaborator Author

fw-bot commented May 20, 2022

Ping @rrahir, @pro-odoo cherrypicking of pull request #1369 failed.

stderr:

11:08:42.105882 git.c:344               trace: built-in: git cherry-pick dd20f76bbbcb648cd368db96e1f926f0139cc14d
error: could not apply dd20f76b... [FIX] edition: do not reset selection when range selecting on different sheet
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

@rrahir rrahir force-pushed the saas-15.3-15.0-fix-cell-reset-on-composition-rar-kRVW-fw branch from 748d5df to f8a7d37 Compare May 20, 2022 10:18
src/selection_stream/selection_stream_processor.ts Outdated Show resolved Hide resolved
src/plugins/ui/edition.ts Outdated Show resolved Hide resolved
…nt sheet

When starting a ranger selection in the composer, we would
systematically reset the selection anchor to the cell that we were
editing in the first place. This behaviour is required when a user edits
a formula and wants to select some ranges by moving the position with
their keyboard.

e.g. User wants to sum several range together -
they start editing  cell A1
select a first cell anywhere on the grid,
input a comma to add another zone,
hit ArrowDown,
it should select A2.

Unfortunately, when selecting a range on another sheet, this behaviour
does not make sense anymore since there is no guarantee that the anchor
coordinates of the edited cell existes on another sheet.

e.g. Edit on A1000  sheet 1 with 1000 rows, then move to sheet 2 with 10
rows, When selecting several ranges for a formula, you cannot be 'moved'
to A1000 since it doesn't exist.

To solve this, we choose to prevent the reset of the anchor when the
current sheet differs from the sheet of the edited cell.

task 2859417

X-original-commit: cdd61a0
@rrahir rrahir force-pushed the saas-15.3-15.0-fix-cell-reset-on-composition-rar-kRVW-fw branch from f8a7d37 to 83fe0d6 Compare May 20, 2022 12:09
@rrahir
Copy link
Collaborator

rrahir commented May 20, 2022

@robodoo r+

robodoo pushed a commit that referenced this pull request May 20, 2022
…nt sheet

When starting a ranger selection in the composer, we would
systematically reset the selection anchor to the cell that we were
editing in the first place. This behaviour is required when a user edits
a formula and wants to select some ranges by moving the position with
their keyboard.

e.g. User wants to sum several range together -
they start editing  cell A1
select a first cell anywhere on the grid,
input a comma to add another zone,
hit ArrowDown,
it should select A2.

Unfortunately, when selecting a range on another sheet, this behaviour
does not make sense anymore since there is no guarantee that the anchor
coordinates of the edited cell existes on another sheet.

e.g. Edit on A1000  sheet 1 with 1000 rows, then move to sheet 2 with 10
rows, When selecting several ranges for a formula, you cannot be 'moved'
to A1000 since it doesn't exist.

To solve this, we choose to prevent the reset of the anchor when the
current sheet differs from the sheet of the edited cell.

task 2859417

closes #1372

X-original-commit: cdd61a0
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
@robodoo robodoo closed this May 20, 2022
@robodoo robodoo temporarily deployed to merge May 20, 2022 12:18 Inactive
@fw-bot fw-bot deleted the saas-15.3-15.0-fix-cell-reset-on-composition-rar-kRVW-fw branch June 3, 2022 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants