-
Notifications
You must be signed in to change notification settings - Fork 729
Description
Summary
The Command.Select, Selecting event, and related APIs (OnSelecting, RaiseSelecting) should be renamed to Command.Activate, Activating event, OnActivating, and RaiseActivating to improve clarity and consistency in the Command system. The term "select" is ambiguous for stateless views (e.g., Button, MenuItemv2, where it only sets focus) and less precise for non-list-based state changes (e.g., toggling a CheckBox). "Activate" better captures the action of changing a view’s state or preparing it for interaction, aligning with both stateful (e.g., CheckBox, FlagSelector, ListView) and stateless (e.g., Button, MenuItemv2) use cases while clearly distinguishing from Accepting.
Additionally, a targeted propagation model for Command.Activate is needed to support hierarchical components like MenuBarv2, which requires Activating events from subviews (e.g., MenuItemv2) to manage PopoverMenu visibility. To maintain decoupling, subviews should not specify propagation behavior. Instead, superviews should opt-in to receive propagated Activating events via a new PropagateActivating property in View, allowing MenuBarv2 to handle subview activations without subviews knowing superview details.
This issue is related to the planned Handled change in CommandEventArgs (issue #3913), which replaces Cancel with Handled to align with input event semantics.
Motivation
The current Command.Select and Selecting terminology has several issues:
- Ambiguity in Stateless Views: For views like
ButtonorMenuItemv2,Command.Selectonly sets focus, which isn’t typically considered “selection,” leading to developer confusion. - Imprecise for Non-List Contexts: In
CheckBox(toggling state) orFlagSelector(toggling flags), “select” doesn’t fully capture the state change, unlike list-based selection inListView. - Overlap with Accepting: In views like
ListVieworMenuv2,SelectingandAcceptingcan feel similar (e.g., Enter triggering both), requiring clearer distinction. - Propagation Needs:
MenuBarv2requiresActivatingevents from subviews to managePopoverMenuvisibility, but the current local handling model relies on view-specific events (e.g.,SelectedMenuItemChanged), which isn’t generalizable. Subviews should remain decoupled from superviews, avoiding knowledge of superview implementation details (e.g.,MenuBarv2’s popover logic).
The Activate terminology addresses these by:
- Representing both state changes (e.g., toggling a
CheckBox, selecting aListViewitem) and preparatory actions (e.g., focusing aButton, navigating aMenuItemv2). - Distinguishing clearly from
Accepting, which confirms actions (e.g., executing a menu command, submitting a dialog). - Supporting a targeted propagation model with
PropagateActivating, enabling superviews likeMenuBarv2to opt-in to subviewActivatingevents without coupling subviews to superview details.
Proposed Changes
1. Rename Command.Select and Related APIs to Activate
- Rename
Command.SelecttoCommand.Activatein theCommandenum. - Rename
Selectingevent toActivating,OnSelectingtoOnActivating, andRaiseSelectingtoRaiseActivatingin theViewclass and all derived classes (e.g.,Menuv2,MenuItemv2,CheckBox,FlagSelector). - Update related code:
- Modify
SetupCommandsinViewto useCommand.Activate. - Update
Shortcut.DispatchCommandand other command handlers to referenceActivating. - Ensure all event handlers and documentation reflect the new terminology.
- Modify
- Example:
public enum Command { Activate, // Previously Select Accept, HotKey, NotBound, // ... } // In View public event EventHandler<CommandEventArgs>? Activating; protected virtual bool OnActivating(CommandEventArgs args) { return false; } protected bool? RaiseActivating(ICommandContext? ctx) { CommandEventArgs args = new() { Context = ctx }; if (OnActivating(args) || args.Handled) { return true; } Activating?.Invoke(this, args); if (!args.Handled && SuperView?.PropagateActivating == true) { return SuperView.InvokeCommand(Command.Activate, ctx); } return Activating is null ? null : args.Handled; }
2. Introduce Targeted Propagation Model with PropagateActivating
- Add
bool PropagateActivatingtoViewto allow superviews (e.g.,MenuBarv2) to opt-in to receivingActivatingevents from subviews, defaulting tofalsefor local handling.public class View { // Existing properties and methods... public bool PropagateActivating { get; set; } // Opt-in to receive subview Activating events }
- Implement propagation in
RaiseActivating:- If
SuperView.PropagateActivatingistrueandargs.Handledisfalse, propagate to the superview. - Example: Propagate to
MenuBarv2for menu hierarchies.
- If
- Handle propagated events in
MenuBarv2:public class MenuBarv2 : Menuv2 { public MenuBarv2() { PropagateActivating = true; // Opt-in to subview Activating events // Other initialization... } protected override bool? RaiseActivating(ICommandContext? ctx) { if (ctx?.Source is MenuItemv2 menuItem && menuItem.SuperView is Menuv2 menu) { HideItem(GetActiveItem()); // Hide current popover ShowItem(menu.SuperMenuItem); // Show new popover return true; } return base.RaiseActivating(ctx); } }
- Apply to menu hierarchies initially, with potential extension to other components (e.g.,
Toplevelfor dialogs). Views likeCheckBoxandFlagSelectorretain local handling by default.
3. Update Documentation
- Clarify
Activatingvs.Accepting:Activating: State changes or interaction preparation (e.g., toggling aCheckBox, focusing aMenuItemv2, selecting aListViewitem).Accepting: Action confirmation (e.g., executing a menu command, submitting a dialog).- Example: “
Activatingis raised when aCheckBoxis toggled or aMenuItemv2is focused, whileAcceptingis raised when a menu command is executed or aButtonis activated.”
- Document
Handled: Emphasize thatHandledmeans “processed and complete,” aligning withKey.Handled. - Document propagation: Explain the targeted model with
PropagateActivating, providing examples forMenuBarv2and potentialTopleveluse cases. - Update view-specific docs:
Menuv2/MenuBarv2: HighlightActivatingfor focus/navigation andAcceptedfor action completion.CheckBox: ClarifyActivatingfor state toggling andAcceptingfor confirmation.FlagSelector: Address theSelecting/Acceptingconflation, recommending separation.
4. Address FlagSelector Design Flaw
- Issue:
FlagSelector’sCheckBox.Selectinghandler raises bothSelectingandAccepting, conflating state change and confirmation. - Fix: Limit
Selecting(to beActivating) to subview state changes, reservingAcceptingfor parent-level confirmation.checkbox.Activating += (sender, args) => { if (RaiseActivating(args.Context) is true) { args.Handled = true; } };
Impact
- Code Changes: Renaming affects
View,Shortcut,Menuv2,MenuItemv2,CheckBox,FlagSelector, and other views usingCommand.Select. Propagation requires addingPropagateActivatingtoViewand updatingRaiseActivating. - Documentation: Extensive updates to clarify
Activating/Accepting,Handled, and propagation. - Compatibility: Breaking change for users relying on
Command.Select/Selecting. Provide migration guidance in release notes. - Benefits:
- Improved clarity for stateless and stateful views.
- Enhanced flexibility for hierarchical components like
MenuBarv2while maintaining subview decoupling. - Better alignment with
Handledsemantics (issue IsDefault on button makes it click itself on close #3913).
TODO
[] - Rename Command.Select to Command.Activate in the Command enum.
[] - Rename Selecting event to Activating, OnSelecting to OnActivating, and RaiseSelecting to RaiseActivating in View and derived classes.
[] - Update all references to Command.Select/Selecting in the codebase (e.g., SetupCommands, Shortcut.DispatchCommand).
[] - Add bool PropagateActivating to View class, defaulting to false.
[] - Implement targeted propagation in RaiseActivating to check SuperView.PropagateActivating and propagate to the superview.
[] - Implement MenuBarv2 handling for propagated Activating events to manage PopoverMenu visibility, setting PropagateActivating = true.
[] - Refactor FlagSelector to separate Activating and Accepting in CheckBox.Selecting handler.
[] - Update unit tests for renamed APIs, propagation behavior, and FlagSelector fix.
[] - Revise Command system documentation to reflect Activate, Handled, and propagation.
[] - Update view-specific documentation for Menuv2, MenuBarv2, CheckBox, and FlagSelector.
[] - Document breaking changes and migration steps in release notes.
Related Issues
- IsDefault on button makes it click itself on close #3913: Replace
CancelwithHandledinCommandEventArgs.
Additional Context
This proposal is informed by analysis of Menuv2, MenuBarv2, CheckBox, and FlagSelector, which highlight:
Activate’s clarity for state changes (e.g.,CheckBoxtoggling,FlagSelectorflags) and preparation (e.g.,MenuItemv2focus).- The need for targeted propagation in
MenuBarv2to managePopoverMenuvisibility, with subviews decoupled from superview details. - The sufficiency of view-specific post-events (e.g.,
CheckedStateChanged,ValueChanged,SelectedMenuItemChanged) over genericActivated. - The value of
Acceptedin hierarchical views, suggesting inclusion inBarorToplevelrather thanView. - The
FlagSelectordesign flaw conflatingSelectingandAccepting, requiring a fix to align withActivating/Acceptingseparation.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status