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

Unable to 'undo' the paste action, if there is no reset to Select tool. #3005

Closed
Vehemen opened this issue Jul 31, 2023 · 9 comments · Fixed by #4161
Closed

Unable to 'undo' the paste action, if there is no reset to Select tool. #3005

Vehemen opened this issue Jul 31, 2023 · 9 comments · Fixed by #4161
Assignees
Labels

Comments

@Vehemen
Copy link
Collaborator

Vehemen commented Jul 31, 2023

Steps to Reproduce

  1. open settings in the up-right corner
  2. in the dropdown list for "reset to Select tool" choose "Off"
  3. place a Benzene ring on the canvas
  4. Use select tool to choose and CTRL+C placed ring.
  5. Press CTRL+V and place the ring. There are now 2 rings on the canvas.
  6. press CTRL+Z

Actual behavior
Preview of the third ring is vanished. It will return, if you move the cursor. You can only delete these previews, but you will never delete the second ring with undo.

2023-07-31_17h03_27.mp4

Expected behavior
Second ring is deleted with the first press of undo.
Screenshots

  • OS: Windows10
  • Browser: Firefox
  • Ketcher: v2.13.0 rc2
@nanoblit
Copy link
Collaborator

nanoblit commented Aug 9, 2023

Done in scope of #3050

@Vehemen
Copy link
Collaborator Author

Vehemen commented Aug 14, 2023

Done in scope of #3050

I see there is no reset to paste action in your video.
It looks like that for me:
https://github.com/epam/ketcher/assets/131955641/b696a4f1-aa40-436c-9dff-7da28f88afcf
Problem is that pressing paste shows a demo of an object you are pasting, and this action is a separate history step. Pressing CTRL+V and putting on the screen - are two history objects, and if you switch off turning to select tool, after past you will enter an infinite loop, when preshow is activated just the moment after you press CTRL+Z
Ctrl+v -> preshow -> ctrl+z -> preshow.
@nanoblit
@Zhirnoff

@Zhirnoff
Copy link
Collaborator

Zhirnoff commented Sep 5, 2023

Pre-tested. By repeatedly pressing Undo/Redo, the structures move beyond the canvas. Should remain in same place.

2023-09-05_15h09_00.mp4

@nanoblit
Copy link
Collaborator

nanoblit commented Sep 5, 2023

Pre-tested. By repeatedly pressing Undo/Redo, the structures move beyond the canvas. Should remain in same place.

2023-09-05_15h09_00.mp4

Should be fixed now

@Zhirnoff
Copy link
Collaborator

Zhirnoff commented Sep 6, 2023

Pre-tested. Fixed.

@MartaWilliams
Copy link
Collaborator

Test cases written

@ilya-asiyuk-epam
Copy link
Contributor

Removed flag "Firefox only" as I reproduce it in Google Chrome as well

@ilya-asiyuk-epam
Copy link
Contributor

Found exact commit which adds such behavior:
3b34bc2

Namely this lines:

        if (this._tool instanceof toolMap['paste']) {
		this.event.change.dispatch();
		return;
	}

ilya-asiyuk-epam added a commit that referenced this issue Feb 27, 2024
…on-if-there-is-no-reset-to-select-tool-2

#3005 - Unable to 'undo' the paste action, if there is no reset to Select tool
@Zhirnoff
Copy link
Collaborator

Tested. Bug is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment