-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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 for changing links in tooltip #113 #129
Conversation
@@ -37,6 +37,7 @@ class LinkTooltip extends Tooltip | |||
return unless range? and range.isCollapsed() | |||
anchor = this._findAnchor(range) | |||
if anchor | |||
@range = range |
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.
This is actually unnecessary. The Tooltip class will save the range before the tooltip showing.
Great thanks! I added some comments to the file. If you could fix those issues I can merge into develop. |
Requested changes complete. |
@@ -54,7 +54,11 @@ class LinkTooltip extends Tooltip | |||
|
|||
saveLink: -> | |||
url = this._normalizeURL(@textbox.value) | |||
@quill.formatText(@range, 'link', url, 'user') if @range? | |||
if @range? and @range.isCollapsed() |
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.
Sry I meant the if range should be its own followed by another if else block so:
if @range?
if @range.isCollapsed()
# Anchor
else
# formatText
This is because formatText needs range to be defined as well.
Fix made. |
anchor = this._findAnchor(@range) | ||
anchor.href = url if anchor? | ||
else | ||
@quill.formatText(@range, 'link', url, 'user') if @range? |
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.
One last thing: can you get rid of the if range at the end? It's already inside a check for range being defined
fix for changing links in tooltip closes #113
Fixes cross browser bug in #113. Allows links to be changed using the tooltip.