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

Fix tooltip behaviour #414

Merged
merged 12 commits into from
May 8, 2024
Merged

Fix tooltip behaviour #414

merged 12 commits into from
May 8, 2024

Conversation

Narref95
Copy link
Contributor

@Narref95 Narref95 commented Apr 9, 2024

fix #413

As keyboard_navigatable only works when element is focused I cant use it to remove the tooltip, so I added the tooltip to the new WindowHandle field instead of overlays map (to be able to remove it directly).

New tooltip behaviour:
Hide tooltip when some key is pressed
Hide tooltip when PointerUp / PointerDown

@dzhou121 dzhou121 requested a review from MinusGix April 9, 2024 17:40
@Long0x0
Copy link
Contributor

Long0x0 commented Apr 11, 2024

The tooltip still shows when dragging something over:

1

Could you also fix this?

@Narref95
Copy link
Contributor Author

The tooltip still shows when dragging something over:

Could you also fix this?

Nice catch! I didn't though about dragging over the tooltip. I will try to fix that too.

@jrmoulton
Copy link
Collaborator

My thoughts copy/paste from discord.

I don't think there is a strong need to special case tooltips from overlays.
To me it looks like a lot of duplicated API just so that you can ensure that there is only one tooltip showing at a time. I think a better solution would be to have the events that deactivate the tooltip be better handled.

Part of that would be having a ClickedOutside and (probably) KeyEventOutide events

I guess specifically in this case what is happening is that the PointerLeave event doesn't fire because the underlying view can be swapped out with keyboard events. Maybe we need to send PointerLeave if that happens

@Narref95
Copy link
Contributor Author

My thoughts copy/paste from discord.

I don't think there is a strong need to special case tooltips from overlays.
To me it looks like a lot of duplicated API just so that you can ensure that there is only one tooltip showing at a time. I think a better solution would be to have the events that deactivate the tooltip be better handled.

Part of that would be having a ClickedOutside and (probably) KeyEventOutide events

I guess specifically in this case what is happening is that the PointerLeave event doesn't fire because the underlying view can be swapped out with keyboard events. Maybe we need to send PointerLeave if that happens

ClickedOutside would not be needed for this case as the tooltip needs to be removed when any PointerDown is performed (inside and outside), this has been fixed adding the handle of this event.
I like the idea of adding KeyEventOutside if this can be used to manage the overlay inside the tooltip view implementation and remove the duplicated code added.
KeyEventOutside should be sent to all widgets of the screen instead of the active one?

Narref95 and others added 4 commits April 15, 2024 18:22
still won't work if border_radius < border_width / 2,
since the radius is limited by the stroke width
@Narref95 Narref95 marked this pull request as draft April 16, 2024 14:51
@Narref95
Copy link
Contributor Author

Fixed tooltip showing when dragging something over it, also remove tooltip when pointer wheel is triggered.

PR set as draft to implement a better solution that reduces code duplication.

@Narref95 Narref95 marked this pull request as ready for review April 27, 2024 09:06
@Narref95
Copy link
Contributor Author

  • Added the possibility to listen keyboard events without being focused, this gives the ability to close the tooltip when any key is pressed.
    This reduces code duplication instead of duplicating overlay logic for tooltips and adds this functionality to new use cases.

@dzhou121 dzhou121 merged commit b1b6bf4 into lapce:main May 8, 2024
7 checks passed
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.

Tooltips not hiding automatically on some cases
6 participants