Fixes #4050 - Select->Activate
#4126
Draft
Conversation
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
…(old implementation of WantContinuousButtonPressed)
…on firing twice immediately as mouse is pressed down.
BDisp
approved these changes
Jun 4, 2025
Collaborator
Author
|
I'm not convinced we don't still need a Code like this in // BUGBUG: OnCellActivated is misnamed, it should be OnCellAccepted? Or is it OnCellSelected?
// BUGBUG: Does this mean we still need Command.Select?
AddCommand (Command.Accept, () => OnCellActivated (new (Table, SelectedColumn, SelectedRow)));And TreeView: /// <summary>
/// <para>Triggers the <see cref="ObjectActivated"/> event with the <see cref="SelectedObject"/>.</para>
/// <para>This method also ensures that the selected object is visible.</para>
/// </summary>
/// <returns><see langword="true"/> if <see cref="ObjectActivated"/> was fired.</returns>
public bool? ActivateSelectedObjectIfAny (ICommandContext commandContext)
{
// By default, Command.Accept calls OnAccept, so we need to call it here to ensure that the event is fired.
if (RaiseAccepting (commandContext) == true)
{
return true;
}
T o = SelectedObject;
if (o is { })
{
// TODO: Should this be cancelable?
ObjectActivatedEventArgs<T> e = new (this, o);
OnObjectActivated (e);
return true;
}
return false;
}??? |
Collaborator
|
I think it make sense to have both. |
Updated `PopoverBaseImpl.cs` to clarify mouse event handling in popovers. Modified `View.Layout.cs` to adjust view count checks for active popovers. Corrected documentation in `ViewportSettingsFlags.cs` regarding the `TransparentMouse` flag. Added a new test method in `ApplicationPopoverTests.cs` to validate view retrieval under mouse coordinates when a popover is active, with multiple test cases included. See: gui-cs#4122
- Updated `MouseBinding` struct to include a new constructor for `MouseEventArgs`. - Enhanced documentation in `View.Mouse.cs` to clarify low-level API methods. - Refined command handling in `CheckBox` to improve state management. - Renamed method in `Label` for better clarity in hotkey handling. - Updated `command.md` to reflect changes in the `Command` system. - Restructured `mouse.md` for clearer understanding of mouse APIs and best practices.
Working through FlagSelector as an example.
Refactored the assignment of `Application.ForceDriver` and `_forceDriver` by removing the conditional check for null or empty values in `options.Driver`. The new implementation directly assigns `options.Driver`, simplifying the logic and assuming it is always valid.
Uncommented and activated `Command.Accept`, `Command.HotKey`, and `Command.Activate` in the `AddCommands` method of `Shortcut.cs`, enabling previously disabled functionality. Commented out test cases for `MouseFlags.Button2Pressed`, `MouseFlags.Button3Pressed`, and `MouseFlags.Button4Pressed` in `MouseTests.cs`, temporarily disabling these tests while retaining the `MouseFlags.Button1Pressed` test case.
Ensure mouse events are consistently routed to MouseGrabView. - Updated `MouseImpl.cs` to always set `viewRelativeMouseEvent.View` to `MouseGrabView`, addressing Issue gui-cs#4370. - Adjusted conditional logic to handle single-click events properly. - Added a new test case `MouseGrab_EventSentToGrabView_HasCorrectView` in `ApplicationMouseTests.cs` to validate the fix. - Documented behavior before and after the fix in the test case. These changes ensure correct event routing and improve test coverage.
The `[Fact]` attribute for the `MouseEvent_Test` method was updated to include a `Skip` parameter with the message "See Issue gui-cs#4370. Not gonna try to fix menu v1." This change temporarily disables the test while the issue remains unresolved. The `[AutoInitShutdown]` attribute is retained, ensuring the test can be re-enabled in the future.
The `Add` method in the `TimedEvents` class was updated to accept a `Timeout` object, call `AddTimeout` with `timeout.Span` and the `timeout` object, and return the `timeout` object. XML documentation comments were adjusted for consistency: - `<inheritdoc />` was changed to `<inheritdoc/>` in the `Add` method. - The `<inheritdoc/>` comment was removed from the `CheckTimers` method, with no functional changes to the method itself.
The namespace of the `OptionSelectorTests` class was updated from `UnitTests.ViewsTests` to `UnitTests_Parallelizable.ViewsTests`. This change reflects a reorganization or renaming of the namespace, potentially to indicate parallelizable tests or align with a new project structure. No functional changes were made to the test class itself.
Added `#nullable enable` to improve null safety and static analysis. Introduced the `Terminal.Gui.ViewBase` namespace to reorganize the codebase for better modularity. Marked the `View` class as `partial` to split its implementation across multiple files, enhancing maintainability. Added a private `_canFocus` field to manage focus state. Included `System.Diagnostics` for potential debugging or diagnostics.
Removed multiple redundant and outdated tests from `FlagSelectorTests` to streamline the test suite. These include tests for hotkey behavior, default constructors, and null value handling. Added a new test, `Mouse_Click_On_NotActivated_NoneFlag_Toggles`, to validate checkbox toggle behavior. Simplified the test structure by removing the `Mouse Tests` region and associated tests.
Refactored mouse event handling in `TableEditor` by replacing `MouseClick` with `MouseEvent` for more generic handling. Standardized command handling across `CheckBox`, `FlagSelector`, and `OptionSelector` by replacing `Command.Select` with `Command.Activate`. Updated associated methods and event handlers to align with the new activation model. Removed `OnSelecting` from `SelectorBase` and replaced its functionality with `Command.Activate`. Updated tests to reflect these changes, including replacing `MouseClick` with `MouseEvent` and simplifying test setups by removing `SetupFakeDriver`. Performed general refactoring for consistency, ensuring all methods and event names align with the new activation terminology.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v2_develop #4126 +/- ##
==============================================
- Coverage 74.41% 73.39% -1.02%
==============================================
Files 388 388
Lines 46568 46586 +18
Branches 6548 6540 -8
==============================================
- Hits 34653 34192 -461
- Misses 10053 10519 +466
- Partials 1862 1875 +13
... and 38 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes
Command.SelectandSelectingEvent toActivate#4050View.MouseraisesMouseClick3 times when it should just raise it once #4167View.OnMouseClick/MouseClickshould be deleted in favor ofActivating#4176Button.IsDefaultis confusing #4170Proposed Changes/Todos
Command.Select->ActivateSelectingevent toActivating,OnSelectingtoOnActivating, andRaiseSelectingtoRaiseActivatingin theViewclass and all derived classes (e.g.,Menuv2,MenuItemv2,CheckBox,FlagSelector).SetupCommandsinViewto useCommand.Activate.Shortcut.DispatchCommandand other command handlers to referenceActivating.Activatingvs.Accepting:OptionSelector&FlagSelector-> less duplicated code.ActivateandAcceptpropogate - esp w/inShortcutetc...For another PR
I was going to do this here, but I'll do it in another PR:
PropagateActivatingPropagateActivating, providing examples forMenuBarv2and potentialTopleveluse cases.RadioGroup-OptionSelectoris now a 100% compatible clone