Skip to content

Fixes #5143. ColorBar mouse events use MouseBindings with grab/ungrab #5289

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.

Sign up for GitHub

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

Merged
YourRobotOverlord merged 1 commit into from
May 10, 2026
Merged

Fixes #5143. ColorBar mouse events use MouseBindings with grab/ungrab #5289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 49 additions & 10 deletions Terminal.Gui/Views/Color/ColorBar.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,47 @@ protected ColorBar ()
AddCommand (Command.LeftStart, _ => SetZero ());
AddCommand (Command.RightEnd, _ => SetMax ());

// Override Activate to handle mouse-press and drag. When triggered by a left-button press
// (initial click or drag), extract the position from the MouseBinding context, update
// Value, set focus, and grab the mouse so subsequent drag events are routed to this bar
// exclusively. Grabbing here (rather than in OnMouseEvent) ensures that if a consumer
// cancels the event by setting e.Handled=true in MouseEvent, the press command is never
// reached and no grab occurs.
AddCommand (
Command.Activate,
ctx =>
{
if (ctx?.Binding is MouseBinding { MouseEvent: { } mouse }
&& mouse.Flags.FastHasFlags (MouseFlags.LeftButtonPressed))
{
UpdateValueFromMousePosition (mouse);
SetFocus ();
App?.Mouse.GrabMouse (this);

return true;
}

return DefaultActivateHandler (ctx);
});

KeyBindings.Add (Key.CursorLeft, Command.Left);
KeyBindings.Add (Key.CursorRight, Command.Right);
KeyBindings.Add (Key.CursorLeft.WithShift, Command.LeftExtend);
KeyBindings.Add (Key.CursorRight.WithShift, Command.RightExtend);
KeyBindings.Add (Key.Home, Command.LeftStart);
KeyBindings.Add (Key.End, Command.RightEnd);
MouseBindings.Remove (MouseFlags.LeftButtonClicked);

// Remove the base LeftButtonReleased → Activate binding so that releasing the mouse
// does not fire DefaultActivateHandler (Activating/Activated). UngrabMouse is still
// called from OnMouseEvent on release, so the grab lifecycle is unaffected.
MouseBindings.Remove (MouseFlags.LeftButtonReleased);

// Bind press and drag to Activate so external code can fully suppress mouse interaction
// by removing these two bindings (e.g. TerminalGuiDesigner editor mode). When both are
// absent no value update, focus change, grab, or drag routing occurs.
MouseBindings.Add (MouseFlags.LeftButtonPressed, Command.Activate);
MouseBindings.Add (MouseFlags.LeftButtonPressed | MouseFlags.PositionReport, Command.Activate);
}
Comment thread
YourRobotOverlord marked this conversation as resolved.

/// <summary>
Expand Down Expand Up @@ -123,19 +157,24 @@ protected override bool OnDrawingContent (DrawContext? context)
/// <inheritdoc/>
protected override bool OnMouseEvent (Mouse mouse)
{
if (mouse.Flags.FastHasFlags (MouseFlags.LeftButtonPressed))
// Release the grab on button-up so drag events stop routing exclusively to this bar.
// The grab itself is established in the Command.Activate handler so that cancelling the
// MouseEvent (e.Handled = true) prevents both the value update and the grab.
if (mouse.IsReleased && App?.Mouse.IsGrabbed (this) == true)
{
if (mouse.Position!.Value.X >= _barStartsAt)
{
double v = MaxValue * ((double)mouse.Position!.Value.X - _barStartsAt) / (_barWidth - 1);
Value = Math.Clamp ((int)v, 0, MaxValue);
}
SetFocus ();

// Do not mark as handled to allow Activating to be raised
App.Mouse.UngrabMouse ();
}

return mouse.Handled;
return false;
}
Comment thread
YourRobotOverlord marked this conversation as resolved.

private void UpdateValueFromMousePosition (Mouse mouse)
{
if (mouse.Position is { } pos && pos.X >= _barStartsAt)
{
double v = MaxValue * ((double)pos.X - _barStartsAt) / (_barWidth - 1);
Value = Math.Clamp ((int)v, 0, MaxValue);
}
}

/// <summary>
Expand Down
109 changes: 85 additions & 24 deletions Tests/UnitTestsParallelizable/Views/ColorPickerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public void ClickingAtEndOfBar_SetsMaxValue ()
cp.Draw (); // Draw is needed to update TrianglePosition

