Fixes #5143. ColorBar mouse events use MouseBindings with grab/ungrab#5289
Merged
YourRobotOverlord merged 1 commit intoMay 10, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses #5143 by reworking ColorBar mouse handling so callers can cancel/suppress mouse-driven value changes and so drag operations remain bounded to the originating bar (via mouse grab/ungrab).
Changes:
- Routed
ColorBarmouse press/drag intoMouseBindings→Command.Activate, moving value updates out ofOnMouseEvent. - Added explicit grab/ungrab lifecycle handling to prevent cross-bar drag contamination.
- Updated and added unit tests to validate click updates, cancellation, and drag bounding behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Terminal.Gui/Views/Color/ColorBar.cs | Moves mouse-driven value updates into Command.Activate and adds grab/ungrab management for bounded drags. |
| Tests/UnitTestsParallelizable/Views/ColorPickerTests.cs | Updates mouse-invocation paths and adds regression tests for cancelability and drag bounding. |
…/ungrab Move value-update logic from OnMouseEvent into a Command.Activate handler bound via MouseBindings.Add. This makes mouse interaction cancellable by external code (e.g. TerminalGuiDesigner) which can clear the bindings. Add GrabMouse/UngrabMouse in OnMouseEvent so drag events are routed exclusively to the originating bar, preventing cross-bar drag contamination. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
YourRobotOverlord
force-pushed
the
fix/5143-colorbar-mouse-bindings
branch
from
7f00968 to
273ca61
Compare
tig
approved these changes
May 9, 2026
This was referenced May 21, 2026
This was referenced May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copilot Session
cf168204-1066-49b3-be7f-34d23d1ef2c0
Summary
Fixes #5143.
ColorBar.OnMouseEventpreviously updatedValuedirectly before theMouseEventevent was raised. This meant external code (e.g. TerminalGuiDesigner) couldn't cancel mouse interaction by subscribing toMouseEventor by clearingMouseBindings.Also, drag operations were not bounded to the originating bar — pressing on one
ColorBarand dragging into a sibling caused the second bar to react.Changes
Terminal.Gui/Views/Color/ColorBar.csCommand.Activate: Add aMouseBindings.Add(MouseFlags.LeftButtonPressed, Command.Activate)andMouseBindings.Add(MouseFlags.LeftButtonPressed | MouseFlags.PositionReport, Command.Activate)in the constructor. TheCommand.Activatehandler checks whether it was triggered by aLeftButtonPressedmouse event; if so, it updatesValueand callsSetFocus(). For all other invocations (e.g.LeftButtonReleased) it falls through toDefaultActivateHandler.OnMouseEvent:OnMouseEventnow only manages grab:GrabMouse(this)on first press,UngrabMouse()on release, always returnsfalse(so the event chain continues toRaiseCommandsBoundToButtonFlags).ColorBarmouse interaction by callingmouseBindings.Remove(MouseFlags.LeftButtonPressed)— consistent with how Terminal.Gui cancellable commands work generally.Tests/UnitTestsParallelizable/Views/ColorPickerTests.csRaiseMouseEvent(...)toNewMouseEvent(...)— necessary because value update moved fromOnMouseEvent(called byRaiseMouseEvent) toCommand.Activate(only reachable viaNewMouseEvent→RaiseCommandsBoundToButtonFlags).ClickingDifferentBars_ChangesFocus: updated to send aLeftButtonReleasedbetween the two press events (releasing the grab before the second press), matching real terminal behavior.ColorBar_MousePress_UpdatesValue— verifies value updates on clickColorBar_MouseEvent_CanBeCancelled— subscribes toMouseEvent, setsHandled=true, verifies value is NOT updated (the cancellability fix)ColorBar_Drag_BoundedToOriginatingBar— press on RBar, drag into GBar area, verify only RBar value changed (the drag-bounded fix)Test Results
All 16,862 tests pass (0 failures, 17 pre-existing skips).