[GH-ISSUE #972] Box.SetMouseCapture does not capture/consume events properly [+PR] #710

New issue

Closed

opened 2026-03-04 01:07:11 +03:00 by kerem · 1 comment

kerem commented

2026-03-04 01:07:11 +03:00

Owner

Originally created by @GiGurra on GitHub (Apr 20, 2024).
Original GitHub issue: https://github.com/rivo/tview/issues/972

At present, it is not possible to consume events in the mouse event capture function. Even if you return nil, the event will not be marked as consumed. This causes (among other things) Application.draw() to not fire properly, and essentially, means that any gui state changes you make inside the capture function, isn't rendered.

The following code (from the Application event loop) shows where the draw call would be skipped when the wrong consume status is returned

case *tcell.EventMouse:
	consumed, isMouseDownAction := a.fireMouseActions(event)
	if consumed {
		a.draw()
	}
	a.lastMouseButtons = event.Buttons()
	if isMouseDownAction {
		a.mouseDownX, a.mouseDownY = event.Position()
	}

The problem/bug exists in Box.WrapMouseHandler, and I have created a PR with suggestion on how to fix this here: https://github.com/rivo/tview/pull/967

As I understand the tview maintainers prefer an issue created together with a PR. Is this correct?

Originally created by @GiGurra on GitHub (Apr 20, 2024). Original GitHub issue: https://github.com/rivo/tview/issues/972 At present, it is not possible to consume events in the mouse event capture function. Even if you return nil, the event will not be marked as consumed. This causes (among other things) `Application.draw()` to not fire properly, and essentially, means that any gui state changes you make inside the capture function, isn't rendered. The following code (from the Application event loop) shows where the draw call would be skipped when the wrong consume status is returned ``` case *tcell.EventMouse: consumed, isMouseDownAction := a.fireMouseActions(event) if consumed { a.draw() } a.lastMouseButtons = event.Buttons() if isMouseDownAction { a.mouseDownX, a.mouseDownY = event.Position() } ``` The problem/bug exists in `Box.WrapMouseHandler`, and I have created a PR with suggestion on how to fix this here: https://github.com/rivo/tview/pull/967 As I understand the tview maintainers prefer an issue created together with a PR. Is this correct?

kerem closed this issue

2026-03-04 01:07:12 +03:00

kerem commented

2026-03-04 01:07:12 +03:00

Author

Owner

@rivo commented on GitHub (May 19, 2024):

In a sense, @digitallyserviced is right that we don't want to assume that an event that is intercepted and not passed on needs to result in the screen being redrawn. Let's say a primitive handles mouse movements and you want to disable them. You wouldn't want the screen to be redrawn every time the user moves the mouse in that case. So this is why I've closed your PR.

To be honest, this is something I simply forgot to implement. The mouse capture function should also return a consumed flag (and a capture primitive). But I cannot simply add this now. Instead, my latest commit introduces a MouseConsumed action which you can return, to indicate that you've consumed an event and that you want the screen to be redrawn. Sure, you could also call Application.Draw() yourself but whenever possible, I want to allow access to this functionality without having to keep a global reference to Application. (It's not always possible but it should be possible in the most common cases.)

As for opening an issue alongside a PR, I would even suggest only opening an issue first, without a PR. (This is mentioned in the contributing guide.) Most PR's I get ignore important details, don't consider negative side effects, introduce unnecessary complexity, or simply lack the elegance I'd like to see in this project. Unfortunately, I have to reject most of them. So it's better to simply open an issue and we'll take it from there.

@rivo commented on GitHub (May 19, 2024): In a sense, @digitallyserviced is right that we don't want to assume that an event that is intercepted and not passed on needs to result in the screen being redrawn. Let's say a primitive handles mouse movements and you want to disable them. You wouldn't want the screen to be redrawn every time the user moves the mouse in that case. So this is why I've closed your PR. To be honest, this is something I simply forgot to implement. The mouse capture function should also return a `consumed` flag (and a `capture` primitive). But I cannot simply add this now. Instead, my latest commit introduces a `MouseConsumed` action which you can return, to indicate that you've consumed an event and that you want the screen to be redrawn. Sure, you could also call `Application.Draw()` yourself but whenever possible, I want to allow access to this functionality without having to keep a global reference to `Application`. (It's not always possible but it should be possible in the most common cases.) As for opening an issue alongside a PR, I would even suggest only opening an issue first, without a PR. (This is mentioned in the [contributing guide](https://github.com/rivo/tview/blob/master/CONTRIBUTING.md).) Most PR's I get ignore important details, don't consider negative side effects, introduce unnecessary complexity, or simply lack the elegance I'd like to see in this project. Unfortunately, I have to reject most of them. So it's better to simply open an issue and we'll take it from there.

kerem referenced this issue

2026-03-04 01:09:04 +03:00

[PR #711] [CLOSED] Allow setting the pause between redraws #1022