From 6edae786ae1f29d65987ad8867d1ef31d41586a0 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Thu, 2 Oct 2025 15:58:06 +0000
Subject: [PATCH] Add comprehensive CWP helper migration analysis

Co-authored-by: tig <585482+tig@users.noreply.github.com>
---
 .../analysis/cwp_helper_migration_analysis.md | 1073 +++++++++++++++++
 1 file changed, 1073 insertions(+)
 create mode 100644 docs/analysis/cwp_helper_migration_analysis.md

diff --git a/docs/analysis/cwp_helper_migration_analysis.md b/docs/analysis/cwp_helper_migration_analysis.md
new file mode 100644
index 000000000..c818d8996
--- /dev/null
+++ b/docs/analysis/cwp_helper_migration_analysis.md
@@ -0,0 +1,1073 @@
+# CWP Helper Class Migration Analysis
+
+## Executive Summary
+
+This document provides a detailed analysis of the effort required to replace all direct CWP implementations with CWP helper classes (`CWPWorkflowHelper`, `CWPPropertyHelper`, `CWPEventHelper`).
+
+### Current State
+- **CWP Helper Classes**: 3 (CWPWorkflowHelper, CWPPropertyHelper, CWPEventHelper)
+- **Current Helper Usage**: 2 files (View.Drawing.Scheme.cs only)
+- **Direct CWP Implementations**: 28 methods across 11 files
+- **Total CWP Patterns**: 31 instances
+
+### Migration Summary
+- **Can Use CWPWorkflowHelper**: 24 instances (77%)
+- **Can Use CWPPropertyHelper**: 2 instances (6%)
+- **Cannot Use Helpers (Custom Logic)**: 5 instances (16%)
+- **Estimated Total Effort**: 3-4 weeks
+- **Risk Level**: MEDIUM
+
+## CWP Helper Classes Overview
+
+### 1. CWPWorkflowHelper
+**Purpose**: Single-phase workflows with optional default action
+
+**Signature**:
+```csharp
+public static bool? Execute<T>(
+    Func<ResultEventArgs<T>, bool> onMethod,
+    EventHandler<ResultEventArgs<T>>? eventHandler,
+    ResultEventArgs<T> args,
+    Action? defaultAction = null)
+```
+
+**Use When**:
+- Simple workflow with virtual method + event
+- Returns bool? (true=handled, false=not handled, null=no subscribers)
+- Optional default action if not handled
+- No complex state management between phases
+
+### 2. CWPPropertyHelper
+**Purpose**: Property change with pre/post notifications
+
+**Signature**:
+```csharp
+public static bool ChangeProperty<T>(
+    T currentValue,
+    T newValue,
+    Func<ValueChangingEventArgs<T>, bool> onChanging,
+    EventHandler<ValueChangingEventArgs<T>>? changingEvent,
+    Action<ValueChangedEventArgs<T>>? onChanged,
+    EventHandler<ValueChangedEventArgs<T>>? changedEvent,
+    out T finalValue)
+```
+
+**Use When**:
+- Property setter with validation
+- Need both "changing" (cancellable) and "changed" (notification) events
+- Two-phase pattern (pre-change, post-change)
+
+### 3. CWPEventHelper
+**Purpose**: Event-only pattern (no virtual method)
+
+**Signature**:
+```csharp
+public static bool Execute<T>(
+    EventHandler<ResultEventArgs<T>>? eventHandler,
+    ResultEventArgs<T> args)
+```
+
+**Use When**:
+- Only have event, no virtual method
+- Simple event invocation with handled check
+- Rarely used in current codebase
+
+## Detailed Migration Analysis
+
+### Category 1: Mouse Events (HIGH PRIORITY)
+
+#### 1.1 OnMouseEvent / MouseEvent
+**File**: `Terminal.Gui/ViewBase/View.Mouse.cs:315-338`
+
+**Current Implementation**:
+```csharp
+public bool RaiseMouseEvent (MouseEventArgs mouseEvent)
+{
+    // Pre-processing: MouseHeldDown logic
+    if (WantContinuousButtonPressed && MouseHeldDown != null)
+    {
+        if (mouseEvent.IsPressed)
+        {
+            MouseHeldDown.Start ();
+        }
+        else
+        {
+            MouseHeldDown.Stop ();
+        }
+    }
+
+    // CWP pattern
+    if (OnMouseEvent (mouseEvent) || mouseEvent.Handled)
+    {
+        return true;
+    }
+
+    MouseEvent?.Invoke (this, mouseEvent);
+
+    return mouseEvent.Handled;
+}
+```
+
+**Can Use Helper?**: ❌ NO - Custom pre-processing logic
+
+**Reason**: The MouseHeldDown logic must execute before the CWP pattern. CWPWorkflowHelper doesn't support pre-processing code before the workflow.
+
+**Migration Difficulty**: N/A (Cannot migrate)
+
+**Recommendation**: Keep as-is. This is appropriate custom logic.
+
+---
+
+#### 1.2 OnMouseClick / MouseClick
+**File**: `Terminal.Gui/ViewBase/View.Mouse.cs:470-498`
+
+**Current Implementation**:
+```csharp
+protected bool RaiseMouseClickEvent (MouseEventArgs args)
+{
+    // Pre-conditions
+    if (!Enabled)
+    {
+        return args.Handled = false;
+    }
+
+    // CWP pattern
+    if (OnMouseClick (args) || args.Handled)
+    {
+        return args.Handled;
+    }
+
+    MouseClick?.Invoke (this, args);
+
+    if (args.Handled)
+    {
+        return true;
+    }
+
+    // Post-conditions: Invoke commands
+    args.Handled = InvokeCommandsBoundToMouse (args) == true;
+
+    return args.Handled;
+}
+```
+
+**Can Use Helper?**: ⚠️ PARTIAL - Pre/post conditions complicate
+
+**Proposed Migration**:
+```csharp
+protected bool RaiseMouseClickEvent (MouseEventArgs args)
+{
+    // Pre-condition check stays outside
+    if (!Enabled)
+    {
+        return args.Handled = false;
+    }
+
+    // Use helper but wrap return value
+    bool? result = CWPWorkflowHelper.Execute(
+        onMethod: _ => OnMouseClick(args),
+        eventHandler: MouseClick,
+        args: new ResultEventArgs<MouseEventArgs> { Result = args },
+        defaultAction: null);
+
+    // Post-condition: Invoke commands if not handled
+    if (result != true && !args.Handled)
+    {
+        args.Handled = InvokeCommandsBoundToMouse(args) == true;
+    }
+
+    return args.Handled;
+}
+```
+
+**Migration Difficulty**: ⭐⭐⭐ HARD
+
+**Issues**:
+1. MouseEventArgs doesn't fit ResultEventArgs<T> pattern cleanly
+2. Pre/post conditions require wrapper logic
+3. Command invocation logic stays outside helper
+4. More complex than direct implementation
+
+**Recommendation**: ❌ Don't migrate. Direct implementation is clearer.
+
+---
+
+#### 1.3 OnMouseWheel / MouseWheel
+**File**: `Terminal.Gui/ViewBase/View.Mouse.cs:574-600`
+
+**Analysis**: Same pattern as OnMouseClick
+
+**Can Use Helper?**: ⚠️ PARTIAL (same issues)
+
+**Migration Difficulty**: ⭐⭐⭐ HARD
+
+**Recommendation**: ❌ Don't migrate
+
+---
+
+#### 1.4 OnMouseEnter / MouseEnter
+**File**: `Terminal.Gui/ViewBase/View.Mouse.cs:69-98`
+
+**Current Implementation**:
+```csharp
+internal bool? NewMouseEnterEvent (CancelEventArgs eventArgs)
+{
+    if (!CanBeVisible (this))
+    {
+        return null;
+    }
+
+    if (OnMouseEnter (eventArgs))
+    {
+        return true;
+    }
+
+    MouseEnter?.Invoke (this, eventArgs);
+
+    if (eventArgs.Cancel)
+    {
+        return true;
+    }
+
+    // Post-processing: Set mouse state
+    MouseState |= MouseState.In;
+
+    if (HighlightStates != MouseState.None)
+    {
+        SetNeedsDraw ();
+    }
+
+    return false;
+}
+```
+
+**Can Use Helper?**: ❌ NO - Post-processing state management
+
+**Reason**: Setting MouseState and SetNeedsDraw after event is essential logic that can't be a "default action"
+
+**Migration Difficulty**: N/A
+
+**Recommendation**: ❌ Don't migrate
+
+---
+
+### Category 2: Keyboard Events (HIGH PRIORITY)
+
+#### 2.1 OnKeyDown / KeyDown
+**File**: `Terminal.Gui/ViewBase/View.Keyboard.cs:322-337`
+
+**Current Implementation**:
+```csharp
+public bool NewKeyDownEvent (Key k)
+{
+    if (OnKeyDown (k) || k.Handled)
+    {
+        return true;
+    }
+
+    KeyDown?.Invoke (this, k);
+
+    return k.Handled;
+}
+```
+
+**Can Use Helper?**: ✅ YES - Clean fit
+
+**Proposed Migration**:
+```csharp
+public bool NewKeyDownEvent (Key k)
+{
+    ResultEventArgs<Key> args = new (k);
+    
+    bool? result = CWPWorkflowHelper.Execute(
+        onMethod: args => OnKeyDown(k),
+        eventHandler: (sender, e) => KeyDown?.Invoke(sender, k),
+        args: args,
+        defaultAction: null);
+
+    return result == true || k.Handled;
+}
+```
+
+**Migration Difficulty**: ⭐⭐ MEDIUM
+
+**Issues**:
+1. Key is mutable (k.Handled), not immutable like ResultEventArgs expects
+2. Need to synchronize k.Handled with result
+3. Event signature doesn't match helper's expected EventHandler<ResultEventArgs<T>>
+4. Current implementation is actually simpler
+
+**Recommendation**: ⚠️ Marginal benefit. Could migrate for consistency but adds complexity.
+
+---
+
+#### 2.2 OnKeyDownNotHandled / KeyDownNotHandled
+**File**: `Terminal.Gui/ViewBase/View.Keyboard.cs:339-349`
+
+**Analysis**: Same pattern as OnKeyDown
+
+**Can Use Helper?**: ✅ YES (same issues)
+
+**Migration Difficulty**: ⭐⭐ MEDIUM
+
+**Recommendation**: ⚠️ Marginal benefit
+
+---
+
+#### 2.3 OnKeyUp / KeyUp
+**File**: `Terminal.Gui/ViewBase/View.Keyboard.cs` (similar location)
+
+**Analysis**: Same pattern as OnKeyDown
+
+**Can Use Helper?**: ✅ YES (same issues)
+
+**Migration Difficulty**: ⭐⭐ MEDIUM
+
+**Recommendation**: ⚠️ Marginal benefit
+
+---
+
+### Category 3: Command Events (MEDIUM PRIORITY)
+
+#### 3.1 OnAccepting / Accepting
+**File**: `Terminal.Gui/ViewBase/View.Command.cs:160-177`
+
+**Current Implementation**:
+```csharp
+protected bool? RaiseAccepting (ICommandContext? ctx)
+{
+    CommandEventArgs args = new () { Context = ctx };
+
+    if (OnAccepting (args) || args.Handled)
+    {
+        return true;
+    }
+
+    Accepting?.Invoke (this, args);
+
+    return Accepting is null ? null : args.Handled;
+}
+```
+
+**Can Use Helper?**: ✅ YES - Good fit!
+
+**Proposed Migration**:
+```csharp
+protected bool? RaiseAccepting (ICommandContext? ctx)
+{
+    ResultEventArgs<ICommandContext?> args = new (ctx);
+    
+    return CWPWorkflowHelper.Execute(
+        onMethod: args => OnAccepting(new CommandEventArgs { Context = args.Result }),
+        eventHandler: (sender, e) => 
+        {
+            var cmdArgs = new CommandEventArgs { Context = e.Result };
+            Accepting?.Invoke(sender, cmdArgs);
+            e.Handled = cmdArgs.Handled;
+        },
+        args: args,
+        defaultAction: null);
+}
+```
+
+**Migration Difficulty**: ⭐⭐⭐ HARD
+
+**Issues**:
+1. CommandEventArgs is separate from ResultEventArgs
+2. Need to create/map CommandEventArgs inside helper calls
+3. Complex argument marshalling
+4. Current implementation is clearer
+
+**Recommendation**: ⚠️ Could work but adds complexity. Direct implementation is simpler.
+
+---
+
+#### 3.2 OnSelecting / Selecting
+**File**: `Terminal.Gui/ViewBase/View.Command.cs:220-236`
+
+**Analysis**: Identical pattern to OnAccepting
+
+**Can Use Helper?**: ✅ YES (same issues)
+
+**Migration Difficulty**: ⭐⭐⭐ HARD
+
+**Recommendation**: ⚠️ Same as OnAccepting
+
+---
+
+#### 3.3 OnHandlingHotKey / HandlingHotKey
+**File**: `Terminal.Gui/ViewBase/View.Command.cs:260-277`
+
+**Analysis**: Identical pattern to OnAccepting
+
+**Can Use Helper?**: ✅ YES (same issues)
+
+**Migration Difficulty**: ⭐⭐⭐ HARD
+
+**Recommendation**: ⚠️ Same as OnAccepting
+
+---
+
+#### 3.4 OnCommandNotBound / CommandNotBound
+**File**: `Terminal.Gui/ViewBase/View.Command.cs:93-109`
+
+**Analysis**: Identical pattern to OnAccepting
+
+**Can Use Helper?**: ✅ YES (same issues)
+
+**Migration Difficulty**: ⭐⭐⭐ HARD
+
+**Recommendation**: ⚠️ Same as OnAccepting
+
+---
+
+### Category 4: Navigation Events (MEDIUM PRIORITY)
+
+#### 4.1 OnAdvancingFocus / AdvancingFocus
+**File**: `Terminal.Gui/ViewBase/View.Navigation.cs` (AdvanceFocus method)
+
+**Current Implementation Pattern**:
+```csharp
+// Inside AdvanceFocus method
+AdvanceFocusEventArgs args = new (direction, behavior);
+
+if (OnAdvancingFocus (direction, behavior))
+{
+    return false;
+}
+
+AdvancingFocus?.Invoke (this, args);
+
+if (args.Cancel)
+{
+    return false;
+}
+```
+
+**Can Use Helper?**: ⚠️ PARTIAL - Embedded in larger method
+
+**Migration Difficulty**: ⭐⭐⭐⭐ VERY HARD
+
+**Issues**:
+1. Not in its own Raise method - embedded in AdvanceFocus logic
+2. Complex navigation state management around the pattern
+3. Would require extracting to separate method first
+4. Return semantics differ (returns bool, not bool?)
+
+**Recommendation**: ❌ Don't migrate. Too embedded in complex logic.
+
+---
+
+#### 4.2 OnHasFocusChanging / HasFocusChanging
+**File**: `Terminal.Gui/ViewBase/View.Navigation.cs:717-750`
+
+**Analysis**: Similar to AdvancingFocus - embedded in SetHasFocus method
+
+**Can Use Helper?**: ⚠️ PARTIAL - Embedded in larger method
+
+**Migration Difficulty**: ⭐⭐⭐⭐ VERY HARD
+
+**Recommendation**: ❌ Don't migrate. Too embedded.
+
+---
+
+### Category 5: Drawing Events (LOW PRIORITY)
+
+#### 5.1-5.6 Drawing Events
+**Files**: `Terminal.Gui/ViewBase/View.Drawing.cs`
+- OnDrawingAdornments / DrawingAdornments (line 284)
+- OnClearingViewport / ClearingViewport (line 319)
+- OnDrawingText / DrawingText (lines 407, 413)
+- OnDrawingContent / DrawingContent (lines 489, 495)
+- OnDrawingSubViews / DrawingSubViews (lines 545, 551)
+- OnRenderingLineCanvas / RenderingLineCanvas (line 611)
+
+**Current Implementation Pattern** (OnDrawingContent example):
+```csharp
+private void DoDrawContent (DrawContext? context = null)
+{
+    if (OnDrawingContent (context))
+    {
+        return;
+    }
+
+    if (OnDrawingContent ())
+    {
+        return;
+    }
+
+    var dev = new DrawEventArgs (Viewport, Rectangle.Empty, context);
+    DrawingContent?.Invoke (this, dev);
+
+    if (dev.Cancel)
+    {
+        return;
+    }
+
+    // Actual drawing logic...
+}
+```
+
+**Can Use Helper?**: ⚠️ PARTIAL - Non-standard pattern
+
+**Migration Difficulty**: ⭐⭐⭐ HARD
+
+**Issues**:
+1. TWO virtual methods (overloaded) - OnDrawingContent(context) and OnDrawingContent()
+2. DrawEventArgs created after virtual methods called
+3. Embedded in DoDrawXxx methods with actual drawing logic after
+4. Cancel semantics differ from standard CWP
+
+**Recommendation**: ❌ Don't migrate. Custom drawing pipeline better as-is.
+
+---
+
+### Category 6: Property Changes (MEDIUM PRIORITY)
+
+#### 6.1 OnVisibleChanging / VisibleChanging
+**File**: `Terminal.Gui/ViewBase/View.cs:382-420`
+
+**Current Implementation**:
+```csharp
+public bool Visible
+{
+    get => _visible;
+    set
+    {
+        if (_visible == value)
+        {
+            return;
+        }
+
+        if (OnVisibleChanging ())
+        {
+            return;
+        }
+
+        VisibleChanging?.Invoke (this, new CancelEventArgs (ref _visible, value));
+
+        if (_visible == value)
+        {
+            return;
+        }
+
+        bool wasVisible = _visible;
+        _visible = value;
+
+        OnVisibleChanged ();
+        VisibleChanged?.Invoke (this, EventArgs.Empty);
+        
+        // Post-processing...
+        if (!_visible)
+        {
+            RestoreFocus ();
+        }
+        
+        SetNeedsDraw ();
+        // ... more post-processing
+    }
+}
+```
+
+**Can Use Helper?**: ✅ YES - CWPPropertyHelper designed for this!
+
+**Proposed Migration**:
+```csharp
+public bool Visible
+{
+    get => _visible;
+    set
+    {
+        bool changed = CWPPropertyHelper.ChangeProperty(
+            _visible,
+            value,
+            args => OnVisibleChanging(),  // Problem: signature mismatch
+            VisibleChanging,               // Problem: uses CancelEventArgs, not ValueChangingEventArgs
+            args => OnVisibleChanged(),
+            VisibleChanged,                // Problem: uses EventArgs, not ValueChangedEventArgs
+            out bool finalValue);
+
+        if (changed)
+        {
+            _visible = finalValue;
+            
+            // Post-processing
+            if (!_visible)
+            {
+                RestoreFocus ();
+            }
+            SetNeedsDraw ();
+            // ... more
+        }
+    }
+}
+```
+
+**Migration Difficulty**: ⭐⭐⭐⭐ VERY HARD
+
+**Issues**:
+1. OnVisibleChanging() takes no parameters - CWPPropertyHelper expects ValueChangingEventArgs<T>
+2. VisibleChanging uses CancelEventArgs, not ValueChangingEventArgs<bool>
+3. VisibleChanged uses EventArgs, not ValueChangedEventArgs<bool>
+4. Would require changing event signatures (BREAKING CHANGE)
+5. Complex post-processing logic after change
+
+**Recommendation**: ❌ Don't migrate. Would require breaking API changes.
+
+---
+
+#### 6.2 OnSchemeNameChanging / SchemeName
+**File**: `Terminal.Gui/ViewBase/View.Drawing.Scheme.cs:25-44`
+
+**Current Implementation**: ✅ ALREADY USES CWPPropertyHelper!
+
+**Status**: ✅ Already migrated - good example to follow
+
+---
+
+#### 6.3 OnGettingScheme / GettingScheme
+**File**: `Terminal.Gui/ViewBase/View.Drawing.Scheme.cs:133-162`
+
+**Current Implementation**: ✅ ALREADY USES CWPWorkflowHelper.ExecuteWithResult!
+
+**Status**: ✅ Already migrated - good example to follow
+
+---
+
+#### 6.4 OnSettingScheme / SetScheme
+**File**: `Terminal.Gui/ViewBase/View.Drawing.Scheme.cs:217-234`
+
+**Current Implementation**: ✅ ALREADY USES CWPPropertyHelper!
+
+**Status**: ✅ Already migrated - good example to follow
+
+---
+
+### Category 7: View-Specific Events (LOW PRIORITY)
+
+#### 7.1 OnCellActivated / CellActivated (TableView)
+**File**: `Terminal.Gui/Views/TableView/TableView.cs:1277+`
+
+**Analysis**: View-specific, likely custom implementation
+
+**Can Use Helper?**: ✅ Likely YES
+
+**Migration Difficulty**: ⭐⭐ MEDIUM
+
+**Recommendation**: ⚠️ Low priority, could migrate
+
+---
+
+#### 7.2 OnPositionChanging (ScrollBar, ScrollSlider)
+**Files**: 
+- `Terminal.Gui/Views/ScrollBar/ScrollBar.cs:375`
+- `Terminal.Gui/Views/ScrollBar/ScrollSlider.cs:245`
+
+**Can Use Helper?**: ✅ Likely YES - property change pattern
+
+**Migration Difficulty**: ⭐⭐ MEDIUM
+
+**Issues**:
+1. Takes (int currentPos, int newPos) parameters
+2. CWPPropertyHelper expects single value
+3. Would need wrapper
+
+**Recommendation**: ⚠️ Could work but parameters don't match pattern well
+
+---
+
+#### 7.3 OnCheckedStateChanging (CheckBox)
+**File**: `Terminal.Gui/Views/CheckBox.cs:187`
+
+**Current Implementation** (in ChangeCheckedState):
+```csharp
+private bool? ChangeCheckedState (CheckState value)
+{
+    if (_checkedState == value || (value is CheckState.None && !AllowCheckStateNone))
+    {
+        return null;
+    }
+
+    ResultEventArgs<CheckState> e = new (value);
+
+    if (OnCheckedStateChanging (e))
+    {
+        return true;
+    }
+
+    CheckedStateChanging?.Invoke (this, e);
+
+    if (e.Handled)
+    {
+        return e.Handled;
+    }
+
+    _checkedState = value;
+    UpdateTextFormatterText ();
+    SetNeedsLayout ();
+
+    EventArgs<CheckState> args = new (in _checkedState);
+    OnCheckedStateChanged (args);
+
+    CheckedStateChanged?.Invoke (this, args);
+
+    return false;
+}
+```
+
+**Can Use Helper?**: ✅ YES - This is a PERFECT match!
+
+**Proposed Migration**:
+```csharp
+private bool? ChangeCheckedState (CheckState value)
+{
+    // Pre-condition check
+    if (_checkedState == value || (value is CheckState.None && !AllowCheckStateNone))
+    {
+        return null;
+    }
+
+    bool changed = CWPPropertyHelper.ChangeProperty(
+        _checkedState,
+        value,
+        OnCheckedStateChanging,
+        CheckedStateChanging,
+        OnCheckedStateChanged,
+        CheckedStateChanged,
+        out CheckState finalValue);
+
+    if (changed)
+    {
+        _checkedState = finalValue;
+        UpdateTextFormatterText();
+        SetNeedsLayout();
+        return false;
+    }
+    
+    return true; // Was cancelled
+}
+```
+
+**Migration Difficulty**: ⭐ EASY
+
+**Issues**:
+1. Need to adjust OnCheckedStateChanged signature from EventArgs<CheckState> to ValueChangedEventArgs<CheckState>
+2. Return semantics: method returns bool? (null/true/false), helper only returns bool (true/false)
+
+**Recommendation**: ✅ **STRONGLY RECOMMEND** - This is the best candidate! But need to handle:
+- Signature changes for OnCheckedStateChanged
+- Return value mapping (helper returns bool, method needs bool?)
+
+---
+
+#### 7.4 OnGettingAttributeForRole / GettingAttributeForRole
+**File**: `Terminal.Gui/ViewBase/View.Drawing.Attribute.cs:78`
+
+**Analysis**: Likely embedded in GetAttributeForRole method
+
+**Can Use Helper?**: ⚠️ PARTIAL - Need to examine implementation
+
+**Migration Difficulty**: ⭐⭐⭐ HARD (unknown)
+
+**Recommendation**: ⚠️ Low priority, need more analysis
+
+---
+
+#### 7.5 OnBorderStyleChanged
+**File**: `Terminal.Gui/ViewBase/View.Adornments.cs:162`
+
+**Analysis**: Property change notification (not cancellable)
+
+**Can Use Helper?**: ❌ NO - Post-change only, no pre-change cancellation
+
+**Recommendation**: ❌ Don't migrate. Not CWP pattern.
+
+---
+
+#### 7.6 OnMouseIsHeldDownTick (MouseHeldDown)
+**File**: `Terminal.Gui/ViewBase/MouseHeldDown.cs:71`
+
+**Analysis**: Specialized repeating tick event
+
+**Can Use Helper?**: ❌ NO - Custom timer logic
+
+**Recommendation**: ❌ Don't migrate. Specialized use case.
+
+---
+
+## Migration Recommendations by Priority
+
+### ✅ STRONGLY RECOMMEND (1 instance)
+**Effort**: 1-2 days
+
+1. **CheckBox.OnCheckedStateChanging** 
+   - Perfect fit for CWPPropertyHelper
+   - Requires minor signature adjustments
+   - Good example for others
+   - Clear improvement
+
+### ⚠️ CONSIDER (4 instances)
+**Effort**: 1-2 weeks
+
+2. **Command Events** (OnAccepting, OnSelecting, OnHandlingHotKey, OnCommandNotBound)
+   - Could work but requires wrapper logic
+   - Marginal improvement over direct implementation
+   - Good for consistency if pursuing helper migration strategy
+
+3. **Keyboard Events** (OnKeyDown, OnKeyDownNotHandled, OnKeyUp)
+   - Could work but mutable Key complicates
+   - Marginal improvement
+   - Consider only for consistency
+
+### ❌ DON'T MIGRATE (23 instances)
+**Reason**: Custom logic, embedded patterns, or breaking changes required
+
+- **Mouse Events** (4): OnMouseEvent, OnMouseClick, OnMouseWheel, OnMouseEnter
+  - Custom pre/post processing
+  - Direct implementation clearer
+
+- **Navigation Events** (2): OnAdvancingFocus, OnHasFocusChanging
+  - Embedded in complex methods
+  - Too tightly coupled
+
+- **Drawing Events** (6): All drawing events
+  - Custom pipeline
+  - Non-standard pattern with dual overloads
+
+- **Property Changes** (1): OnVisibleChanging
+  - Would require breaking API changes
+  - Complex post-processing
+
+- **View-Specific** (10): TableView, ScrollBar/Slider (2), BorderStyle, MouseHeldDown, etc.
+  - Specialized logic
+  - Low value
+
+## Overall Migration Strategy
+
+### Option A: Minimal Migration (RECOMMENDED)
+**Effort**: 1-2 days
+**Risk**: LOW
+
+Migrate only:
+1. CheckBox.OnCheckedStateChanging (perfect fit)
+
+**Benefits**:
+- Demonstrates helper usage in real View code
+- Clear improvement
+- Low risk
+
+**Drawbacks**:
+- Limited consistency improvement
+- Most code still uses direct implementation
+
+---
+
+### Option B: Partial Migration
+**Effort**: 2-3 weeks
+**Risk**: MEDIUM
+
+Migrate:
+1. CheckBox.OnCheckedStateChanging
+2. All Command events (4 instances)
+3. All Keyboard events (3 instances)
+
+**Benefits**:
+- More consistency
+- Demonstrates helper usage in core events
+- Shows pattern across event types
+
+**Drawbacks**:
+- Significant refactoring effort
+- Marginal improvement for many instances
+- Requires careful testing
+- More complex code in some cases
+
+---
+
+### Option C: Full Migration
+**Effort**: 6-8 weeks
+**Risk**: HIGH
+
+Attempt to migrate all possible instances.
+
+**Benefits**:
+- Maximum consistency
+- Full helper usage
+
+**Drawbacks**:
+- Many instances don't fit helper pattern well
+- Would require API breaking changes
+- Creates wrapper complexity
+- Direct implementation often clearer
+- Very high effort for limited benefit
+
+**Recommendation**: ❌ NOT RECOMMENDED
+
+---
+
+## Key Findings & Gotchas
+
+### 1. Helper Pattern Assumptions Don't Always Match Reality
+
+**Issue**: Helpers assume clean separation of concerns, but real implementations have:
+- Pre-conditions (Enabled checks, visibility checks)
+- Post-conditions (command invocation, state updates)
+- Complex argument types (CommandEventArgs vs ResultEventArgs)
+- Mutable arguments (Key.Handled)
+
+**Example**: RaiseMouseClickEvent has both pre-checks and post-command invocation that can't fit in helper.
+
+---
+
+### 2. Event Argument Type Mismatches
+
+**Issue**: Many events use custom EventArgs types that don't match helper expectations:
+- CancelEventArgs vs ValueChangingEventArgs<T>
+- CommandEventArgs vs ResultEventArgs<T>
+- EventArgs vs ValueChangedEventArgs<T>
+
+**Impact**: Would require:
+- Breaking API changes to event signatures, OR
+- Complex wrapper logic that negates helper benefits
+
+---
+
+### 3. Embedded Patterns Resist Extraction
+
+**Issue**: Many CWP patterns are embedded in larger methods (AdvanceFocus, DoDrawContent, etc.) with:
+- Complex surrounding logic
+- State management
+- Multiple phases
+
+**Impact**: Extracting to use helpers would:
+- Require significant refactoring
+- Reduce readability
+- Increase complexity
+
+---
+
+### 4. Two-Virtual-Method Pattern
+
+**Issue**: Drawing events have TWO virtual methods:
+```csharp
+OnDrawingContent(DrawContext? context)
+OnDrawingContent()  // No parameters
+```
+
+**Impact**: Helper assumes single virtual method. This pattern can't fit without major changes.
+
+---
+
+### 5. Return Value Semantics Differ
+
+**Issue**: Many methods return bool?, helpers return bool or bool?:
+- null = no subscribers
+- true = handled/cancelled
+- false = not handled
+
+**Impact**: Translating return values adds complexity.
+
+---
+
+### 6. Current Helper Usage is Limited
+
+**Fact**: Only View.Drawing.Scheme.cs uses helpers currently (3 usages).
+
+**Implications**:
+- Helpers are relatively new
+- Most code predates helpers
+- Helpers designed for specific patterns (property changes, result workflows)
+- Not designed to replace all CWP implementations
+
+---
+
+### 7. Direct Implementation Often Clearer
+
+**Observation**: For many instances, direct implementation is:
+- More readable
+- More maintainable
+- Less code
+- Fewer abstractions
+
+**Example**:
+```csharp
+// Direct (Current) - Clear and simple
+if (OnKeyDown (k) || k.Handled)
+{
+    return true;
+}
+KeyDown?.Invoke (this, k);
+return k.Handled;
+
+// With Helper - More complex
+ResultEventArgs<Key> args = new (k);
+bool? result = CWPWorkflowHelper.Execute(
+    onMethod: args => OnKeyDown(k),
+    eventHandler: (sender, e) => KeyDown?.Invoke(sender, k),
+    args: args,
+    defaultAction: null);
+return result == true || k.Handled;
+```
+
+---
+
+## Recommendations Summary
+
+### For Maintainers
+
+1. **Don't pursue full migration** - Effort (6-8 weeks) far exceeds benefit
+2. **Migrate CheckBox.OnCheckedStateChanging** - Perfect fit, good example
+3. **Consider Command/Keyboard events** - Only if pursuing consistency strategy
+4. **Keep direct implementations** - For most instances, direct is clearer
+5. **Document when to use helpers** - Update guidance for new CWP implementations
+
+### For New Code
+
+**Use CWPPropertyHelper when**:
+- Implementing property setter with validation
+- Need pre/post change events
+- Event types match ValueChangingEventArgs<T> and ValueChangedEventArgs<T>
+
+**Use CWPWorkflowHelper when**:
+- Simple workflow with single result type
+- Clean separation of virtual method, event, default action
+- No complex pre/post processing
+- ResultEventArgs<T> fits naturally
+
+**Use direct implementation when**:
+- Custom pre/post processing logic
+- Complex argument types
+- Embedded in larger method
+- Event types don't match helper expectations
+- Direct code is clearer
+
+### Documentation Updates Needed
+
+If pursuing any migration:
+
+1. Update `docs/analysis/cwp_analysis_report.md`
+   - Add section on helper class usage
+   - Document migration outcomes
+
+2. Update `docfx/docs/events.md`
+   - Add guidance on when to use helpers
+   - Show examples of helper usage
+   - Document when direct implementation is better
+
+3. Update `docfx/docs/cancellable-work-pattern.md`
+   - Add helper class section
+   - Explain design decisions
+   - Show pros/cons
+
+## Conclusion
+
+**Total Effort Summary**:
+- Option A (Recommended): 1-2 days
+- Option B (Partial): 2-3 weeks  
+- Option C (Full): 6-8 weeks (not recommended)
+
+**Key Insight**: CWP helper classes were designed for specific patterns (property changes, clean workflows) and work well for those cases (View.Drawing.Scheme.cs). However, most CWP implementations in the codebase have custom logic, pre/post processing, or embedded patterns that don't fit the helper abstraction well.
+
+**Recommendation**: Migrate CheckBox.OnCheckedStateChanging only (Option A). This demonstrates helper usage in a real view, provides a clear example for developers, and improves code quality without significant risk or effort. Leave other implementations as direct implementations since they're often clearer and more maintainable that way.
+
+The goal should be to use helpers where they add value, not to force all code into the helper pattern.