-
Notifications
You must be signed in to change notification settings - Fork 94
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
fix: copy-paste markdown/raw text inconsistencies #5487
Conversation
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.
Gave this a quick test and think the behaviour is quite nice now, left a question inline
Nice, I just gave it a try and indeed it improves the copy and paste situation quite a lot! When adding the missing Still, I want to share some feedback that I got from different users after Nextcloud instances got upgraded to 28: The fact that the markdown syntax is copied into the In essence I think we should revert the behaviour to copy markdown syntax in to the To my understanding, the only use-case where having markdown content in the Some concrete examples with a broken user experience:
|
Results from the call discussing this:
|
I think for most users having "random" characters inside the copy result is unexpected behavior. I really have a lot complains from my users that can not copy anything from code blocks. Especially as codeblocks are used to preserve the formatting. When copying multiple elements the behavior is often not defined, so it would be fine to have the markdown e.g. for a table. Another common solution is to populate the clipboard with multiple mimetypes, e.g. office programs often provide HTML besides plain text. Thats why I suggested to either provide multiple mime types so the application pasted to can select the content it supports. Or have a special command for copying markdown only for experienced users, but most normal users expect simply text no "weird" characters. |
I totally agree. This PR fixes these issues as far as i can tell.
We do provide html and plain mime types. I'm not aware of any app that uses the plain/markdown mime type when pasting. So while this would be a good approach in general it would not solve the problems in the |
Yes makes sense |
…ste behavior Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Max <[email protected]>
41c8dfb
to
a2bd8de
Compare
Thanks for looking into this @max-nextcloud. While I agree with most of your analysis, the concrete pain points that I lined out that users (regularly) report to me with the current implementation remain with this pull requests. I think two things need to change at least to satisfy them:
Without these changes, copying from Text documents to any plain text processors will confuse people, and - most importantly - breaks their current workflows. |
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.
After discussing this with @max-nextcloud I'm confident that you have a good path forward in mind. Makes sense to get this PR merged first and improve the situation in a folllow-up afterwards 😊
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
/backport f10d3dc to stable29 |
* fix: traverse through nodes in order to determine the correct copy-paste behavior --------- Signed-off-by: Elizabeth Danzberger <[email protected]> Signed-off-by: Max <[email protected]> Co-authored-by: Max <[email protected]>
/backport f10d3dc to stable28 |
* fix: traverse through nodes in order to determine the correct copy-paste behavior --------- Signed-off-by: Elizabeth Danzberger <[email protected]> Signed-off-by: Max <[email protected]> Co-authored-by: Max <[email protected]>
* fix: traverse through nodes in order to determine the correct copy-paste behavior --------- Signed-off-by: Elizabeth Danzberger <[email protected]> Signed-off-by: Max <[email protected]> Co-authored-by: Max <[email protected]>
* fix: traverse through nodes in order to determine the correct copy-paste behavior --------- Signed-off-by: Elizabeth Danzberger <[email protected]> Signed-off-by: Max <[email protected]> Co-authored-by: Max <[email protected]>
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
📝 Summary
Fix #5331
As described in #5331 and #5212, there are some issues with the copy-paste behavior and some inconsistencies regarding how markdown is copied vs. raw text. I think it would be helpful to define what the expected behavior would be in certain situations so that this can be refined.
This draft PR currently contains some additions I made which traverses ProseMirror Nodes upon copying text, and tries to determine if markdown should be copied or if the raw text should be copied. In my opinion, it works a bit better than before and behaves more consistently, but I did still notice some issues, such as losing block nesting sometimes (discovered when copying a code block nested in a list).
Currently defined behavior
All testing and feedback is welcome.