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

fix: Link can't be pasted on selected text in block content #7618

Merged
merged 2 commits into from Dec 7, 2022

Conversation

sallto
Copy link
Contributor

@sallto sallto commented Dec 6, 2022

fixes #7268

Fixed by testing whether text is selected while pasting and formatting accordingly

The code is kinda ugly, but I don't know of a nicer way

Copy link
Collaborator

@logseq-cldwalker logseq-cldwalker left a comment

Choose a reason for hiding this comment

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

Hi. This works as advertised

EDIT: for core team
I'm surprised we have existing functionality like this for paste as I consider it surprising for selected text to combine with something from the clipboard. I think this untested complexity is a reason why we have a number of copy paste bugs. My advice would be not add more functionality like this but I'll defer to someone else for this review

@tiensonqin
Copy link
Contributor

I'm surprised we have existing functionality like this for paste as I consider it surprising for selected text to combine with something from the clipboard

@logseq-cldwalker afaik GitHub does the same thing when you select some text and paste a URL.

@tiensonqin
Copy link
Contributor

I do think that the warning notification here is not needed.

Copy link
Contributor

@tiensonqin tiensonqin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

@logseq-cldwalker
Copy link
Collaborator

logseq-cldwalker commented Dec 7, 2022

@logseq-cldwalker afaik GitHub does the same thing when you select some text and paste a URL.

@tiensonqin Fair point 👍

Copy link
Collaborator

@logseq-cldwalker logseq-cldwalker left a comment

Choose a reason for hiding this comment

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

@sallto Thanks for the fix! 👍 🚢 My previous comment was intended more for the core team. Tienson made a good point about this becoming a more common paste functionality so I remove my main concern

@logseq-cldwalker logseq-cldwalker merged commit c9d58eb into logseq:master Dec 7, 2022
@sallto sallto deleted the cpfix branch December 21, 2022 10:11
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.

Link can't be pasted on selected text in block content
3 participants