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

Editor unresponsive after undoing crop #519

Closed
fjwong opened this issue Feb 3, 2021 · 7 comments
Closed

Editor unresponsive after undoing crop #519

fjwong opened this issue Feb 3, 2021 · 7 comments
Labels

Comments

@fjwong
Copy link

fjwong commented Feb 3, 2021

Describe the bug
The image editor is mostly unresponsive after cropping an image and then undoing twice.

This only happens if the cropping rectangle is resized at least once before cropping.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://ui.toast.com/tui-image-editor (Or https://nhn.github.io/tui.image-editor/latest/tutorial-example02-useApiDirect)
  2. Switch to crop mode
  3. Make a cropping rectangle, by either making a custom rectangle or selecting one of the fixed ones.
  4. Resize the cropping rectangle
    • At this point, you should see the 'Undo' button enable. I assume this event gets pushed into the undo stack.
    • Clicking 'Undo' here triggers the error as well.
  5. Crop the image
  6. Click 'Undo'. The image returns to original dimensions.
  7. Click 'Undo' again.
    • Javascript console throws an Uncaught TypeError: Cannot read property 'set' of undefined exception.
  8. Image editor is mostly unresponsive afterwards:
    • Most commands fail with an Uncaught (in promise) The executing command state is locked. exception.
    • Free drawing seems to work though, but changes cannot be undone.

Expected behavior

  • Image editor works after undoing crop.
  • 'Undo' stack is not changed by resizing cropping rectangles.

Desktop (please complete the following information):

  • OS: Mac OS
  • Browser: Chrome
  • Version: 88.0.4324.96 (Official Build) (x86_64)
@lja1018
Copy link
Contributor

lja1018 commented Feb 10, 2021

@fjwong
I understand this issue. Thank you for your detailed report. I'll handle it.

@ayappaP
Copy link

ayappaP commented Feb 25, 2021

@lja1018 Running into the same problem that @fjwong described.

When can we expect a fix for this? Would really appreciate it!! Thanks!!

@lja1018
Copy link
Contributor

lja1018 commented Feb 26, 2021

@ayappaP
It will be fixed soon. Maybe next week.

@fjwong
Copy link
Author

fjwong commented Feb 26, 2021

@lja1018 Thank you for looking into the matter. We are also hoping this can be fixed soon. 😃

At the moment, we've implemented a somewhat hacky workaround that seems to work for our case.
@ayappaP if you are in need of addressing this quickly, perhaps this could be of help to you:

const editorInstance = ... // Tui Image Editor instance
const graphics = editorInstance._graphics;
const canvas = graphics._canvas;
const { onObjectModified } = graphics._handler;

// Detach default 'object:modified' listener and replace it with our own
canvas.off('object:modified', onObjectModified);
canvas.on('object:modified', (event) => {
  const { target } = event;
  // Ignore all 'object:modified' events triggered by a cropzone.
  if (target.type === 'cropzone') return;
  // Use the original event handler otherwise.
  onObjectModified(event);
});

@ayappaP
Copy link

ayappaP commented Feb 26, 2021

Thanks a lot @fjwong !!

I will try this.

@ayappaP
Copy link

ayappaP commented Mar 1, 2021

@lja1018 Thank you for looking into the matter. We are also hoping this can be fixed soon. 😃

At the moment, we've implemented a somewhat hacky workaround that seems to work for our case.
@ayappaP if you are in need of addressing this quickly, perhaps this could be of help to you:

const editorInstance = ... // Tui Image Editor instance
const graphics = editorInstance._graphics;
const canvas = graphics._canvas;
const { onObjectModified } = graphics._handler;

// Detach default 'object:modified' listener and replace it with our own
canvas.off('object:modified', onObjectModified);
canvas.on('object:modified', (event) => {
  const { target } = event;
  // Ignore all 'object:modified' events triggered by a cropzone.
  if (target.type === 'cropzone') return;
  // Use the original event handler otherwise.
  onObjectModified(event);
});

This worked for me. Thanks for your help @fjwong !!

lja1018 added a commit that referenced this issue Mar 5, 2021
# Conflicts:
#	apps/image-editor/src/js/imageEditor.js
lja1018 added a commit that referenced this issue Mar 5, 2021
* fix: not fire object modified event when cropzone is modified [#519]

# Conflicts:
#	apps/image-editor/src/js/imageEditor.js

* refactor: apply const

# Conflicts:
#	apps/image-editor/src/js/imageEditor.js
lja1018 added a commit that referenced this issue Mar 5, 2021
* feat: add history feature (#544)

* feat: add history element

* feat: add history list element, event listener

* feat: add history, panel menu component

* test: add history component test

* chore: remove default history

* refactor: constant classname

* feat: add history by action

* feat: add multiUndo, multiRedo

* chore: apply api review

* chore: apply detail history

* chore: add execution command event

* chore: add mask history

* chore: select load image history when init history

* chore: apply code review

* fix: list scroll position is bottom when list element added

* feat: add help toolbar

* chore: add help menus const

* chore: add history icon svg file

* chore: add toggle history menu

* chore: add list, panel, item style

* chore: add history action icon

* chore: change history title type

* chore: add history template html

* chore: resolve detail history information

* chore: resolve tooltip position with helpmenubar position

* test: fix broken test

* chore: add 3 history icons

* refactor: apply history name constants

* chore: apply alias

* chore: not toggle when history menu clicked

* fix: save undo data when text added (#545)

* fix: group selection object position when undo, redo in API direct page (#522) (#546)

* fix: fire object modified event when cropzone changed [#519] (#547)

* fix: not fire object modified event when cropzone is modified [#519]

# Conflicts:
#	apps/image-editor/src/js/imageEditor.js

* refactor: apply const

# Conflicts:
#	apps/image-editor/src/js/imageEditor.js

* fix: resize cropzone with ratio by pressing shift key (#548)

* fix: save undo data when text added [#545]

* test: resolve broken tests
@lja1018
Copy link
Contributor

lja1018 commented Mar 10, 2021

@fjwong @ayappaP
Thanks for the report. Resolved in v3.13.0.

@lja1018 lja1018 closed this as completed Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants