Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
606 changes: 606 additions & 0 deletions .github/agents/maui-expert-reviewer.md

Large diffs are not rendered by default.

32 changes: 32 additions & 0 deletions .github/instructions/collectionview-android.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
applyTo:
- "src/Controls/src/Core/Handlers/Items/Android/**"
- "src/Controls/src/Core/Handlers/Items/*.Android.cs"
- "src/Controls/src/Core/Handlers/Items/*.android.cs"
---
# CollectionView — Android (Items/ Handler)

> Items/ is the sole Android handler — see `collectionview-handler-detection.instructions.md` for the full platform→handler mapping.

## RecyclerView Adapter Patterns
- Use range-specific notifications (`NotifyItemRangeInserted`, `NotifyItemRangeRemoved`, `NotifyItemRangeChanged`) when INCC semantics provide exact affected ranges — prefer these over `NotifyDataSetChanged` for preserving scroll position and animations
- Full refresh via `NotifyDataSetChanged` is valid for `Reset` actions, ambiguous index cases, and header/footer template changes
- Handle all `ObservableCollection` change actions: Add, Remove, Replace, Move, Reset
- On `Reset`, recalculate adapter state from scratch — do not assume incremental consistency

## ViewHolder Recycling
- `BindingContext` MUST be updated on every rebind — stale data from a previous holder is a common source of visual glitches
- Do not store item-specific state in the ViewHolder outside of `BindViewHolder` — recycled holders carry state from previous items
- Dispose and recreate platform views only when the template changes, not on every rebind

## Layout Manager
- Select layout manager (Linear, Grid, custom) based on `ItemsLayout` specification — do not hardcode
- `ItemsLayout` property changes require full layout manager replacement, not partial reconfiguration
- Account for Android pixel rounding in item measurement — fractional dp values cause 1px gaps

## Memory and Lifecycle
- Unsubscribe from `ScrollChange` and adapter observers in `DisconnectHandler` — do NOT call Dispose on platform objects
- Scroll position restoration after adapter data changes must handle empty-collection edge case

## Regression Patterns
- Test across empty collection, single item, many items, and with grouping — a fix for one layout scenario routinely breaks another
34 changes: 34 additions & 0 deletions .github/instructions/collectionview-ios.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
applyTo:
- "src/Controls/src/Core/Handlers/Items2/**"
- "src/Controls/src/Core/Handlers/Items/iOS/**"
- "src/Controls/src/Core/Handlers/Items/*.iOS.cs"
- "src/Controls/src/Core/Handlers/Items/*.ios.cs"
---
# CollectionView — iOS/MacCatalyst (Items2/ Handler)

> New iOS/MacCatalyst work targets Items2/. Items/iOS/ is deprecated. See `collectionview-handler-detection.instructions.md` for the full platform→handler mapping.

## UICollectionView Cell Measurement
- Scope cell layout invalidation to the affected cell — avoid `InvalidateLayout()` on the entire collection for single-cell changes
- Custom `UICollectionViewCell` subclasses should handle `MeasureInvalidated` only when the hosted MAUI control actually needs remeasuring
- Cell sizing must stay in sync between the measure pass and the layout pass — out-of-sync causes visual glitches

## UICollectionViewCompositionalLayout
- Layout configuration must match the `ItemsLayout` specification (Linear, Grid, or custom)
- `ItemsLayout` property changes require full layout reconfiguration — partial updates leave stale section configuration
- Group header/footer template changes must invalidate the correct section scope, not the entire layout

## Memory Management
- Use static callback patterns to avoid retain cycles between cells and their hosting handler
- Remove `NSNotificationCenter` observers in `DisconnectHandler`
- Weak references for long-lived observers of short-lived cells — cells are recycled and reused

## Regression Patterns
- Test across empty collection, single item, many items, and with grouping — a fix for one layout scenario routinely breaks another

## Items/ iOS (Deprecated)
- Files in `Handlers/Items/*.iOS.cs` are deprecated — prefer Items2/ for new work
- Only modify Items/ iOS code for explicit legacy maintenance or backward-compatibility fixes

