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

WebLinks plugin don't follow link on selection #1958

Merged
merged 7 commits into from
Mar 30, 2019

Conversation

bmf-ribeiro
Copy link
Contributor

@bmf-ribeiro bmf-ribeiro commented Mar 6, 2019

This is not the correct solution, but it's something to get the ball rolling...

Feedback on what's the best approach is welcome, right now it is in a working state, but I believe we should be smarter about this... I have 2 questions regarding how to proceed:

  1. Should this logic live in the plugin, or should it go to the Linkifier?
  2. If I wanted to be smarter about the selection, what's the recommended way to access non public API's in the addon world?

Fixes #1947

src/addons/webLinks/webLinks.ts Outdated Show resolved Hide resolved
@bmf-ribeiro bmf-ribeiro marked this pull request as ready for review March 7, 2019 21:02
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Actually after further testing and thinking on this I don't think we can go this direction as then people can't click links if they have any selection. What we we add a mousedown listener in MouseZoneManager which measures the length of the selection, and verify it hasn't checked in MouseZoneManager._onClick instead?

@bmf-ribeiro
Copy link
Contributor Author

That seems like a better approach to this problem.. I'll try to have a POC during the week! 😄

@bmf-ribeiro
Copy link
Contributor Author

After some testing, the recommended approach seems to be working! Awaiting review 😃

@Tyriar Tyriar added this to the 3.13.0 milestone Mar 30, 2019
@Tyriar Tyriar self-assigned this Mar 30, 2019
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

👌

@Tyriar Tyriar merged commit bab94eb into xtermjs:master Mar 30, 2019
@Tyriar Tyriar mentioned this pull request May 10, 2019
10 tasks
@bmf-ribeiro bmf-ribeiro deleted the 1947-weblinks-with-selection branch November 27, 2019 17:49
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.

2 participants