[FIX] FigureComponent: Stop Ctrl+A propagation #4091
Conversation
dhrp-odoo
force-pushed
the
16.0-fix-only-propagate-ctrl-y-z-figure-component-dhrp
branch
5 times, most recently
from
65184a8
to
86a50d8
Compare
| break; | ||
| case "Ctrl+Y": | ||
| case "Ctrl+Z": | ||
| // Allow the event to bubble to the grid only if Ctrl+Y or Ctrl+Z is pressed |
There was a problem hiding this comment.
since #1803 we try an approach where every single component is responsible of their own action and do not rely on their parent potential reaction.
I would rather reimplement the call to REQUEST_UNDO/REDO (or maybe have a function defined in the spreadsheet env ) and call it explicitely
There was a problem hiding this comment.
@rrahir, I believe I fixed this issue similarly in the initial PR #3678. However, I recall @hokolomopo advised me to allow events to propagate up the grid. Here is the discussion: Link. Perhaps, I will explore the option of reverting back to the previous implementation.
There was a problem hiding this comment.
I wrote a message on the PR and will try to bring this discussion up on monday with the whole team present ;-) Your approach is not worse than it was before but I fear we took some liberties that we shouldn't have and now is as good time as any to correct things. Let's keep this PR on hold until we discuss this so you avoid doing the same work twice.
There was a problem hiding this comment.
We still haven't decided exactly how to handle things but I would like to follow this commit idea : 3660bf2#diff-91190cae1bee7c5780c36f202f34ede615a061931ff957bf339ba10eee08f9ee which was to simply handle what you should leave the other behaviours alone. This means that you should not stop the propagation by default but only if you handle the shortcut.
There was a problem hiding this comment.
@rrahir, Changes made!
This function is used to convert a KeyboardEvent to a string that represents the shortcut that was pressed. Task: 3863300
Previously, when a figure was selected, And pressing CTRL + A would select all rows and columns in the sheet, and pressing the Delete key would delete the selected figure. This occurred because all key events were bubbling up to the grid component. This commit stops the propagation for the Ctrl+A key event in the figure component. Task: 3863300
dhrp-odoo
changed the title
dhrp-odoo
force-pushed
the
16.0-fix-only-propagate-ctrl-y-z-figure-component-dhrp
branch
from
86a50d8
to
929c67e
Compare
| @@ -212,6 +214,21 @@ export class FigureComponent extends Component<Props, SpreadsheetChildEnv> { | |||
| ev.preventDefault(); | |||
| ev.stopPropagation(); | |||
| break; | |||
| case "Ctrl+A": | |||
| // Maybe in the future we will implement a way to select all figures | |||
|
@robodoo rebase-ff r+ @dhrp-odoo Thanks a lot ! :) |
This function is used to convert a KeyboardEvent to a string that represents the shortcut that was pressed. Task: 3863300 Part-of: #4091
Previously, when a figure was selected, And pressing CTRL + A would select all rows and columns in the sheet, and pressing the Delete key would delete the selected figure. This occurred because all key events were bubbling up to the grid component. This commit stops the propagation for the Ctrl+A key event in the figure component. closes #4091 Task: 3863300 Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
|
Merge method set to rebase and fast-forward. |
Description:
Previously, when a figure was selected, And pressing CTRL + A would select all rows and columns in the sheet, and pressing the Delete key would delete the selected figure. This occurred because all key events were bubbling up to the grid component.
This PR stops the propagation for the Ctrl+A key event in the figure component.
Task: : 3863300
review checklist