> **Platform file extension rules** (`.ios.cs` vs `.maccatalyst.cs` compilation targets) are defined in `copilot-instructions.md` § Platform-Specific File Extensions. See also `collectionview-handler-detection.instructions.md` for which handler directory to target per platform.
26 changes: 26 additions & 0 deletions .github/instructions/collectionview-windows.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
applyTo:
- "src/Controls/src/Core/Handlers/Items/*.Windows.cs"
- "src/Controls/src/Core/Handlers/Items/*.windows.cs"
---
# CollectionView — Windows (Items/ Handler)

Items/ is the **ONLY** Windows CollectionView implementation. Items2/ has NO Windows code.

## WinUI ListView/ItemsRepeater Patterns
- Preserve WinUI XAML styles applied via native theming — clearing a MAUI property must restore the style-applied value, not a hardcoded default
- `double.NaN` is the WinUI convention for unconstrained dimensions — do not confuse with MAUI's `double.PositiveInfinity`
- Use `DispatcherQueue.TryEnqueue` for deferred UI thread work — do not use `Dispatcher.BeginInvoke`

## Data Source and Change Notifications
- Handle all `ObservableCollection` change actions (Add, Remove, Replace, Move, Reset) with range-scoped updates
- Avoid full source refresh (`NotifyDataSetChanged` equivalent) — it kills selection state and scroll position
- Selection mode changes must propagate correctly to the native `SelectionMode` property

## Layout Configuration
- `ItemsLayout` changes require reconfiguration of the underlying panel (e.g., `ItemsWrapGrid`, `ItemsStackPanel`)
- Verify that `ItemsLayout.Span` for grid layouts maps correctly to WinUI's `MaximumRowsOrColumns`

## Cross-Platform Consistency
- Default values for control properties must produce the same visual result as Android and iOS
- Event firing order (selection changed, scrolled) should match other platforms for the same user interaction
36 changes: 36 additions & 0 deletions .github/instructions/handler-patterns.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
applyTo:
- "src/Core/src/Handlers/**"
- "src/Controls/src/Core/Handlers/**"
---
# Handler Mapper and Property Patterns

## Property Update Flow
- Property updates MUST go through `Handler.UpdateValue(nameof(Property))` — never call mapper methods directly
- Direct calls bypass user-registered `AppendToMapping`/`PrependToMapping` customizations
- Map dependencies before dependents — if property B reads property A, A's mapper must run first
- `CommandMapper` entries return void and use the `(handler, view, args)` signature

## Lifecycle Management
- **ConnectHandler**: Register listeners, subscribe to events, capture native defaults BEFORE applying cross-platform properties
- **DisconnectHandler**: Unsubscribe all events, dispose platform resources, null out references
- Call `base.ConnectHandler`/`base.DisconnectHandler` — base class performs platform hookup/teardown (NOT mapper initialization, which happens in `SetVirtualView`)

## Null Safety in Callbacks
- Null-check `VirtualView` before access in every mapper method and platform callback — it is null during disconnect
- Validate `MauiContext` before use — throw `InvalidOperationException` with descriptive message if null
- After async operations, verify the handler is still connected before applying results

## Native Defaults Preservation
- Capture native default values (colors, fonts, styles) in `ConnectHandler` BEFORE any cross-platform property is applied
- Clearing a cross-platform property (setting to null/default) must restore the captured native default, not a hardcoded fallback
- On Windows, preserve WinUI XAML style-applied values; on Android, cache theme-inherited drawables; on iOS, capture UIAppearance defaults

## Mapper Extensibility
- When extending existing mappers, ensure the base mapper chain is called — do not silently replace
- Static mapper methods should be `public static` to enable platform-specific overrides
- Use `nameof()` for property keys — avoid magic strings

## Fire-and-Forget Async
- Use `.FireAndForget(handler)` for async operations in mapper methods — never use bare `async void`
- The `FireAndForget` overload accepting a handler logs exceptions through the handler's service provider
30 changes: 30 additions & 0 deletions .github/instructions/layout-system.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
applyTo:
- "src/Core/src/Layouts/**"
- "src/Controls/src/Core/Layout/**"
- "src/Core/src/Handlers/Layout/**"
---
# Layout System Rules

