Skip to content

Conversation

@steffen
Copy link
Contributor

@steffen steffen commented Oct 4, 2021

All pastes that are modified by @github/paste-markdown can currently not be undone by pressing cmd + z (on Mac) or by clicking Undo in 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.execCommand seems to be deprecated.
Though, we are using it in our @github/markdown-toolbar-element library:
https://github.com/github/markdown-toolbar-element/blob/a5b444a0405348be2af01535a8b594147370dc4b/src/index.ts#L379-L418

I copied the implementation of the markdown-toolbar-element implementation 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?

@steffen steffen requested a review from a team as a code owner October 4, 2021 17:59
@steffen steffen mentioned this pull request Oct 4, 2021
Copy link
Collaborator

@dgreif dgreif left a 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?

@steffen
Copy link
Contributor Author

steffen commented Oct 5, 2021

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?

@steffen
Copy link
Contributor Author

steffen commented Oct 7, 2021

👋 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.

@steffen steffen requested a review from dgreif October 15, 2021 12:51
Copy link
Contributor

@keithamus keithamus left a 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?

@manuelpuyol manuelpuyol merged commit 808c3d8 into github:main Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants