Fixes #4473 - Fix Command system to support bubbling etc...#4620
Draft
tig wants to merge 121 commits intogui-cs:v2_developfrom
Draft
Fixes #4473 - Fix Command system to support bubbling etc...#4620tig wants to merge 121 commits intogui-cs:v2_developfrom
Command system to support bubbling etc...#4620tig wants to merge 121 commits intogui-cs:v2_developfrom
Conversation
- Add POST-GENERATION-VALIDATION.md with comprehensive checklist for validating AI-generated code before commits - Add formatting.md with detailed spacing, brace, and blank line rules - Update REFRESH.md to reference the validation checklist - Update CLAUDE.md with references to new validation docs These additions address the most common formatting violations that AI agents make when writing code for Terminal.Gui. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add clean-code-review.md task guide for reimplementing branches with narrative-quality commit histories - Remove scenario-modernization.md (outdated task) The clean code review task provides a workflow for AI agents to reimplement messy branch histories into clean, reviewer-friendly commit sequences suitable for PR review. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addresses: gui-cs#4473 (Menu etc. with Selectors are broken) - Add command-propagation-plan.md with phased implementation plan - Remove command-propagation-analysis.md (superseded by plan) The plan introduces PropagatedCommands property to enable opt-in command propagation from SubViews to SuperViews, solving the issue where commands from nested views (e.g., FlagSelector in MenuBar) don't reach their containing views. Phase breakdown: 1. Foundation - PropagatedCommands infrastructure 2. WeakReference - Lifecycle-safe CommandContext.Source 3. Shortcut propagation - Enable Command.Activate for Shortcuts 4. Test cleanup - Re-enable skipped tests 5. MenuBar propagation - Full menu hierarchy support (deferred) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addresses: gui-cs#4473 (Menu etc. with Selectors are broken) This commit implements Phases 1 and 2 of the command propagation plan: Phase 1 - PropagatedCommands Infrastructure: - Add PropagatedCommands property to View (default: [Command.Accept]) - Add PropagateCommand helper method for command bubbling - Refactor RaiseAccepting to use PropagateCommand helper - Reorganize View.Command.cs regions for clarity Phase 2 - WeakReference for CommandContext.Source: - Change ICommandContext.Source from View? to WeakReference<View>? - Update CommandContext to use WeakReference for lifecycle safety - Update InvokeCommand overloads to wrap 'this' in WeakReference - Update all views using ctx.Source to use TryGetTarget pattern: - Dialog, DialogTResult - MenuBar, PopoverMenu - ComboBox, OptionSelector - ScrollSlider, Shortcut The PropagatedCommands property enables opt-in command propagation: when a SubView raises a command that isn't handled and the command is in the SuperView's PropagatedCommands list, the command bubbles up. BREAKING CHANGE: ICommandContext.Source is now WeakReference<View>? Code accessing ctx.Source must use TryGetTarget pattern: if (ctx?.Source?.TryGetTarget (out View? view) == true) { ... } Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a standalone test application for verifying Command.Activate propagation through the Shortcut → Window hierarchy. The example demonstrates: - CheckBox (CanFocus=false) in Shortcut → propagates to Window - CheckBox (CanFocus=true) in Shortcut → propagates to Window - Button in Shortcut → propagates to Window This serves as a manual test bed for the PropagatedCommands feature and helps verify that command context (Source) is properly preserved when commands bubble up through the view hierarchy. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add tests to verify PropagatedCommands and WeakReference changes: CommandPropagationTests.cs (NEW - 588 lines): - Tests for command propagation through view hierarchies - Tests for WeakReference lifecycle safety - Tests for PropagatedCommands customization - Tests for Command.Accept and Command.Activate propagation ViewCommandTests.cs (updated): - Add tests for PropagatedCommands default value - Add tests for PropagateCommand helper behavior - Add tests for IsDefault button propagation CommandContextTests.cs (updated): - Update tests for WeakReference-based Source property - Add TryGetTarget pattern tests InputBindingTests.cs (updated): - Update tests for WeakReference compatibility All tests follow Terminal.Gui patterns and are added to UnitTestsParallelizable for parallel execution. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
command.md: - Document PropagatedCommands property - Explain command propagation behavior - Add examples for customizing propagated commands events.md: - Add section on command events and propagation - Document CommandEventArgs and ICommandContext - Explain WeakReference pattern for Source property Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update scenarios to use TryGetTarget pattern for accessing CommandContext.Source after the WeakReference change: Menus.cs: - Update command handlers to use TryGetTarget MouseTester.cs: - Update mouse event handlers for new pattern UICatalogRunnable.cs: - Update command context handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change absolute path to relative path for cross-machine compatibility. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v2_develop #4620 +/- ##
==============================================
- Coverage 77.35% 76.09% -1.26%
==============================================
Files 444 447 +3
Lines 44992 44991 -1
Branches 6536 6583 +47
==============================================
- Hits 34805 34238 -567
- Misses 8296 8806 +510
- Partials 1891 1947 +56
... and 38 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
tig
changed the title
tig
changed the title
Clarify and strengthen event forwarding between Shortcut and its CommandView, ensuring Activating and Accepting events propagate correctly without recursion. MouseHighlightStates now defaults to MouseState.In for better UI feedback. Refactor property implementations and clean up code. Add comprehensive unit tests verifying CheckBox state changes, event propagation, and default behaviors. Enhance ShortcutTestWindow to log detailed event info and demonstrate propagation scenarios.
- Updated Key equality operators to handle nulls safely and avoid NullReferenceExceptions. - ComboBox Accept command now uses non-nullable View in TryGetTarget and passes a WeakReference<View> to CommandContext for consistency. - Shortcut.Key property now always returns Key.Empty if unset, and setter no longer throws on null. - Simplified GetWidthDimAuto by removing unnecessary null-forgiving operators. - Fixed CommandViewOnActivating logic to prevent recursion and ensure correct event propagation. - Cleaned up redundant key initialization and clarified KeyView.Text assignment. - Overall, these changes improve code safety, event handling, and clarity.
Refactored the Shortcut class to replace explicit backing fields (_alignmentModes and _minimumKeyTextSize) with auto-implemented properties for AlignmentModes and MinimumKeyTextSize. Set default values directly in the property declarations and used the C# field keyword in setters. This streamlines property definitions and simplifies the codebase.
…/Terminal.Gui into copilot/fix-command-propagation-issue-clean
…/Terminal.Gui into copilot/fix-command-propagation-issue-clean
Improve command/event handling for menus and shortcuts Refactor Shortcut, Menu, MenuBar, and PopoverMenu to enhance command propagation, event handling, and resource management. Set PropagatedCommands by default, improve design-time support, and clarify event subscriptions. Add helper methods for event origin detection, enforce proper menu hierarchy, and update documentation and logging for better maintainability and debugging.
Update SelectorBase to use IsBubblingDown guard for BubbleDown, allowing programmatic InvokeCommand calls and preventing recursion. Remove OnHandlingHotKey override to avoid double-Activate bugs. Add custom HotKey handler to FlagSelector: restores focus, no flag toggle when focused. Rewrite OptionSelector and FlagSelector tests for correct HotKey/Activate behavior. Update command.md docs and table to match keyboard spec. Skip unrelated Menu HotKey test.
tig
force-pushed
the
copilot/fix-command-propagation-issue-clean
branch
from
c10c405 to
608ac79
Compare
…/Terminal.Gui into copilot/fix-command-propagation-issue-clean
Update OnAccepted to use ICommandContext instead of CommandEventArgs across View and related classes. Revise all overrides and usages in Dialogs, CharMap, MenuBar, MenuItem, Prompt, OptionSelector, SelectorBase, Shortcut, and tests. Add logging to command handlers for improved debugging. Improve OptionSelector activation and cycling logic. Modernize and streamline command event handling throughout the codebase.
Refactor context menu lifecycle in TextField and TextView to create, register, and dispose menus on focus changes, improving resource management and preventing leaks. Update mouse event handling to avoid null reference errors. In debug builds, assign unique Ids to context menus for easier debugging. In FlagSelector, streamline checkbox event handling by attaching handlers in OnSubViewAdded and removing the _updatingChecked flag. Move activation and value change logic to dedicated methods for clarity. Clean up redundant and commented code. Update SelectorBase to prevent double event bubbling. These changes enhance robustness, maintainability, and debuggability of UI components.
…/Terminal.Gui into pr-4620
tig
changed the title
Command system to support bubbling etc...
Updated Dialog<TResult> to use command binding source for Accept actions. Added and revised tests in ViewCommandTests and DialogTests to verify Accept command bubbling to DefaultAcceptView and dialog acceptance from DatePicker, including event count assertions. Adjusted test expectations for new bubbling behavior.
- Always forward Accept to DefaultAcceptView after Accepting, regardless of bubbling source - Dialog sets Result based on pressed button or default before base Accepting - Use command context Source consistently in Accepting logic - Update tests to reflect new Accept bubbling and event firing behavior - Ensure dialog Accepted event is not fired for handled Cancel actions - Improves consistency and clarity of Accept handling and event order
Refactored View, Dialog, and Wizard command logic to ensure Accept/Enter key events propagate correctly and dialogs are properly accepted. Updated input injection and dialog acceptance tests for accuracy. Removed obsolete ProgressBar and SpinnerView command tests. Code refactored for clarity and maintainability.
Align FlagSelector with spec: hotkey now only restores focus and never toggles value, even when focused. Refactor activation logic to ensure correct event bubbling and value changes, especially for user interactions and programmatic activation. Update tests to reflect new double-click behavior. Minor code cleanups and documentation improvements.
HotKey now only restores focus and never toggles the flag value. A new Command.HotKey handler is added to skip activation. OnHandlingHotKey is simplified to be a no-op when focused. Comments and documentation are updated for clarity.
Refactored HotKey handling in FlagSelector to properly restore focus without toggling when not focused, using a new _suppressHotKeyActivate flag and method overrides. Removed the old AddCommand HotKey handler. Also fixed logic for removing the "None" checkbox to only do so when ShowNoneFlag is not set, and cleaned up related redundant code.
Previously, typing a character matching the TextField's HotKey (e.g., 'E' in "_Enter Path") would trigger the HotKey even when the TextField was focused, preventing normal text input. Now, when the TextField has focus, the HotKey is canceled and the character is inserted as expected. This is achieved by overriding OnHandlingHotKey in TextField and updating the View base logic to allow key processing when the HotKey is canceled. Includes code cleanups, improved null checks, and a new unit test to verify correct behavior. Minor formatting and style improvements are also included.
Removed Space key binding from PopoverMenu to allow text input. Improved TextView hotkey handling so Space and other printable characters are not consumed by Command.Activate. Added unit tests to verify correct insertion of all printable characters and proper Space key behavior. Clarified hotkey handling differences between TextField and TextView.
- Uncomment BubbleDown conditionals in OnActivating and OnAccepting so commands only bubble down when triggered by a binding (not programmatic) and the source is not the CommandView itself - Remove incorrect InvokeCommand(Activate) from OnAccepting, keeping Accept and Activate as separate command paths per design - Add custom Activate handler that returns false when IsBubblingUp is true, allowing SubViews like CheckBox to complete their own activation - Split ShortcutTests into partial classes (Command, KeyDown, Mouse) - Add 5 new tests covering Accept/Activate separation, BubbleDown via Space key, Enter key Accept path, and Action invocation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Don't set Handled when activation comes from the CommandView so ColorPicker16.OnActivated runs and picks the color from the mouse position. Only set Handled when cycling via HotKey or other sources. Fix the matching code example in shortcut.md which had the logic inverted. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Menu, add tests - Move TargetView and Command properties from MenuItem down to Shortcut base class - Move OnAccepted logic (TargetView invocation) from MenuItem to Shortcut - Fix Menu.OnSubViewAdded: replace AddCommand with Accepting event (collision fix) - Remove dead code from MenuItem (~65 lines) and Menu (OnAccepting, OnVisibleChanged) - Simplify Menu.SelectedMenuItem to getter-only property - Add 9 Shortcut.Command tests, 8 MenuItem tests, 10 Menu tests, 5 Bar tests - Update command.md with Shortcut/MenuItem/Menu/Bar command table rows - Add menus-refactor.md plan for upcoming MenuBar/PopoverMenu work - Clean up stale plan files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mouse clicks on a MenuBarItem land on its CommandView SubView, so the event source is CommandView, not MenuBarItem. Added OnActivating override that walks the SuperView chain via FindMenuBarItemForSource to find the containing MenuBarItem and toggle its PopoverMenu. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Added MenuItem click non-functional issue to Phase 6d - Added Phase 8: fix integration tests (MenuBar/PopoverMenu, FileDialog) - Updated execution order and verification steps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Command Bubbling Refactor - Progress Tracker
Branch:
pr-4620Last Updated: 2026-02-14
Status Summary
STREAM 2: View Command Framework — DONE
File:
Terminal.Gui/ViewBase/View.Command.csAll planned features implemented:
CommandsToBubbleUpproperty (read-onlyIReadOnlyList<Command>)DefaultAcceptViewproperty with auto-detection ofIAcceptTarget+IsDefaultTryBubbleUpToSuperView()— checks DefaultAcceptView, then SuperView.CommandsToBubbleUp, then PaddingBubbleDown(target, ctx)— dispatches commands downward withIsBubblingDown=trueRaiseAccepting/RaiseActivating/RaiseHandlingHotKey— cancelable, with bubblingRaiseAccepted/RaiseActivated— non-cancelable post-eventsDefaultAcceptHandler/DefaultActivateHandler/DefaultHotKeyHandlerICommandContextwithIsBubblingUp,IsBubblingDown,Source(WeakReference),BindingCommandEventArgsusesHandled(fromHandledEventArgs)STREAM 3: View-Specific Changes
3.1 Button — DONE
File:
Terminal.Gui/Views/Button.csIAcceptTargetinterfaceCommand.Accept(was HotKey)Command.Accept(was HotKey)Command.AcceptMouseHoldRepeatsupportOnHotKeyCommandoverride invokesCommand.Accept3.2 Shortcut — DONE
File:
Terminal.Gui/Views/Shortcut.csCommandsToBubbleUp = [Command.Activate, Command.Accept]OnActivatingoverride — BubbleDown to CommandView (conditional: binding exists and source != CommandView)OnAcceptingoverride — BubbleDown to CommandView (same conditional)OnActivated— invokes ActionOnAccepted— invokes Action3.3 Dialog — DONE
Files:
Terminal.Gui/Views/Dialog.cs,Terminal.Gui/Views/DialogTResult.csCommandsToBubbleUp = [Command.Accept](in DialogTResult)OnActivatingoverride — sets Result from button indexOnAcceptingoverride — handles non-default button detectionOnAcceptedoverride — callsRequestStopafter button pressDefaultAcceptViewused for IsDefault button routing3.4 Menu System — IN PROGRESS
Files:
Terminal.Gui/Views/Menu/Menu.cs,MenuBar.cs,MenuItem.cs,PopoverMenu.csDone:
Menu.CommandsToBubbleUp = [Command.Accept, Command.Activate]Not Done (active work):
Menu.OnAcceptingis fully commented out (~25 lines, lines 107-136)3.5 TextView — DONE
File:
Terminal.Gui/Views/TextInput/TextView/TextView.Commands.csMultiline ? Command.NewLine : Command.AcceptProcessEnterKeycallsRaiseAcceptingwhenEnterKeyAddsLine=false3.6 TextField — DONE
File:
Terminal.Gui/Views/TextInput/TextField/TextField.Commands.csCommand.Accept(default)3.7 SelectorBase — DONE
File:
Terminal.Gui/Views/Selectors/SelectorBase.csCommandsToBubbleUp = [Command.Activate, Command.Accept]OnAcceptingoverride3.8 FlagSelector — DONE
STREAM 4: Testing — MOSTLY DONE
Key test files:
Tests/UnitTestsParallelizable/Views/ShortcutTests.Command.cs— comprehensiveTests/UnitTestsParallelizable/ViewBase/CommandBubblingTests.cs— hierarchy testsTests/UnitTestsParallelizable/ViewBase/CommandContextTests.csTests/UnitTestsParallelizable/ViewBase/ViewCommandTests.csRemaining:
[Fact(Skip = "...")]— need to be enabled once Menu work completesSTREAM 5: Documentation — DONE
File:
docfx/docs/command.mdComprehensive coverage including:
Remaining Work
Must Complete (Menu System) — see menu-refactor.md
Menu.OnAccepting— uncomment/rewrite the commented-out logic:Verify Before Merge
dotnet test Tests/UnitTestsParallelizable)[Fact(Skip=...)]tests resolved (enabled or removed with rationale)