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

@uppy/image-editor: Add image editor cancel event #3875

Merged
merged 6 commits into from Jul 12, 2022

Conversation

jamestiotio
Copy link
Contributor

@jamestiotio jamestiotio commented Jul 11, 2022

This PR adds a cancel event to the @uppy/image-editor package. This should close #3846.

Do let me know if I missed anything or if any further changes are required.

Thank you!

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. We should document this in image-editor.md before this can be merged.

@@ -121,6 +121,16 @@ uppy.on('file-editor:complete', (updatedFile) => {
})
```

### file-editor:cancel

Emitted when `uninstall` is called.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also call this event when you are editing, but discard the changes?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to use this with uppy dashboard and it does not fire when changes are discarded. I might have understood it wrong but this event does not get triggered at all @Murderlon.

This is what I'm trying to test with

    uppy.on("file-editor:cancel", (e) => {
      console.log("CANCEL")
    })

and hitting this button is when I'm expecting it to get called
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you on the latest version?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, I'm on 2.13.4 @Murderlon

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update to latest 2.x (or 3.x) and try again?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.3.3 is the latest which I'm on. Are you able to repro this when using the image editor with dashboard @Murderlon?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was shipped in 2.13.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a new issue: #4045

@jamestiotio
Copy link
Contributor Author

Hmm... I am not sure if we should emit the cancel event when the Revert button is pressed as well or not.

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can test this final nit change then we're good to merge :)

packages/@uppy/dashboard/src/components/FileCard/index.jsx Outdated Show resolved Hide resolved
@Murderlon
Copy link
Member

Hmm... I am not sure if we should emit the cancel event when the Revert button is pressed as well or not.

No I don't think so

@jamestiotio
Copy link
Contributor Author

Hmm... I am not sure if we should emit the cancel event when the Revert button is pressed as well or not.

No I don't think so

Alright, got it!

@Murderlon Murderlon merged commit 329c1d0 into transloadit:main Jul 12, 2022
@Murderlon
Copy link
Member

Thank you!

@github-actions github-actions bot mentioned this pull request Jul 18, 2022
github-actions bot added a commit that referenced this pull request Jul 18, 2022
| Package            | Version | Package            | Version |
| ------------------ | ------- | ------------------ | ------- |
| @uppy/dashboard    |   2.4.0 | @uppy/robodog      |   2.9.0 |
| @uppy/image-editor |   1.4.0 | uppy               |  2.13.0 |
| @uppy/transloadit  |   2.3.4 |                    |         |

- @uppy/transloadit: fix outdated file ids and incorrect usage of files (Merlijn Vos / #3886)
- @uppy/image-editor: remove beta notice (Merlijn Vos / #3877)
- meta: Fix broken links in _posts/2019-08-1.3.md (YukeshShr / #3884)
- meta: Fix broken link in _posts/2017-03-0.15.md (YukeshShr / #3883)
- @uppy/image-editor: Add image editor cancel event (James R T / #3875)
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.

@uppy/image-editor - Cancel Event
3 participants