## Measure/Arrange Contract
- `Measure(widthConstraint, heightConstraint)` returns the desired size — it must not mutate layout state or trigger side effects
- `ArrangeChildren(bounds)` positions children within the given bounds — it must respect the size returned from `Measure`, not compute independently
- A child measured with `widthConstraint=200` must not be arranged with `width=300` — constraints must stay consistent across passes
- Subtract padding, margin, and border thickness from constraints BEFORE passing to children

## Constraint Propagation
- Stack layouts pass `double.PositiveInfinity` along the stacking axis and the parent constraint along the cross axis
- Grid cells compute per-cell constraints from column/row definitions — do not pass the full grid constraint to each child
- `MeasureContent` helpers on `IContentView` handle inset subtraction — use them instead of manual arithmetic
- On Windows, `double.NaN` represents unconstrained dimensions (WinUI convention) — do not confuse with `double.PositiveInfinity` (MAUI convention)

## Infinite Loop Avoidance
- Never create circular dependencies where child size depends on parent AND parent size depends on child
- Layout invalidation (`InvalidateMeasure`) must not re-trigger during an active measure pass
- ScrollView content must always be re-measured on layout trigger — do not aggressively cache child measurements in scrollable containers

## Performance on Hot Paths

> Layout measure/arrange methods are hot paths. All rules from `performance-hotpaths.instructions.md` apply — no LINQ, closures, or allocations. Additionally for layout:
- Cache expensive computations (e.g., `GridStructure`) and invalidate only when inputs change
- Bindable property change handlers should skip layout invalidation when the value has not actually changed
24 changes: 24 additions & 0 deletions .github/instructions/performance-hotpaths.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
applyTo:
- "src/Core/src/Layouts/**"
- "src/Controls/src/Core/Handlers/**"
---
# Performance-Critical Path Rules

## Hot Paths in MAUI
Measure/arrange cycles, scrolling callbacks, binding propagation, and property change notifications are called at high frequency. All rules below apply to code on these paths.

## Allocation Avoidance
- No LINQ methods (`.Where`, `.Select`, `.FirstOrDefault`, `.ToList`) — use indexed `for` loops
- No closures or lambdas that capture variables — these allocate a compiler-generated class per invocation
- No string concatenation or interpolation — use `StringBuilder` or pre-allocated strings if logging is required
- Prefer `Count` + indexer over `IEnumerable` iteration to avoid enumerator allocation

## Caching and Invalidation
- Cache results of expensive computations called multiple times per layout pass
- Invalidate caches when inputs change — stale caches cause incorrect layout
- Skip redundant work: if a property's new value equals the old value, do not trigger layout invalidation or re-render

## Collection Iteration
- When the source implements `IList` or `IReadOnlyList`, use `for (int i = 0; i < list.Count; i++)` instead of `foreach`
- `ObservableCollection` change handlers should process only the affected range, not re-enumerate the entire collection
25 changes: 25 additions & 0 deletions .github/instructions/public-api.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
applyTo:
- "**/PublicAPI.Unshipped.txt"
---
# Public API Surface Design

## API Addition Rules
- New public APIs must have clear, demonstrated use cases — no speculative additions
- API naming must follow .NET design guidelines and be consistent with existing MAUI patterns
- Interfaces belong at `IView`/`IElement` level when behavior is cross-cutting — do not duplicate per-control

## PublicAPI.Unshipped.txt
- Entries must exactly match the actual API shape (namespace, type, member signature)

> For PublicAPI.Unshipped.txt file management workflow (never disable analyzers, `dotnet format analyzers`, revert-then-add pattern), see `copilot-instructions.md` § PublicAPI.Unshipped.txt File Management.

## Obsolescence and Removal
- Deprecated APIs must go through `[Obsolete("message")]` with migration guidance before removal
- Include the replacement API or pattern in the obsolete message
- Breaking changes require explicit design justification documented in the PR

## Visibility Decisions
- Default to `internal` — make `public` only when external consumers need access
- `protected` members in unsealed types are part of the public API surface — treat with same rigor
- Use `[EditorBrowsable(EditorBrowsableState.Never)]` for APIs that must be public for technical reasons but should not appear in IntelliSense
34 changes: 34 additions & 0 deletions .github/instructions/threading-async.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
applyTo:
- "**/*.Android.cs"
- "**/*.android.cs"
- "**/*.iOS.cs"
- "**/*.ios.cs"
- "**/*.Windows.cs"
- "**/*.windows.cs"
- "**/Platform/**"
- "**/Platforms/**"
---
# Threading and Async Patterns

