-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
enhance(editor): confirm before reset #2052
base: main
Are you sure you want to change the base?
Conversation
2b4b3ea
to
be49915
Compare
be49915
to
d937916
Compare
editor/js/editable-css.ts
Outdated
@@ -95,7 +95,12 @@ import "../css/editable-css.css"; | |||
function handleResetEvents() { | |||
const resetButton = document.getElementById("reset") as HTMLElement; | |||
|
|||
resetButton.addEventListener("click", () => { | |||
resetButton.addEventListener("click", (event) => { | |||
if (!window.confirm("Do you really want to reset everything?")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the copy, I like wording like this:
Are you sure you want to reset the editor?
Any changes you have made will be lost.
-> OK / Cancel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, one minor comment about copy, but it's working well! 👍🏻
Thank you!
Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Fixes #1739.