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

Changing hyperlink fails. #113

Closed
byronm opened this issue May 21, 2014 · 7 comments
Closed

Changing hyperlink fails. #113

byronm opened this issue May 21, 2014 · 7 comments

Comments

@byronm
Copy link
Contributor

byronm commented May 21, 2014

  1. Select (or place the cursor within) hyperlinked text
  2. Click change on the link-tooltip
  3. Change the url value
  4. Click done

Though the link-tooltip UI works fine, the formatted text retains the original url instead of the newly set one. Logging the call to formatText I see that the range is length 0, so seems like an issue with the range saving and selection change event.

@timcosta
Copy link

actually, my fix for this isnt 100% complete.

is desired behavior to change the formatted text ONLY when the formatted text is exactly the old link, or change the formatted text regardless?

@jhchen
Copy link
Member

jhchen commented May 29, 2014

The href should be changed regardless of what the text of the link is.

@timcosta
Copy link

Agreed, however I asked about changing the formatted text, not the href. My current PR changes the href regardless.

@jhchen
Copy link
Member

jhchen commented May 29, 2014

Hmm I think I'm confused by the question then. The only way you can change the textual content right now is by typing, regardless of the format whether it's bolded, linked, or otherwise. Maybe if you provide a specific scenario?

@timcosta
Copy link

Currently, when you create a link out of selected text it all works fine. If you attempt to change the URL of the link after using the tooltip, the href of the actual is not updated. This can be verified by viewing the inspector in Chrome/FF.

My PR fixes the issue of not updated the href of the tag.

I am asking if the desired functionality is to also change the formatted text <a>THIS TEXT</a> to the new URL because the issue talks about the formattedText retaining the old URL, which I would expect.

@jhchen
Copy link
Member

jhchen commented May 29, 2014

Ah I see. Thank you for the clarification. No it should not change the formatted text. In the future we may have the link tooltip also have a text field (similar to how Gmail/GDocs works) but for now the tooltip should just control the href.

@timcosta
Copy link

alright. well in that case I believe this PR is ready to be accepted. I am not sure how to check the functionality of the Deltas in relation to my change, so that is something I would appreciate help testing.

@jhchen jhchen closed this as completed in 5284db4 May 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants