Refine gateway setup wizard UX and fix accessibility gaps#940
Conversation
UX refactor (setup wizard): - Replace radio-button option pickers with single-selection ListView (ItemsView/ItemContainer), first item selected by default. - Standardize the bottom action bar: standard-size Back button, AccentButtonStyle primary, Subtle "Skip"/"Start over" buttons, and a subtle "More options" control. - Move "Skip for now" to the top of every option list. - Replace the "more"/"back" radio entries with a subtle "More v" expander that expands the full list in a single click while preserving the "Skip for now" option, and a dedicated Back button. - Add a filter textbox above long model lists. - Render wizard transcript/security text as selectable RichTextBlock. Bug fix: - ExpandMoreOptionsAsync now updates _stepId from the expanded payload so selecting a newly revealed provider (e.g. Copilot) and pressing Continue no longer fails with a stale step id. Accessibility (verified live with Axe.Windows, 0 violations after): - Give SelectOptions ListView items an AutomationProperties.Name (label + hint) via a shared CreateOptionItem helper; multiselect checkboxes named the same way. Fixes NameNotNull on setup-mode, config-handling, and provider option lists. - Name the three CapabilitiesPage radio buttons (Read-only, Standard, Full access). Fixes NameNotNull on the capabilities step. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Codex review: needs changes before merge. Reviewed July 7, 2026, 5:09 PM ET / 21:09 UTC. Summary Reproducibility: yes. Source inspection on the current PR head shows WizardBack_Click re-renders local payload history without changing gateway state, and WelcomePage.xaml uses pointer-only Borders for setup choices. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Land the UX and accessibility refresh after Back is synchronized with gateway-owned wizard state and the welcome choices remain real invokable controls with focused proof. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection on the current PR head shows WizardBack_Click re-renders local payload history without changing gateway state, and WelcomePage.xaml uses pointer-only Borders for setup choices. Is this the best way to solve the issue? No. The UX direction is useful, but the implementation should preserve gateway ownership of wizard state and keep setup choices as real invokable controls. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2c6873e8f20f. Label changesLabel justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (2 earlier review cycles)
|
The 'More v' overflow toggle is inserted as a sibling of SelectOptions in the shared StackPanel, but ResetInputs only cleared SelectOptions.Items. Advancing past a select step without clicking More left the button orphaned on subsequent steps. Track it and remove it in ResetInputs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
shanselman
left a comment
There was a problem hiding this comment.
Reviewed with >90% confidence. Found and fixed one stale overflow-button lifecycle issue, reran local validation, and refreshed CI is green.
|
Thanks — both review findings are addressed in [P2] Invokable welcome choices ( [P1] Back vs. gateway wizard state ( Added source-contract tests for both invariants. Validation: |
What & why
Refines the OpenClaw gateway setup wizard UX and fixes real accessibility gaps found while walking the flow.
UX
ListView(ItemsView/ItemContainer), with the first item selected by default.AccentButtonStyleContinue, subtle Skip / Start over / More options controls.RichTextBlock).Bug fix
ExpandMoreOptionsAsyncnow updates_stepIdfrom the expanded payload, so selecting a newly revealed provider (e.g. Copilot) and pressing Continue no longer fails with a stale step id (previously flipped the primary button to "Start wizard again").Accessibility
WizardPageoption items (and multiselect checkboxes) getAutomationProperties.Name(label + hint) via a sharedCreateOptionItemhelper.CapabilitiesPageprofile radios (Read-only / Standard / Full access) getAutomationProperties.Name.Validation
./build.ps1— ✅ all projects built (win-arm64)dotnet test OpenClaw.Shared.Tests— ✅ 2697 passed, 31 skippeddotnet test OpenClaw.Tray.Tests— ✅ 1579 passedReal behavior proof
Screenshots + Axe.Windows (v2.4.1) scans captured against the live isolated dev app on the current PR head. Full set and details in
docs/setup-wizard-ux/.Accessibility — actionable violations (
NameNotNull), before → after:Setup mode — single-selection ListView, first item selected:
Provider list — "Skip for now" pinned top, subtle More ▾, standardized bottom bar:
Provider list expanded — one click, "Skip for now" preserved, full list:
Capabilities — radios now expose accessible names (0 violations):
Known framework limitation (not fixed)
Inline
Hyperlinkinside aRichTextBlock(security disclaimer link) reports a nullBoundingRectangleto UIA when scrolled out of view — a WinUI behavior for inline hyperlinks, not a control we construct.