winui-code-review

When to Use

Run a code review after the app builds and before committing. This catches quality issues that aren't build errors and aren't visible in UI tests — patterns that compile and run but are wrong, fragile, or slow.

How to Review

Read through the project's XAML and C# files and check each section below. For analyzer setup, see winui-dev-workflow; if it isn't installed, tell the user its checks didn't run.

Before reporting an API mismatch or recommending a replacement, verify it against the restored app project's references with CLI 0.7+ winapp find-api, for example winapp find-api members NavigationView --filter selected --json --project-dir <app-project-dir>. See winui-design for batch property checks and project selection; machine-SDK results are not proof of app-package availability.

The analyzer catches a curated set of WinUI 3 / Windows App SDK issues with categorized 4-digit IDs:

• WUI0xxx — UWP → WinUI 3 API compatibility (UwpXamlNamespace, Window.Current, CoreDispatcher, GetForCurrentView)
• WUI1xxx — Migration-table data-driven hints (UWP API has WinAppSDK equivalent, no equivalent, feature-area hint)
• WUI2xxx — Runtime / layout / XAML pitfalls (raw TabView content, nullable binding paths, ineffective binding modes, null Converter, missing AutomationId, attached-property syntax)
• WUI3xxx — MVVM patterns (old [ObservableProperty] field syntax)
• WUI4xxx — Interop (WebView2 not initialized, removed ONNX Runtime GenAI APIs WUI4101-WUI4103)

Use the installed package's diagnostic help links for rule details. Check inherited x:DefaultBindMode and event/command/converter exceptions before treating an omitted mode as a defect. Fix root causes; any justified false-positive suppression must be narrow and documented, not a blanket NoWarn policy. Keep IL/CsWinRT warnings enabled as well; the WinUI analyzer does not replace AOT/trim analysis.

MVVM Compliance

• [ ] ViewModels extend ObservableObject, use [ObservableProperty] partial properties (not fields)
• [ ] Commands use [RelayCommand] attribute, not manual ICommand implementations
• [ ] No UI types in ViewModels (SolidColorBrush, Visibility, BitmapImage) — these belong in converters or XAML
• [ ] No business logic in code-behind — only navigation, dialog coordination, and event wiring
• [ ] async Task for async methods, async void only for event handlers
• [ ] Never replace ObservableCollection<T> — use .Clear() + re-add

x:Bind and Data Binding

• [ ] Prefer {x:Bind} for known source types; runtime {Binding} needs a justified source and, for AOT, a generated property provider
• [ ] Dynamic values use an effective OneWay/TwoWay mode (explicit or inherited x:DefaultBindMode) plus change notifications; don't rewrite stable event/command/converter bindings to satisfy a blanket mode rule
• [ ] x:DataType on DataTemplates using compiled x:Bind, not on Page
• [ ] No nested nullable paths (e.g., ViewModel.Selected.Name) without FallbackValue

Native AOT / Trimming (When Intended)

• [ ] The published artifact was tested (a Release JIT run is not AOT validation), with IL/CsWinRT warnings fixed rather than suppressed
• [ ] ABI-crossing types are partial; JSON and runtime bindings use source generation — see winui-packaging's references/sourcegen-patterns.md

Accessibility

• [ ] AutomationProperties.AutomationId on every interactive control (Button, TextBox, ComboBox, ToggleSwitch, ListView, NavigationViewItem)
• [ ] AutomationProperties.Name on icon-only buttons and controls without visible text
• [ ] Semantic controls (Button, HyperlinkButton) — not clickable Border/TextBlock
• [ ] No information conveyed by color alone

Theming

• [ ] All colors use {ThemeResource} brushes — no hardcoded #FF0000 or Color="Blue"
• [ ] Typography uses built-in styles (TitleTextBlockStyle, SubtitleTextBlockStyle, BodyTextBlockStyle, CaptionTextBlockStyle) — no raw FontSize
• [ ] Spacing uses 4px grid multiples (4, 8, 12, 16, 24, 32, 48)
• [ ] Corner radius uses ControlCornerRadius / OverlayCornerRadius — not hardcoded values
• [ ] Styles referenced with {StaticResource} not {ThemeResource} (except for brush usage sites)

Security

• [ ] No secrets, API keys, or tokens in source code
• [ ] No Process.Start with unsanitized user input
• [ ] External input validated and sanitized before use
• [ ] File paths from user input not used directly in File.Delete / File.WriteAllText without validation

Performance

• [ ] Long or dynamic lists use ListView/GridView (virtualized), not StackPanel with foreach
• [ ] x:Load for content that's not always visible (e.g., dialogs, secondary panels)
• [ ] Heavy work off UI thread via Task.Run or async/await — never block UI
• [ ] No .Result / .Wait() / .GetAwaiter().GetResult() — these deadlock the UI thread
• [ ] using statements on all disposable objects (Model, Tokenizer, InferenceSession, Generator)

Globalization

• [ ] User-facing strings use x:Uid in XAML and ResourceLoader in C# — not hardcoded
• [ ] String resources in Strings/en-us/Resources.resw (not .resx)
• [ ] Date/number formatting uses CultureInfo.CurrentCulture — not hardcoded formats
• [ ] Layout supports RTL (FlowDirection inherited from root, no absolute positioning that breaks in RTL)
• [ ] No string concatenation for user-facing messages — use string.Format or interpolation with resource strings

Review Report

After reviewing, summarize:

1. Issues found: List each with file, line, and what's wrong
2. Severity: Error (must fix), Warning (should fix), or Note (could improve)
3. Suggested fixes: Specific code changes for each issue

References

For detailed rules with code examples, see references/quality-rules.md — covers performance deep dives (x:Phase, layout optimization), security (PasswordVault, DPAPI, WebView2 hardening), accessibility (keyboard nav, screen readers), code quality (.editorconfig, naming), and globalization (x:Uid patterns, RTL, pluralization).