#3005 - Unable to 'undo' the paste action, if there is no reset to Select tool. #3137
Conversation
nanoblit
force-pushed
the
3005-unable-to-undo-the-paste-action-if-there-is-no-reset-to-select-tool
branch
from
bbe6698 to
b90ed60
Compare
auto-assign
bot
requested review from
AnastasiyaPiatrovaKlu,
aproskurnov and
Nitvex
| @@ -535,9 +536,8 @@ class Editor implements KetcherEditor { | |||
|
|
|||
| this.selection(null); | |||
|
|
|||
| if (this._tool instanceof toolsMap.paste) { | |||
| this.event.change.dispatch(); | |||
There was a problem hiding this comment.
My proposal is not to remove the change event dispatch. FYI, it, most likely, doesn't affect our app code, but there can be subscribers in external application and with this we can break their apps
There was a problem hiding this comment.
All right, I'll add it back. Thanks for the head up. :)
| return; | ||
|
|
||
| if (this._tool instanceof PasteTool) { | ||
| this._tool.updatePreview(); |
There was a problem hiding this comment.
could we at least unify logic, for ex. move updatePreview as empty method to Tool class and invoke this._tool.updatePreview() for all tools (remove if condition)? (implementation still be in PasteTool class)
There was a problem hiding this comment.
I did it like this, because I thought it was very specific to the paste tool, but your point makes sense too, I will change it.
…able-to-undo-the-paste-action-if-there-is-no-reset-to-select-tool
| if (this._tool instanceof toolsMap.paste) { | ||
| this.event.change.dispatch(); | ||
| return; |
There was a problem hiding this comment.
are you sure that it will not broke anything?
return; no longer needed here?
There was a problem hiding this comment.
You are right. Changing that fixes the issue, but causes other edge cases. I need to do this differently.
…able-to-undo-the-paste-action-if-there-is-no-reset-to-select-tool
…able-to-undo-the-paste-action-if-there-is-no-reset-to-select-tool
…able-to-undo-the-paste-action-if-there-is-no-reset-to-select-tool
…able-to-undo-the-paste-action-if-there-is-no-reset-to-select-tool
|
All tests passing. The failed test is passing locally: |
|
Fixed in this PR - #4161 |
How the feature works? / How did you fix the issue?
(Screenshots, videos, or GIFs, if applicable)
Fix undoing paste when reset to select tool setting is off
Check list
#1234 – issue name