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

Add xterm mouseleave to dismiss widget #66576

Merged
merged 2 commits into from Jan 24, 2019

Conversation

Projects
None yet
2 participants
@rcbevans
Copy link
Contributor

rcbevans commented Jan 16, 2019

Add a disposable listener to the 'mouseleave' event on the
terminalInstance xterm.element to dismiss any showing widget,
in case the user moves their mouse out of the xterm area directly,
without leaving the link that generated the widget/tooltip.

The real problem is xterm.js does not appear to be listening to the
'mouseleave' event of its own dom element, and so it does not fire
leaveCallback for registered CustomLinkHandler, WebLinkHandler, or
LocalLinkHandlers. As a result, if the user has the terminal split near
the link that generated the widget/tooltip, the widget remains until the
mouse cursor re-enters the xterm area that generated it, for xterm to
dismiss itself. See gif in #66421 for demonstration of the problem.

fixes #66421

Add xterm mouseleave to dismiss widget
Add a disposable listener to the 'mouseleave' event on the
terminalInstance xterm.element to dismiss any showing widget,
in case the user moves their mouse out of the xterm area directly,
without leaving the link that generated the widget/tooltip.

The real problem is xterm.js does not appear to be listening to the
'mouseleave' event of its own dom element, and so it does not fire
leaveCallback for registered CustomLinkHandler, WebLinkHandler, or
LocalLinkHandlers.  As a result, if the user has the terminal split near
the link that generated the widget/tooltip, the widget remains until the
mouse cursor re-enters the xterm area that generated it, for xterm to
dismiss itself.  See gif in #66421 for demonstration of the problem.

fixes #66421
@rcbevans

This comment has been minimized.

Copy link
Contributor Author

rcbevans commented Jan 16, 2019

I have created this PR, which does address the issue, as visible to users, as described in #66421, I believe the real issue actually lies in xterm.js, and this is a workaround rather than a fix.

terminalLinkHander.ts registers the various link handler callbacks which xterm uses to notify vscode that the mouse is over an actionable link, via the tooltipCallback and leaveCallback of the handlers.

If a user mouses over a link which reaches the edge of the xterm window, such as in the case where the terminal is split, and the divider is at the edge of an xterm actionable link, and the user moves the mouse directly out of the xterm element, without leaving the link first, xterm does not fire the leaveCallback event for the handler. This means VS Code wasn't hiding the tooltip/widget. In my opinion, xterm.js should get a fix so that if the mouse leaves the xterm text area where the link is, the leaveCallback of any active links should fire.

A side effect of this workaround, rather than true fix, is that whilst the widget VS Code was displaying now disappears correctly, the xterm link remains underlined in white. If the mouse cursor re-enters the xterm area off of the link, xterm then fires the leaveCallback, and the underline is removed from the link.

@Tyriar

Tyriar approved these changes Jan 24, 2019

Copy link
Member

Tyriar left a comment

Thanks @rcbevans 😃

@Tyriar Tyriar added this to the December/January 2019 milestone Jan 24, 2019

@Tyriar Tyriar merged commit 6846f58 into Microsoft:master Jan 24, 2019

1 of 2 checks passed

VS Code in progress
Details
license/cla All CLA requirements met.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment