-
Notifications
You must be signed in to change notification settings - Fork 41
Add pasting URLs on selected text as Markdown links #23
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
Changes from all commits
da17a94
8f4b8e2
834a61c
9be6083
460e1b6
d4c60b1
ac3e95a
a5d440e
1c6b0cd
a3de35a
7840f7d
6c4781f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,36 @@ | ||
| import {install as installLink, uninstall as uninstallLink} from './paste-markdown-image-link' | ||
| import {install as installImageLink, uninstall as uninstallImageLink} from './paste-markdown-image-link' | ||
| import {install as installLink, uninstall as uninstallLink} from './paste-markdown-link' | ||
| import {install as installTable, uninstall as uninstallTable} from './paste-markdown-table' | ||
| import {install as installText, uninstall as uninstallText} from './paste-markdown-text' | ||
|
|
||
| interface Subscription { | ||
| unsubscribe: () => void | ||
| } | ||
|
|
||
| export default function subscribe(el: HTMLElement): Subscription { | ||
| function subscribe(el: HTMLElement): Subscription { | ||
| installTable(el) | ||
| installImageLink(el) | ||
| installLink(el) | ||
| installText(el) | ||
|
|
||
| return { | ||
| unsubscribe: () => { | ||
| uninstallTable(el) | ||
| uninstallImageLink(el) | ||
| uninstallLink(el) | ||
| uninstallText(el) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export { | ||
| subscribe, | ||
| installImageLink, | ||
| installLink, | ||
| installTable, | ||
| installText, | ||
| uninstallImageLink, | ||
| uninstallTable, | ||
| uninstallLink, | ||
| uninstallText | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import {insertText} from './text' | ||
|
|
||
| export function install(el: HTMLElement): void { | ||
| el.addEventListener('paste', onPaste) | ||
| } | ||
|
|
||
| export function uninstall(el: HTMLElement): void { | ||
| el.removeEventListener('paste', onPaste) | ||
| } | ||
|
|
||
| function onPaste(event: ClipboardEvent) { | ||
| const transfer = event.clipboardData | ||
| if (!transfer || !hasPlainText(transfer)) return | ||
|
|
||
| const field = event.currentTarget | ||
| if (!(field instanceof HTMLTextAreaElement)) return | ||
|
|
||
| const text = transfer.getData('text/plain') | ||
| if (!text) return | ||
|
|
||
| if (isWithinLink(field)) return | ||
|
|
||
| event.stopPropagation() | ||
| event.preventDefault() | ||
|
|
||
| const selectedText = field.value.substring(field.selectionStart, field.selectionEnd) | ||
|
|
||
| insertText(field, linkify(selectedText, text), {addNewline: false}) | ||
| } | ||
|
|
||
| function hasPlainText(transfer: DataTransfer): boolean { | ||
| return Array.from(transfer.types).includes('text/plain') | ||
| } | ||
|
|
||
| function isWithinLink(textarea: HTMLTextAreaElement): boolean { | ||
| const selectionStart = textarea.selectionStart || 0 | ||
|
|
||
| if (selectionStart > 1) { | ||
| const previousChars = textarea.value.substring(selectionStart - 2, selectionStart) | ||
| return previousChars === '](' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about checking for HTML links as well? Since markdown generally supports HTML as well, should we check if we're in a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've thought about some other edge cases as well, for example let's say you have this issue template: If you select Though, I'm not sure we should account for all edge cases or if users will learn quickly that they need to delete the selected text first and then paste the URL in order to have the URL as is. Regarding detecting if the selected text is within an |
||
| } else { | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| function linkify(selectedText: string, text: string): string { | ||
| return selectedText.length && isURL(text) ? `[${selectedText}](${text})` : text | ||
| } | ||
|
|
||
| function isURL(url: string): boolean { | ||
| return /^https?:\/\//i.test(url) | ||
| } | ||
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.
What do you think about having all the
install*functions in here so that consumers that use subscribe will get all the features, but then if they want to conditionally apply features they can import only the installers they plan on using?Uh oh!
There was an error while loading. Please reload this page.
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.
@koddsson I've thought about that as well. Do you know if this line is the only place where we consume the library at GitHub?
Since
subscribeis now a named export, users of the library have to adjust the usage of the library either way, so yeah, I agree with adding the new feature to subscribe right away. We then only need to adjust this line as well to use theaddandremovehooks as I do in the lines below so that the new Link feature is not loaded by default.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.
@koddsson I just saw your proposal on using the
addandremovehooks for all functions. I missed that earlier. I'll go with that approach. 👍 Thank you! 🙇♂️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.
@koddsson I have applied your proposal here: 1c6b0cd Thank you! 🙇♂️