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

Re-show the tooltip on mouse-move following a click in TooltipArea. #1209

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

m-sasha
Copy link

@m-sasha m-sasha commented Mar 20, 2024

The tooltip is currently hidden when the TooltipArea is pressed, and it remains hidden until mouse exit and re-enter.
This is a problem for widgets that have more than one mouse interaction per hover session, e.g. a text field.

Proposed Changes

  • Start the tooltip timer on mouse-move following a press (actually, always, as long as it's not already started).
  • Don't re-show the tooltip on press -> exit -> enter.

See my comments for explanations why each change was made.

Testing

Test: Added several unit tests.

Issues Fixed

Fixes: JetBrains/compose-multiplatform#4515

Comment on lines +112 to +114
if (job?.isActive == true) { // Don't restart the job if it's already active
return
}
Copy link
Author

Choose a reason for hiding this comment

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

This is needed because this function is now potentially called on every mouse-move, and we don't want to restart the timer.

Comment on lines +138 to +140
if (!isVisible && !it.buttons.areAnyPressed) {
startShowing()
}
Copy link
Author

Choose a reason for hiding this comment

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

This fixes the case of press -> exit -> enter
Previously we would re-show the tooltip.

Comment on lines +144 to +146
if (!isVisible && !it.buttons.areAnyPressed) {
startShowing()
}
Copy link
Author

Choose a reason for hiding this comment

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

Re-show the tooltip on mouse-move (but no buttons pressed).

@m-sasha m-sasha force-pushed the m-sasha/restart-tooltip-timer-after-click branch from 08b6919 to f61ed34 Compare March 20, 2024 11:24
Comment on lines -194 to -205
private suspend fun PointerInputScope.detectDown(onDown: (Offset) -> Unit) {
while (true) {
awaitPointerEventScope {
val event = awaitPointerEvent(PointerEventPass.Initial)
val down = event.changes.find { it.changedToDown() }
if (down != null) {
onDown(down.position)
}
}
}
}

Copy link
Author

Choose a reason for hiding this comment

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

Not sure why this was needed, and in any case it's buggy - you're supposed to put the while inside the awaitPointerEventScope.

@@ -121,6 +120,7 @@ fun TooltipArea(

fun hide() {
job?.cancel()
job = null
Copy link
Author

Choose a reason for hiding this comment

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

Because why not.

Comment on lines -147 to +152
.pointerInput(Unit) {
detectDown {
hide()
}
.onPointerEvent(PointerEventType.Press, pass = PointerEventPass.Initial) {
hide()
Copy link
Author

Choose a reason for hiding this comment

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

Not sure why there was a need for a custom implementation here.

@m-sasha m-sasha force-pushed the m-sasha/restart-tooltip-timer-after-click branch from f61ed34 to a0c3171 Compare March 20, 2024 11:42
detectDown {
hide()
}
.onPointerEvent(PointerEventType.Press, pass = PointerEventPass.Initial) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need Initial instead of the main pass here?

Copy link
Author

@m-sasha m-sasha Mar 20, 2024

Choose a reason for hiding this comment

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

I don't know, but it was in the code already, so I decided not to change it just in case.

@m-sasha m-sasha merged commit 832d708 into jb-main Mar 21, 2024
6 checks passed
@m-sasha m-sasha deleted the m-sasha/restart-tooltip-timer-after-click branch March 21, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants