-
Notifications
You must be signed in to change notification settings - Fork 41
Add "undo" support #28
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
Conversation
dgreif
left a comment
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.
Adding some feedback based on the current implementation, but I would love to see this move in a direction that uses newer APIs (if they exist). @koddsson @keithamus have either of you dealt with "Undo" functionality before?
💯 @koddsson @keithamus Do you know of another way to change textarea values programmatically while supporting the undo functionality? |
|
👋 A friendly bump on this pull request. 😊 My thought is that if we don't find newer APIs, the approach from https://github.com/github/markdown-toolbar-element should be fairly battle tested and save. |
keithamus
left a comment
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.
Yeah I think this is perhaps the best option we have, at least for now.
/cc @manuelpuyol as First Responder would you like to review, merge and release this one?
All pastes that are modified by
@github/paste-markdowncan currently not be undone by pressing cmd + z (on Mac) or by clickingUndoin the textarea's context menu.This seems to not have been an issue so far for the existing functionality such as pasting a table or dragging an image URL into a textarea.
For the new linkify feature we expect this to be a more obvious and annoying issue, either when users select accidentally the wrong text, or when they actually expect to replace text with an URL or when they just want to play/test with the new functionality.
The reason for the missing "undo" support is that the current implementation uses
textarea.value = 'value'.The only way that I know of supporting the "undo" functionality is by using
document.execCommand('insertText').Unfortunately
document.execCommandseems to be deprecated.Though, we are using it in our
@github/markdown-toolbar-elementlibrary:https://github.com/github/markdown-toolbar-element/blob/a5b444a0405348be2af01535a8b594147370dc4b/src/index.ts#L379-L418
I copied the implementation of the
markdown-toolbar-elementimplementation mostly as is and it seems to work flawlessly in my testing.I know "copying" an implementation from another source instead of reusing it is not ideal.
I also know that using a deprecated API is not ideal.
Though, I don't have any better ideas for adding "undo" support to
@github/paste-markdown.What do you think @github/ui-frameworks?