Skip to content

Commit 2db2fb1

Browse files
csharpfritzCopilot
andcommitted
docs(ai-team): Log P0 event handler session, merge decisions
Session: 2026-03-05-p0-event-handlers Requested by: Jeffrey T. Fritz Changes: - Logged session to .ai-team/log/2026-03-05-p0-event-handlers.md - Merged 3 decision files from inbox into decisions.md - Summarized Forge's 42KB audit to concise entry in decisions.md - Propagated updates to cyclops, forge, rogue history files - Summarized oversized history.md files (rogue, cyclops, forge) - Deleted inbox files after merge Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1005387 commit 2db2fb1

7 files changed

Lines changed: 80 additions & 823 deletions

File tree

.ai-team/agents/cyclops/history.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,18 @@ Team updates: @rendermode fix (PR #419), EF Core 10.0.3, WebFormsPageBase shippe
8383

8484
### P0 Event Handler Fixes (2026-03-06)
8585

86-
**P0-1 Repeater events:** Added `ItemCommand`/`OnItemCommand` (`RepeaterCommandEventArgs`), `ItemCreated`/`OnItemCreated` (`RepeaterItemEventArgs`), `ItemDataBound`/`OnItemDataBound` (`RepeaterItemEventArgs`). Created `RepeaterCommandEventArgs.cs` and `RepeaterItemEventArgs.cs`. Repeater previously had ZERO EventCallbacks.
86+
All 7 P0 items from Forge's audit implemented:
87+
- **P0-1** Repeater: 3 events added (ItemCommand, ItemCreated, ItemDataBound) — had ZERO before
88+
- **P0-2** DataList: 7 events added + ItemDataBound bare alias. Renamed internal `ItemDataBound()``ItemDataBoundInternal()` to avoid parameter collision
89+
- **P0-3/P0-4** GridView: RowDataBound + RowCreated added, bare RowCommand alias, ButtonField.razor.cs updated to coalesce
90+
- **P0-5** DetailsView: ItemCreated added
91+
- **P0-6** FormView: OnItemInserted type fixed (FormViewInsertEventArgs → FormViewInsertedEventArgs), 6 bare CRUD aliases added
92+
- **P0-7** SelectMethod: moved from OnAfterRender(firstRender) to OnParametersSet(), added RefreshSelectMethod()
8793

88-
**P0-2 DataList events:** Added 7 missing event pairs: `ItemCommand`/`OnItemCommand` (`DataListCommandEventArgs`), `SelectedIndexChanged`/`OnSelectedIndexChanged` (`EventArgs`), `EditCommand`/`OnEditCommand`, `UpdateCommand`/`OnUpdateCommand`, `DeleteCommand`/`OnDeleteCommand`, `CancelCommand`/`OnCancelCommand` (all `DataListCommandEventArgs`), `ItemCreated`/`OnItemCreated` (`DataListItemEventArgs`). Added bare `ItemDataBound` alias for existing `OnItemDataBound`. Renamed internal method from `ItemDataBound()` to `ItemDataBoundInternal()` to avoid conflict with new parameter property — updated both call sites in `DataList.razor`. Created `DataListCommandEventArgs.cs`.
94+
New EventArgs: `RepeaterCommandEventArgs`, `RepeaterItemEventArgs`, `DataListCommandEventArgs`, `GridViewRowEventArgs`, `FormViewInsertedEventArgs`.
8995

90-
**P0-3/P0-4 GridView RowDataBound/RowCreated:** Added `RowDataBound`/`OnRowDataBound` and `RowCreated`/`OnRowCreated` (both `GridViewRowEventArgs`). Also added bare `RowCommand` alias for existing `OnRowCommand`. Updated `ButtonField.razor.cs` to coalesce `RowCommand`/`OnRowCommand` instead of directly accessing `OnRowCommand`. Created `GridViewRowEventArgs.cs`.
96+
**Pattern:** When `[Parameter]` name collides with internal method, rename method with `*Internal` suffix — never rename the parameter.
9197

92-
**P0-5 DetailsView ItemCreated:** Added `ItemCreated`/`OnItemCreated` (`EventCallback<EventArgs>`) to DetailsView events region.
9398

94-
**P0-6 FormView OnItemInserted type fix:** Changed `OnItemInserted` from `EventCallback<FormViewInsertEventArgs>` (wrong — Insert tense) to `EventCallback<FormViewInsertedEventArgs>` (correct — past tense). Created `FormViewInsertedEventArgs.cs` with `AffectedRows`, `Exception`, `ExceptionHandled`, `KeepInInsertMode`, `Values`. Updated `HandleCommandArgs` insert case to construct `FormViewInsertedEventArgs(0)` instead of `FormViewInsertEventArgs("insert")`. Also added bare-name aliases for all 6 FormView CRUD events (`ItemDeleting`, `ItemDeleted`, `ItemInserting`, `ItemInserted`, `ItemUpdating`, `ItemUpdated`) and updated all `HandleCommandArgs` invocation sites to coalesce bare/On-prefix.
99+
Team update (2026-03-06): Forge's Event Handler Fidelity Audit merged to decisions.md (P0 items all resolved by Cyclops, tested by Rogue). 11 P1 and 7 P2 items remain for future work. decided by Forge, implemented by Cyclops
95100

96-
**P0-7 SelectMethod re-fire:** Moved `SelectMethod` invocation from `OnAfterRender(firstRender)` to `OnParametersSet()` in `DataBoundComponent<T>`. This fires on every parameter change, not just first render. Added `RefreshSelectMethod()` helper for post-CRUD data refresh.
97-
98-
**Key files created:** `RepeaterCommandEventArgs.cs`, `RepeaterItemEventArgs.cs`, `DataListCommandEventArgs.cs`, `GridViewRowEventArgs.cs`, `FormViewInsertedEventArgs.cs` — all in `src/BlazorWebFormsComponents/` root.
99-
100-
**Pattern reinforced:** When a `[Parameter]` property name collides with an internal method name (as happened with DataList's `ItemDataBound`), rename the method to `*Internal` suffix — never rename the parameter, since that breaks migration markup.

.ai-team/agents/forge/history.md

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -109,33 +109,14 @@ Team updates (2026-03-04-05): PRs upstream, reports in docs/migration-tests/, be
109109

110110
### LoginStatus → AuthorizeView Redesign Analysis (2026-03-06)
111111

112-
**Task:** Analyze original Web Forms LoginStatus, compare with current BWFC implementation, propose AuthorizeView-based redesign. Follow-up to LoginView redesign.
112+
**Task:** Analyze LoginStatus for AuthorizeView-based redesign (follow-up to LoginView).
113113

114-
**Key findings about original Web Forms LoginStatus:**
115-
- Inherits `CompositeControl``WebControl``Control` — HAS style properties (unlike LoginView which inherits plain `Control`)
116-
- Renders single `<a>` (text link) or `<input type="image">` (image button) — no wrapper element
117-
- Properties: LoginText, LogoutText, LoginImageUrl, LogoutImageUrl, LogoutPageUrl, LogoutAction (enum: Redirect/RedirectToLoginPage/Refresh)
118-
- Events: LoggingOut (LoginCancelEventArgs with Cancel), LoggedOut (EventArgs)
119-
- Does NOT have a `LoginPageUrl` property — uses `FormsAuthentication.LoginUrl` from web.config
114+
**Key findings:** Inherits CompositeControl→WebControl (HAS style properties, unlike LoginView). Renders `<a>` or `<input type="image">` with no wrapper. 4 issues found: manual AuthenticationStateProvider (HIGH), LogoutAction as abstract class vs enum (MEDIUM, separate PR), misleading LoginPageUrl comment (LOW), null-check missing on LoginPageUrl (LOW). Base class, HTML rendering, events, and style application all correct.
120115

121-
**Current BWFC issues identified (4 total, 1 high severity):**
122-
1. Manual `AuthenticationStateProvider` injection — checks auth once in OnInitializedAsync, doesn't react to changes (HIGH)
123-
2. `LogoutAction` implemented as abstract class hierarchy instead of standard enum (MEDIUM — separate PR)
124-
3. `LoginPageUrl` comment misleading — says "not in Webforms" but doesn't explain why it exists in Blazor (LOW)
125-
4. `LoginHandle` doesn't null-check `LoginPageUrl` — NavigateTo(null) will throw (LOW)
116+
**Architecture:** Wrap markup in `<AuthorizeView>`. ZERO breaking changes (unlike LoginView's 4). All 12 tests need mechanical update to bUnit AddTestAuthorization().
126117

127-
**What's CORRECT (keep unchanged):**
128-
- Base class `BaseStyledComponent` — correct, WebControl has style properties
129-
- HTML rendering: `<a>` for text, `<input type="image">` for images — no wrapper
130-
- Style/class applied directly on elements
131-
- Event names with On-prefix: OnLoggingOut, OnLoggedOut
132-
- Event cancellation via LoginCancelEventArgs.Cancel
133-
- Default text: LoginText="Login", LogoutText="Logout"
134-
135-
**Architecture decision:** Wrap existing Razor markup in `<AuthorizeView><Authorized>...<NotAuthorized>...</AuthorizeView>`. Remove manual AuthenticationStateProvider injection, UserAuthenticated field, and OnInitializedAsync override. ZERO breaking changes — public API identical.
118+
**Decision document:** `.ai-team/decisions/inbox/forge-loginstatus-authorizeview-redesign.md`
136119

137-
**Key difference from LoginView redesign:** LoginView had 4 breaking changes (base class, wrapper div, parameter types). LoginStatus has 0 — only the internal auth mechanism changes. Much narrower scope.
138120

139-
**Test impact:** All 12 tests need mechanical update: replace manual AuthenticationStateProvider mock with bUnit's AddTestAuthorization(). Same assertions, same expected markup.
121+
Team update (2026-03-06): Event Handler Fidelity Audit merged to decisions.md. All 7 P0 items implemented by Cyclops and tested by Rogue (49 tests, all 1519 pass). P1/P2 items remain in the prioritized backlog. audit by Forge
140122

141-
**Decision document:** `.ai-team/decisions/inbox/forge-loginstatus-authorizeview-redesign.md`

.ai-team/agents/rogue/history.md

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -90,29 +90,7 @@ Team updates (2026-03-02-03): Skins roadmap (Forge), M22 planned (Forge), projec
9090

9191
### LoginStatus AuthorizeView Test Redesign (12 files)
9292

93-
Updated all 12 LoginStatus bUnit tests to use bUnit's `this.AddAuthorization()` API instead of manual `Mock<AuthenticationStateProvider>` with `ClaimsPrincipal`/`ClaimsIdentity` setup. This mirrors the LoginView test redesign pattern.
94-
95-
**Pattern applied:**
96-
- Removed `@using Microsoft.AspNetCore.Components.Authorization`, `@using System.Security.Claims;`, and (where present) `@using Moq.Protected;`
97-
- Replaced `ClaimsPrincipal` + `ClaimsIdentity` + `Mock<AuthenticationStateProvider>` + `Services.AddSingleton` block with `var authContext = this.AddAuthorization();`
98-
- Logged-in tests (8 files): `authContext.SetAuthorized("James Bond")`
99-
- Not-logged-in tests (4 files): `authContext.SetNotAuthorized()`
100-
- All NavigationManager mocks (both `Mock<NavigationManager>` and `MockNavigationManager`) left unchanged
101-
- All assertions left unchanged — HTML output is identical
102-
103-
**Files updated:**
104-
1. LoggedInDefault.razor — SetAuthorized
105-
2. LoggedInText.razor — SetAuthorized
106-
3. LoggedInImageWithText.razor — SetAuthorized
107-
4. LoggedInEmpty.razor — SetAuthorized
108-
5. NotLoggedInDefault.razor — SetNotAuthorized
109-
6. NotLoggedInText.razor — SetNotAuthorized
110-
7. NotLoggedInImageWithText.razor — SetNotAuthorized
111-
8. NotLoggedInEmpty.razor — SetNotAuthorized
112-
9. LogoutActionRedirect.razor — SetAuthorized (uses MockNavigationManager)
113-
10. LogoutActionRefresh.razor — SetAuthorized (uses MockNavigationManager)
114-
11. LogoutEvent.razor — SetAuthorized
115-
12. LogoutEventCancelOnLoggingOut.razor — SetAuthorized
93+
Updated all 12 LoginStatus bUnit tests: replaced manual `Mock<AuthenticationStateProvider>` with `this.AddAuthorization()` API (mirrors LoginView pattern). 8 logged-in tests use `SetAuthorized("James Bond")`, 4 not-logged-in use `SetNotAuthorized()`. NavigationManager mocks and assertions unchanged.
11694

11795
### P0 Event Handler Tests (2026-03-06)
11896

@@ -143,3 +121,7 @@ Updated all 12 LoginStatus bUnit tests to use bUnit's `this.AddAuthorization()`
143121
- RowCreated should fire BEFORE RowDataBound (ordering test)
144122
- SelectMethod re-fire after sort requires sort links in thead — test conditionally clicks if present
145123
- FormViewInsertedEventArgs is distinct from FormViewInsertEventArgs — compile-time type safety test proves this
124+
125+
126+
Team update (2026-03-06): P0 event handler decisions merged to decisions.md. All 49 bUnit tests passing. DataList double-render and RowCreated ordering findings documented. tested by Rogue
127+

.ai-team/decisions.md

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6762,3 +6762,63 @@ authContext.SetNotAuthorized(); // for not-logged-in tests
67626762

67636763
**Prerequisite:** `CascadingAuthenticationState` must be in the app's component tree (standard Blazor auth setup). This is already required if the app uses `AuthorizeView` anywhere — and since LoginView now uses it, this is a given.
67646764

6765+
### 2026-03-06: P0 Event Handler Implementation Decisions
6766+
6767+
**By:** Cyclops
6768+
6769+
**What:** Implemented all 7 P0 event handler fixes from Forge's audit. Key decisions:
6770+
6771+
1. **DataList `ItemDataBound` method→parameter collision:** Renamed the internal method from `ItemDataBound()` to `ItemDataBoundInternal()` rather than changing the `[Parameter]` name. The parameter name must match Web Forms markup for migration fidelity.
6772+
6773+
2. **GridView `RowCommand` bare alias:** The existing `OnRowCommand` had no bare-name alias. Added `RowCommand` and updated `ButtonField.razor.cs` to coalesce both, since `ButtonField` directly accessed the GridView's `OnRowCommand` property.
6774+
6775+
3. **FormView CRUD event aliases:** The 6 CRUD events (`OnItemDeleting`, `OnItemDeleted`, `OnItemInserting`, `OnItemInserted`, `OnItemUpdating`, `OnItemUpdated`) previously only had On-prefix forms. Added all bare-name aliases and updated `HandleCommandArgs` to coalesce consistently.
6776+
6777+
4. **FormView `OnItemInserted` type correction:** Changed from `FormViewInsertEventArgs` (present tense, for "Inserting") to new `FormViewInsertedEventArgs` (past tense, for "Inserted"). Follows the same Insert/Inserted pattern seen in `DetailsViewInsertEventArgs`/`DetailsViewInsertedEventArgs`.
6778+
6779+
5. **SelectMethod moved to `OnParametersSet`:** Was previously in `OnAfterRender(firstRender)` which only fires once. Now in `OnParametersSet()` so it re-evaluates whenever parameters change. Added `RefreshSelectMethod()` helper for explicit post-CRUD refreshes.
6780+
6781+
**Why:** Forge's audit identified these as P0 fidelity gaps — migrated Web Forms markup would fail silently because event handlers wouldn't bind. Every data control needs its full event surface to support real-world migration scenarios.
6782+
6783+
### 2026-03-06: P0 Event Handler Test Coverage for All 7 Audit Items
6784+
**By:** Rogue
6785+
**What:** Created 49 bUnit tests across 6 test files covering all P0 event handler additions/fixes from Forge's event handler fidelity audit. Tests verify both bare-name and On-prefix aliases, EventArgs types and properties, lifecycle firing behavior, empty-data edge cases, and Sender property population. 12 tests pass now (FormViewInsertedEventArgs shape, SelectMethod initial render, empty-data guards, type fix). 37 tests are expected-fail pending Cyclops's event wiring in component .razor templates.
6786+
**Why:** These tests define the acceptance criteria for Cyclops's P0 implementation work. They document the exact parameter names, types, and behavioral contracts from the audit. Once Cyclops finishes wiring events in the .razor templates, these tests become the regression safety net. Writing tests against the spec (not the implementation) ensures we catch any deviation from the audit's requirements.
6787+
6788+
### 2026-03-06: DataList ItemDataBound fires 2x per item during Blazor lifecycle
6789+
**By:** Rogue
6790+
**What:** Discovered that DataList's `OnItemDataBound` callback fires twice per data item (6 times for 3 items) due to Blazor's double-render cycle. Tests use `ShouldBeGreaterThanOrEqualTo(N)` instead of exact count assertions.
6791+
**Why:** This is a behavioral characteristic, not necessarily a bug — Blazor re-renders during lifecycle (OnInitialized + OnAfterRender). However, this could cause performance issues with large datasets if event handlers do expensive work. Flagging for team awareness. Future optimization: debounce or guard against duplicate firings in the DataList template.
6792+
6793+
### 2026-03-06: RowCreated must fire before RowDataBound (ordering test)
6794+
**By:** Rogue
6795+
**What:** Added a test in GridView/RowEvents.razor that verifies RowCreated fires BEFORE RowDataBound for each row, matching Web Forms behavior where the row is structurally created before data is bound to it.
6796+
**Why:** Web Forms developers may depend on this ordering (e.g., modifying row structure in RowCreated before data binding populates values in RowDataBound). The ordering test ensures Cyclops implements the events in the correct sequence.
6797+
6798+
### 2026-03-06: Event Handler Fidelity & SelectMethod Audit (summary)
6799+
6800+
**By:** Forge
6801+
**Status:** Implemented (P0 items completed by Cyclops, tested by Rogue)
6802+
6803+
**What:** Comprehensive audit of event handler fidelity across all BWFC controls, triggered by Run 8 benchmark report ("Event handler signatures 15 instances"). Covers data-bound, input, navigation, and login controls.
6804+
6805+
**P0 Items (all resolved):**
6806+
| # | Issue | Component |
6807+
|---|-------|-----------|
6808+
| P0-1 | Repeater had ZERO EventCallbacks | Repeater |
6809+
| P0-2 | DataList missing 7 of 8 events | DataList |
6810+
| P0-3 | GridView missing RowDataBound | GridView |
6811+
| P0-4 | GridView missing RowCreated | GridView |
6812+
| P0-5 | DetailsView missing ItemCreated | DetailsView |
6813+
| P0-6 | FormView OnItemInserted wrong type | FormView |
6814+
| P0-7 | SelectMethod fires only once | DataBoundComponent |
6815+
6816+
**Key findings:**
6817+
- 11 P1 items remain (Button OnClick type, Sender property on ~30 EventArgs, GridView gaps, FormView bare aliases, SelectMethod lambda overload, TreeView/DataGrid alias gaps)
6818+
- 7 P2 items remain (InsertMethod/UpdateMethod/DeleteMethod, remaining bare-name aliases, migration script transforms, public DataBind())
6819+
- Event handler signature migration: Web Forms (object sender, TArgs e) Blazor (TArgs e) Sender accessible via EventArgs.Sender property where available
6820+
- 15 EventArgs classes already have Sender property; ~30 still need it (P1-2)
6821+
6822+
**Full audit archived at:** .ai-team/decisions/inbox/forge-event-handler-selectmethod-audit.md (removed after merge see git history for full text)
6823+
6824+
**Why:** Run 8 benchmark flagged event handler signatures as the largest remaining migration gap. This audit provided the systematic inventory and prioritized roadmap for resolution.

.ai-team/decisions/inbox/cyclops-p0-event-handlers.md

Lines changed: 0 additions & 17 deletions
This file was deleted.

0 commit comments

Comments
 (0)