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

Cancel pending hover on mouse exit #14533

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Conversation

cdamus
Copy link
Contributor

@cdamus cdamus commented Nov 26, 2024

What it does

If the mouse exits before the HoverService has rendered its pop-up, cancel the pending presentation of that pop-up.

Fixes #14532.

How to test

Pass the mouse pointer quickly over items in the status bar that provide hover pop-ups. See that those pop-ups do not appear when the mouse pointer has already gone elsewhere. Verify that the pop-up behaves as before when the mouse pointer lingers on the target element. Both cases as captured here in a screen recording:

CleanShot 2024-11-26 at 11 32 32

Follow-ups

None.

Review checklist

Reminder for reviewers

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

I like this change!

I found one edge/case issue: When switching fast and often between two hoverable elements, I sometimes can reproduce that no more tooltip appears although I'm hovering an element. Once the mouse leaves and enters again the tooltip will reappear.

tooltips

The behavior is better than the status quo with the weird delayed tooltips, so I would be fine with a merge.

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Actually I noticed one more issue:

Previously it was possible to be quick enough to enter the tooltip with the mouse to copy its contents. This is no longer possible.

  • Is this a worthwhile feature?
  • Do we want to keep it?

In this Gif we can see the current state of master with the

  • weird delay of tooltips showing although the element is no longer hovered
  • the ability to enter and copy the tooltip content

EnterToolTips

@sdirix
Copy link
Member

sdirix commented Nov 28, 2024

@tsmaeder any preference on your side for today's release and/or followups?

@tsmaeder
Copy link
Contributor

I don't think this is a critical problem, so let's not force it in if it's not ready.

@cdamus
Copy link
Contributor Author

cdamus commented Nov 28, 2024

I don't think this is a critical problem, so let's not force it in if it's not ready.

I'm certainly not in such a hurry, so I wouldn't ask for it to make this release 😀

Previously it was possible to be quick enough to enter the tooltip with the mouse to copy its contents. This is no longer possible.

  • Is this a worthwhile feature?
  • Do we want to keep it?

This is something that I actually do quite a lot in other tools (some don't support selection in tool-tips, others do). I think I should be able to preserve this capability with a judicious sprinkling of debounces or similar. I'll give it a go.

@cdamus
Copy link
Contributor Author

cdamus commented Nov 28, 2024

Commit ecf3c2d restores mouse pointer access to the hover pop-up itself and also fixes other glitches.

@cdamus cdamus requested a review from sdirix November 28, 2024 16:41
@cdamus
Copy link
Contributor Author

cdamus commented Dec 4, 2024

@sdirix would you like to take another look?

If the mouse exits before the HoverService has rendered its pop-up,
cancel the pending presentation of that pop-up.

Also fix the problem of it being difficult to mouse over to the pop-up
itself after it is shown to interact with it (it being necessary to
carefully track over the bottom triangle). This is accomplished by
cancelling only after a short delay that allows the mouse to reach
either the pop-up or the original hover target.

Fixes eclipse-theia#14532

Signed-off-by: Christian W. Damus <cdamus.ext@eclipsesource.com>
@cdamus
Copy link
Contributor Author

cdamus commented Dec 10, 2024

Commit 56cfe5d takes a much simpler approach to fix both the problem of quick mouse fly-overs popping up the hover widget and also the difficulty of mousing over to the hover pop-up once it's shown.

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me 👍 I can confirm that this fixes the issue without introducing side-effects.

@msujew msujew added the ui/ux issues related to user interface / user experience label Dec 11, 2024
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Very nice! The code is simple and this is a great improvement for the usability in Theia. Great work 👍

@sdirix sdirix merged commit e2c9011 into eclipse-theia:master Dec 12, 2024
11 checks passed
@github-actions github-actions bot added this to the 1.57.0 milestone Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/ux issues related to user interface / user experience
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Hover service renders the hover pop-up too eagerly
4 participants