// Click at the end of the Red bar
cp.Focused!.RaiseMouseEvent (new Mouse
cp.Focused!.NewMouseEvent (new Mouse
{
Flags = MouseFlags.LeftButtonPressed, Position = new Point (19, 0) // Assuming 0-based indexing
});
Expand Down Expand Up @@ -213,7 +213,7 @@ public void ClickingBeyondBar_ChangesToMaxValue ()
cp.Draw (); // Draw is needed to update TrianglePosition

// Click beyond the bar
cp.Focused!.RaiseMouseEvent (new Mouse
cp.Focused!.NewMouseEvent (new Mouse
{
Flags = MouseFlags.LeftButtonPressed, Position = new Point (21, 0) // Beyond the bar
});
Expand Down Expand Up @@ -243,33 +243,17 @@ public void ClickingDifferentBars_ChangesFocus ()

cp.Draw (); // Draw is needed to update TrianglePosition

// Click on Green bar
// Click on Green bar (press then release to complete the click cycle and release grab)
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, ScreenPosition = new Point (0, 1) });

//cp.SubViews.OfType<GBar> ()
// .Single ()
// .OnMouseEvent (
// new ()
// {
// Flags = MouseFlags.LeftButtonPressed,
// Position = new (0, 1)
// });
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonReleased, ScreenPosition = new Point (0, 1) });

cp.Draw (); // Draw is needed to update TrianglePosition

Assert.IsAssignableFrom<GBar> (cp.Focused);

// Click on Blue bar
// Click on Blue bar (press then release to complete the click cycle and release grab)
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, ScreenPosition = new Point (0, 2) });

//cp.SubViews.OfType<BBar> ()
// .Single ()
// .OnMouseEvent (
// new ()
// {
// Flags = MouseFlags.LeftButtonPressed,
// Position = new (0, 2)
// });
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonReleased, ScreenPosition = new Point (0, 2) });

cp.Draw (); // Draw is needed to update TrianglePosition

Expand Down Expand Up @@ -706,14 +690,14 @@ public void RGB_MouseNavigation ()

Assert.IsAssignableFrom<IColorBar> (cp.Focused);

cp.Focused!.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (3, 0) });
cp.Focused!.NewMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (3, 0) });

cp.Draw (); // Draw is needed to update TrianglePosition

Assert.Equal (3, r.TrianglePosition);
Assert.Equal ("#0F0000", hex.Text);

cp.Focused.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (4, 0) });
cp.Focused.NewMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (4, 0) });

cp.Draw (); // Draw is needed to update TrianglePosition

Expand Down Expand Up @@ -1100,4 +1084,81 @@ public void ValueChanging_ReceivesOldAndNewValues ()
}

#endregion

#region ColorBar Mouse Binding Tests (issue #5143)

// Copilot

[Fact]
public void ColorBar_MousePress_UpdatesValue ()
{
// Regression: pressing on a bar must still update its value after the
// value-update logic was moved from OnMouseEvent to Command.Activate.
ColorPicker cp = GetColorPicker (ColorModel.RGB, false);
cp.Draw ();

ColorBar r = GetColorBar (cp, ColorPickerPart.Bar1);
Assert.Equal (0, r.Value);

// Position 10 is inside the bar (bar starts at X=2 for "R:" label, width 18).
r.NewMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (10, 0) });

Assert.True (r.Value > 0, "Value must increase when pressing inside the bar.");

cp.App?.Dispose ();
}

[Fact]
public void ColorBar_MouseEvent_CanBeCancelled ()
{
// Subscribing to MouseEvent and setting Handled=true must prevent the value
// update. Previously the update happened in OnMouseEvent before the event was
// raised, making cancellation impossible.
ColorPicker cp = GetColorPicker (ColorModel.RGB, false);
cp.Draw ();

ColorBar r = GetColorBar (cp, ColorPickerPart.Bar1);
Assert.Equal (0, r.Value);

r.MouseEvent += (_, e) => { e.Handled = true; };

r.NewMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (10, 0) });

Assert.Equal (0, r.Value);

cp.App?.Dispose ();
}

[Fact]
public void ColorBar_Drag_BoundedToOriginatingBar ()
{
// Dragging the mouse from one bar into another bar must not alter the second
// bar's value. GrabMouse in Command.Activate (not OnMouseEvent) ensures all subsequent
// drag events are routed to the bar where the press originated.
ColorPicker cp = GetColorPicker (ColorModel.RGB, false);
cp.Draw ();

ColorBar g = GetColorBar (cp, ColorPickerPart.Bar2);
Assert.Equal (0, g.Value);

// Press on the Red bar row (Y=0 in screen coords).
cp.App!.Mouse.RaiseMouseEvent (new Mouse
{
Flags = MouseFlags.LeftButtonPressed,
ScreenPosition = new Point (10, 0)
});

// Drag into the Green bar row (Y=1) – grab in Command.Activate must route events to Red bar only.
cp.App!.Mouse.RaiseMouseEvent (new Mouse
{
Flags = MouseFlags.LeftButtonPressed | MouseFlags.PositionReport,
ScreenPosition = new Point (10, 1)
});

Assert.Equal (0, g.Value);

cp.App?.Dispose ();
}

#endregion
}
Loading