## UI Thread Dispatch
- Platform view modifications MUST happen on the UI thread
- **Android**: `platformView.Post(() => { })` or `Looper.MainLooper` for thread checks
- **iOS/MacCatalyst**: `MainThread.BeginInvokeOnMainThread()` or `DispatchQueue.MainQueue`
- **Windows**: `DispatcherQueue.TryEnqueue()` — be aware of COM threading apartment model

## Async Handler Operations
- Use `.FireAndForget(handler)` for async work in mapper methods — never bare `async void`
- After `await`, verify the handler is still connected (`VirtualView != null`) before applying results
- Thread `CancellationToken` through long-running operations and check at appropriate yield points

## Race Condition Prevention
- Concurrent access to shared state must use `lock`, `Interlocked`, or immutable patterns
- Image loading and other async pipelines must cancel in-flight operations when the source changes
- Guard against stale callbacks — platform callbacks may fire after `DisconnectHandler`

## Platform-Specific Patterns
- **Android**: API-level checks use `Build.VERSION.SdkInt` comparison, not version string parsing
- **iOS**: Use `System.OperatingSystem.IsIOSVersionAtLeast()` for linker-friendly runtime checks
- **Windows**: WinUI version checks may be needed for features in specific Windows App SDK versions
- Use `System.OperatingSystem` APIs over `RuntimeInformation` — they are trimmer/AOT-friendly
25 changes: 24 additions & 1 deletion .github/scripts/Review-PR.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,8 @@ $gateStatusForPrompt = switch ($gateResult) {
}

$step2Prompt = @"
Use a skill to review PR #$PRNumber.
First, use the code-review skill for dimensional code analysis with the maui-expert-reviewer agent — this writes inline-findings.json.
Then, use the pr-review skill to review PR #$PRNumber. The pr-review Report phase should account for any code quality issues found by the expert reviewer.

$platformInstruction
$autonomousRules
Expand All @@ -565,6 +566,7 @@ $autonomousRules
Do NOT re-run gate verification. The gate phase is handled separately.

📁 Write phase output to ``CustomAgentLogsTmp/PRState/$PRNumber/PRAgent/{phase}/content.md``
📁 Also write inline review findings to ``CustomAgentLogsTmp/PRState/$PRNumber/PRAgent/inline-findings.json``
"@

Invoke-CopilotStep -StepName "STEP 2: PR REVIEW" -Prompt $step2Prompt | Out-Null
Expand Down Expand Up @@ -609,6 +611,27 @@ if (Test-Path $reviewScript) {
Write-Host " ⚠️ post-ai-summary-comment.ps1 not found — skipping review summary" -ForegroundColor Yellow
}

# Post inline review comments (file:line findings from expert-reviewer agent)
$inlineScript = Join-Path $summaryScriptsDir "post-inline-review.ps1"
$findingsFile = Join-Path $RepoRoot "CustomAgentLogsTmp/PRState/$PRNumber/PRAgent/inline-findings.json"
if ((Test-Path $inlineScript) -and (Test-Path $findingsFile)) {
try {
Write-Host " 📝 Posting inline review comments..." -ForegroundColor Cyan
if ($DryRun) {
& $inlineScript -PRNumber $PRNumber -FindingsFile $findingsFile -DryRun
} else {
& $inlineScript -PRNumber $PRNumber -FindingsFile $findingsFile
}
Write-Host " ✅ Inline review comments posted" -ForegroundColor Green
} catch {
Write-Host " ⚠️ Inline review posting failed (non-fatal): $_" -ForegroundColor Yellow
}
} else {
if (-not (Test-Path $findingsFile)) {
Write-Host " ℹ️ No inline findings file — agent may not have produced findings" -ForegroundColor Gray
}
}

# ═════════════════════════════════════════════════════════════════════════════
# STEP 4: Apply Labels
# ═════════════════════════════════════════════════════════════════════════════
Expand Down
Loading
Loading