Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

feat(FormattingToolbar): implement hyperlink tooltip - I100 #142

Merged

Conversation

sahalsaad
Copy link
Contributor

Issue #100

Replace js alert when setting the link to use popup

Changes

alt text

Flags

  • Not sure why the selection color gone once I select the text input.

Signed-off-by: sahalsaad <caalshift@gmail.com>
@jolanglinais
Copy link
Member

Flagging @Michael-Grover for this as well.

@jolanglinais
Copy link
Member

Not sure why the selection color gone once I select the text input.

What do you mean by this?

@sahalsaad
Copy link
Contributor Author

Not sure why the selection color gone once I select the text input.

What do you mean by this?

In previous PR. @DianaLease mention this.

@jolanglinais
Copy link
Member

Regarding all of #142 (comment), I think this is an improvement and could be merged and then iterated upon to improve further. Thoughts @DianaLease?

@DianaLease
Copy link
Member

Regarding all of #142 (comment), I think this is an improvement and could be merged and then iterated upon to improve further. Thoughts @DianaLease?

Yes, I agree. Looks good!

@jolanglinais jolanglinais changed the title Implement hyperlink tooltip feat(FormattingToolbar): implement hyperlink tooltip - I100 Oct 22, 2019
@jolanglinais
Copy link
Member

@sahalsaad I'm also noticing that you need to right click to open the link.

@Michael-Grover
Copy link

@sahalsaad This looks great, thank you for taking this issue. Can you change the form field descriptions and placeholder text to the following:

Link Text
Text

Link URL
https://example.com

Would it be possible for this UI to be placed above or below the text, so that it is easier for the user to tell which text is becoming a Hyperlink? See this example from Google Docs
image
image

@sahalsaad
Copy link
Contributor Author

@sahalsaad This looks great, thank you for taking this issue. Can you change the form field descriptions and placeholder text to the following:

Link Text
Text

Link URL
https://example.com

The text and placeholder already correct. Refer index.js line 293-298.

Would it be possible for this UI to be placed above or below the text, so that it is easier for the user to tell which text is becoming a Hyperlink? See this example from Google Docs
image
image

Sorry. I spent some time to research on that but not able to do it.
I don't have much knowledge on slate.js.

@sahalsaad
Copy link
Contributor Author

@sahalsaad I'm also noticing that you need to right click to open the link.

Which link? Is it different from current behavior?

Copy link
Member

@jolanglinais jolanglinais left a comment

Choose a reason for hiding this comment

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

These are some issues I will file which seem a bit separate - this is an improvement on what currently exists.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Medium Hacktoberfest by DigitalOcean and DEV Type: Feature Request 🛍️ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants