Skip to content

Commit dfabab0

Browse files
Fixes tui-cs#5143. ColorBar mouse updates use MouseBindings with grab/ungrab (tui-cs#5289)
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>
1 parent 976c0be commit dfabab0

2 files changed

Lines changed: 134 additions & 34 deletions

File tree

Terminal.Gui/Views/Color/ColorBar.cs

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,47 @@ protected ColorBar ()
2626
AddCommand (Command.LeftStart, _ => SetZero ());
2727
AddCommand (Command.RightEnd, _ => SetMax ());
2828

29+
// Override Activate to handle mouse-press and drag. When triggered by a left-button press
30+
// (initial click or drag), extract the position from the MouseBinding context, update
31+
// Value, set focus, and grab the mouse so subsequent drag events are routed to this bar
32+
// exclusively. Grabbing here (rather than in OnMouseEvent) ensures that if a consumer
33+
// cancels the event by setting e.Handled=true in MouseEvent, the press command is never
34+
// reached and no grab occurs.
35+
AddCommand (
36+
Command.Activate,
37+
ctx =>
38+
{
39+
if (ctx?.Binding is MouseBinding { MouseEvent: { } mouse }
40+
&& mouse.Flags.FastHasFlags (MouseFlags.LeftButtonPressed))
41+
{
42+
UpdateValueFromMousePosition (mouse);
43+
SetFocus ();
44+
App?.Mouse.GrabMouse (this);
45+
46+
return true;
47+
}
48+
49+
return DefaultActivateHandler (ctx);
50+
});
51+
2952
KeyBindings.Add (Key.CursorLeft, Command.Left);
3053
KeyBindings.Add (Key.CursorRight, Command.Right);
3154
KeyBindings.Add (Key.CursorLeft.WithShift, Command.LeftExtend);
3255
KeyBindings.Add (Key.CursorRight.WithShift, Command.RightExtend);
3356
KeyBindings.Add (Key.Home, Command.LeftStart);
3457
KeyBindings.Add (Key.End, Command.RightEnd);
3558
MouseBindings.Remove (MouseFlags.LeftButtonClicked);
59+
60+
// Remove the base LeftButtonReleased → Activate binding so that releasing the mouse
61+
// does not fire DefaultActivateHandler (Activating/Activated). UngrabMouse is still
62+
// called from OnMouseEvent on release, so the grab lifecycle is unaffected.
63+
MouseBindings.Remove (MouseFlags.LeftButtonReleased);
64+
65+
// Bind press and drag to Activate so external code can fully suppress mouse interaction
66+
// by removing these two bindings (e.g. TerminalGuiDesigner editor mode). When both are
67+
// absent no value update, focus change, grab, or drag routing occurs.
68+
MouseBindings.Add (MouseFlags.LeftButtonPressed, Command.Activate);
69+
MouseBindings.Add (MouseFlags.LeftButtonPressed | MouseFlags.PositionReport, Command.Activate);
3670
}
3771

3872
/// <summary>
@@ -123,19 +157,24 @@ protected override bool OnDrawingContent (DrawContext? context)
123157
/// <inheritdoc/>
124158
protected override bool OnMouseEvent (Mouse mouse)
125159
{
126-
if (mouse.Flags.FastHasFlags (MouseFlags.LeftButtonPressed))
160+
// Release the grab on button-up so drag events stop routing exclusively to this bar.
161+
// The grab itself is established in the Command.Activate handler so that cancelling the
162+
// MouseEvent (e.Handled = true) prevents both the value update and the grab.
163+
if (mouse.IsReleased && App?.Mouse.IsGrabbed (this) == true)
127164
{
128-
if (mouse.Position!.Value.X >= _barStartsAt)
129-
{
130-
double v = MaxValue * ((double)mouse.Position!.Value.X - _barStartsAt) / (_barWidth - 1);
131-
Value = Math.Clamp ((int)v, 0, MaxValue);
132-
}
133-
SetFocus ();
134-
135-
// Do not mark as handled to allow Activating to be raised
165+
App.Mouse.UngrabMouse ();
136166
}
137167

138-
return mouse.Handled;
168+
return false;
169+
}
170+
171+
private void UpdateValueFromMousePosition (Mouse mouse)
172+
{
173+
if (mouse.Position is { } pos && pos.X >= _barStartsAt)
174+
{
175+
double v = MaxValue * ((double)pos.X - _barStartsAt) / (_barWidth - 1);
176+
Value = Math.Clamp ((int)v, 0, MaxValue);
177+
}
139178
}
140179

141180
/// <summary>

Tests/UnitTestsParallelizable/Views/ColorPickerTests.cs

Lines changed: 85 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ public void ClickingAtEndOfBar_SetsMaxValue ()
182182
cp.Draw (); // Draw is needed to update TrianglePosition
183183

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

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

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

246-
// Click on Green bar
246+
// Click on Green bar (press then release to complete the click cycle and release grab)
247247
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, ScreenPosition = new Point (0, 1) });
248-
249-
//cp.SubViews.OfType<GBar> ()
250-
// .Single ()
251-
// .OnMouseEvent (
252-
// new ()
253-
// {
254-
// Flags = MouseFlags.LeftButtonPressed,
255-
// Position = new (0, 1)
256-
// });
248+
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonReleased, ScreenPosition = new Point (0, 1) });
257249

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

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

262-
// Click on Blue bar
254+
// Click on Blue bar (press then release to complete the click cycle and release grab)
263255
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, ScreenPosition = new Point (0, 2) });
264-
265-
//cp.SubViews.OfType<BBar> ()
266-
// .Single ()
267-
// .OnMouseEvent (
268-
// new ()
269-
// {
270-
// Flags = MouseFlags.LeftButtonPressed,
271-
// Position = new (0, 2)
272-
// });
256+
cp.App!.Mouse.RaiseMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonReleased, ScreenPosition = new Point (0, 2) });
273257

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

@@ -706,14 +690,14 @@ public void RGB_MouseNavigation ()
706690

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

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

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

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

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

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

@@ -1100,4 +1084,81 @@ public void ValueChanging_ReceivesOldAndNewValues ()
11001084
}
11011085

11021086
#endregion
1087+
1088+
#region ColorBar Mouse Binding Tests (issue #5143)
1089+
1090+
// Copilot
1091+
1092+
[Fact]
1093+
public void ColorBar_MousePress_UpdatesValue ()
1094+
{
1095+
// Regression: pressing on a bar must still update its value after the
1096+
// value-update logic was moved from OnMouseEvent to Command.Activate.
1097+
ColorPicker cp = GetColorPicker (ColorModel.RGB, false);
1098+
cp.Draw ();
1099+
1100+
ColorBar r = GetColorBar (cp, ColorPickerPart.Bar1);
1101+
Assert.Equal (0, r.Value);
1102+
1103+
// Position 10 is inside the bar (bar starts at X=2 for "R:" label, width 18).
1104+
r.NewMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (10, 0) });
1105+
1106+
Assert.True (r.Value > 0, "Value must increase when pressing inside the bar.");
1107+
1108+
cp.App?.Dispose ();
1109+
}
1110+
1111+
[Fact]
1112+
public void ColorBar_MouseEvent_CanBeCancelled ()
1113+
{
1114+
// Subscribing to MouseEvent and setting Handled=true must prevent the value
1115+
// update. Previously the update happened in OnMouseEvent before the event was
1116+
// raised, making cancellation impossible.
1117+
ColorPicker cp = GetColorPicker (ColorModel.RGB, false);
1118+
cp.Draw ();
1119+
1120+
ColorBar r = GetColorBar (cp, ColorPickerPart.Bar1);
1121+
Assert.Equal (0, r.Value);
1122+
1123+
r.MouseEvent += (_, e) => { e.Handled = true; };
1124+
1125+
r.NewMouseEvent (new Mouse { Flags = MouseFlags.LeftButtonPressed, Position = new Point (10, 0) });
1126+
1127+
Assert.Equal (0, r.Value);
1128+
1129+
cp.App?.Dispose ();
1130+
}
1131+
1132+
[Fact]
1133+
public void ColorBar_Drag_BoundedToOriginatingBar ()
1134+
{
1135+
// Dragging the mouse from one bar into another bar must not alter the second
1136+
// bar's value. GrabMouse in Command.Activate (not OnMouseEvent) ensures all subsequent
1137+
// drag events are routed to the bar where the press originated.
1138+
ColorPicker cp = GetColorPicker (ColorModel.RGB, false);
1139+
cp.Draw ();
1140+
1141+
ColorBar g = GetColorBar (cp, ColorPickerPart.Bar2);
1142+
Assert.Equal (0, g.Value);
1143+
1144+
// Press on the Red bar row (Y=0 in screen coords).
1145+
cp.App!.Mouse.RaiseMouseEvent (new Mouse
1146+
{
1147+
Flags = MouseFlags.LeftButtonPressed,
1148+
ScreenPosition = new Point (10, 0)
1149+
});
1150+
1151+
// Drag into the Green bar row (Y=1) – grab in Command.Activate must route events to Red bar only.
1152+
cp.App!.Mouse.RaiseMouseEvent (new Mouse
1153+
{
1154+
Flags = MouseFlags.LeftButtonPressed | MouseFlags.PositionReport,
1155+
ScreenPosition = new Point (10, 1)
1156+
});
1157+
1158+
Assert.Equal (0, g.Value);
1159+
1160+
cp.App?.Dispose ();
1161+
}
1162+
1163+
#endregion
11031164
}

0 commit comments

Comments
 (0)