Upgrade Uno to 6.7.22 and fix left-nav selection after new session - #174
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
YoungSx
added a commit
that referenced
this pull request
Sep 4, 2026
Every gate workflow filtered pull_request to `branches: [develop]`, so a PR opened against main ran no gates at all - the seven workflows only fired on `push`, which is after the merge. A develop-targeted PR collects 14 checks; #174 against main collected two, both Vercel deploy previews. That leaves main-targeted work unguarded exactly where it matters most: Browser WASM Smoke Gates, Windows MSIX Package and Skia Desktop GUI Smoke cannot run on an arm64 Linux dev box, so for those targets the pull request gate is the only pre-merge signal that exists. `push` already covered [main, develop] in all seven, so this only aligns pull_request with it. No job, step, path filter or permission changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6.7 is the first stable line that contains unoplatform/uno#21132, which re-syncs NavigationView with the latest WinUI sources and fixes the container-recycling selection defect behind #171. 6.8 is not an option: NuGet only publishes 6.8.0-dev.* and global.json sets allowPrerelease: false. Uno.WinUI.Lottie is the only Uno package pinned explicitly, so it moves with the line (6.6.166 -> 6.7.103); the rest stay implicit Uno.Sdk versions. 6.7 also pulls in the transitive WebView2Aot package, whose WebView2.dll declares a top-level "WebView2" namespace. A namespace in the global namespace wins simple-name lookup over a using-imported type, so the bare name in XtermTerminalView stopped resolving to the XAML control (CS0118); name the control type in full. net10.0-desktop builds Release with 0 warnings and 0 errors and the Presentation.Core suite is green. net10.0-browserwasm still fails UNOWA0001 on this machine, unchanged from 6.6.29 - WasmNativeWorkloadAvailable is false on both SDK versions - so the WASM head needs CI to verify. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UnoNumberBoxThemeOverrides.xaml is a copy of the framework template kept for unoplatform/uno#24021, which is still open, so the override stays. Its provenance comment pins an upstream version and a compliance test asserts that string, so the copy has to be re-checked against the new line rather than have the number bumped. Diffing the upstream template between the 6.6.166 commit and 6.7 shows a single change: PlaceholderTextContentPresenter now leaves the accessibility tree via AutomationProperties.AccessibilityView="Raw". Adopt it, then re-anchor the comment and the assertion to 6.7.103. The AppWindow.Closing remark in MainPage.Shutdown.cs cites a decompile of 6.6.166. Only the symbol's presence was re-checked on 6.7.103, not each host's value, so record that limit instead of silently re-pointing the citation at a version nobody verified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Creating a session from the Start page left the pane's selection visual on a different session in the same project (#171). ApplySelectionProjection suppressed the SelectedItem notification whenever the newly projected item was reference-equal to the previously projected one. That guard is deliberate - republishing on every pass is what stranded the gray selection mask across rows - but it keyed on the wrong thing. SyncSessions reuses the row view models on purpose, so inserting a newly created session at the top of its project changes the menu source while leaving the projection reference-equal: every container below the insert now renders a different row, the control is never told, and a OneWay binding offers no readback to notice. The contract the existing tests describe is "stay quiet while the native menu source is stable", so track a structural revision of Items and of each project's Children and re-publish once per revision. Pane toggles and no-op refreshes stay quiet exactly as before. TryMaterializeSession also called RebuildTreeCore() synchronously from inside selection change notifications, mutating the bound collections under the handler that raised them - and RebuildTreeCore ends in NormalizeSelectionAfterRebuild, so the path re-entered itself. Route it through the existing coalescing scheduler, which folds a request raised mid-rebuild into the pending flag and re-schedules it as the running pass unwinds. It still reports that this pass must not publish, because the projection available before materialization resolves to a null selected item and pushing that null costs NavigationView its IsChildSelected ancestor visual. Both changes are pinned by tests that were reverse-verified: reverting the two implementation edits turns exactly these two tests red and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
openApp threw a bare `page.waitForSelector: Timeout 180000ms exceeded` and nothing else, so a boot regression was indistinguishable from a slow cold start. Everything that explains it - a runtime abort, a framework asset that 404s, a JS module that failed to load - had already gone by as console output, and assertNoFatalConsoleMessages never ran because openApp threw first. fatalConsolePattern also keeps only a known-fatal subset, so a new failure shape is dropped by design. Keep a bounded tail of every console and pageerror entry alongside the fatal-filtered list, then attach page state plus both buffers to the error openApp raises. The WASM head cannot be built on every contributor machine, so the gate log has to be self-describing; "reproduce it locally" is not available as a fallback. Diagnostics only: the capture is additive and the new text is produced on the throw path, so a passing smoke behaves exactly as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first diagnostics pass proved the WASM app renders on Uno 6.7 - title, body text and readyState were all correct with zero console or page errors - and that not one element carried aria-label. Uno's WASM semantic DOM decides per ARIA role whether a name lands on aria-label, on aria-labelledby, or as text content, so an empty aria-label set says nothing about which selector would match. Report the attribute names the page actually carries plus a body outerHTML sample, so the locator strategy can be fixed against observed DOM instead of a guess. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uno 6.7's legacy WebAssembly DOM backend still renders the app - title, body text and readyState were all correct with zero console or page errors - but no element carries an accessible name any more. The instrumented smoke gate showed the page's only identifying attributes are id, role and three data-* hooks, with `xamltype`/`uno-*` classes on each div and not one aria-* name. We author AutomationProperties.Name across 13 XAML files, so this is a screen-reader regression for real users, not merely a broken test locator; adapting the smoke selectors would have hidden it. Every accessibility fix in the 6.7 line targets the Skia WASM semantic DOM instead of the DOM backend (unoplatform/uno#23456, #23722, #24383), and Uno 7 removes the Android, iOS and WebAssembly DOM backends outright - "rendering is Skia everywhere". So the browser head moves to the renderer desktop already uses rather than staying on a backend upstream no longer maintains. Verified locally: browserwasm now resolves UnoFeatures ";skiarenderer;webview;", desktop is unchanged, and all three WasmShellNativeFileReference JS modules plus WasmShellEnableIDBFS survive the switch. Rendering, input and the smoke gates can only be confirmed in CI - the WASM head does not build on an arm64 dev box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ocating With the browser head on Skia, aria-label came back but every [aria-label] locator still waited out its full timeout. The diagnostics explained why: bodyText was empty and the whole body was an "Enable accessibility" button plus an "Application content" placeholder. Skia paints into a <canvas>, so the accessibility tree is the only DOM Uno mirrors, and it builds that lazily - until a screen reader is detected or that affordance is activated, no AutomationProperties.Name reaches the DOM. Playwright is not a screen reader. Activate it in openApp so all six WASM smokes get the tree. The affordance renders empty despite role="button" and tabindex="0", so it has no hit box: dispatch focus, click and Enter on the element rather than aiming a pointer at it. A missing affordance is not an error, since the DOM-rendered heads never had one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…one matched Activating the accessibility tree got openApp past first paint, and the smoke now fails one layer deeper: no visible navigation item for `TitleBar.ToggleSidebar`. The candidate dump shows why. Every element reports `automationId: ""` - Skia's semantic DOM publishes no data-automation-* attribute - and the target is there under the accessible name "Toggle sidebar". So aria-label now carries AutomationProperties.Name where one is authored and falls back to the AutomationId otherwise, which is why id-only elements such as StartView.Title still match while named ones do not. The matcher already probed three data-automation spellings; add the plain id as a last resort in case that is where the AutomationId now lives, and report both the raw id and the accessible name in the candidate dump so a miss names which identifier Skia published rather than only which one we asked for. Diagnostic and additive: the extra probe cannot displace an existing match, and 41 automationIds call sites across eight smokes are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The candidate dumps showed every element reporting an empty automation id and
the DOM id being uno-semantics-<handle>, which led me toward reconstructing an
AutomationId-to-accessible-name map from XAML and resw. Uno's own reference makes
that unnecessary and says why the ids were missing:
AutomationId | Web (WASM) | Skia / Native
-> `xamlautomationid` attribute + `aria-label` on the HTML element
"To avoid performance overhead, AutomationId only has an effect when the
IsUiAutomationMappingEnabled MSBuild property is set to true"
So AutomationId was never being published - the mapping is opt-in - and the
attribute to read is `xamlautomationid`, which the matcher did not probe. It is
not a name-versus-id conflict and needs no mapping table: the 38 automationIds
call sites across eight smokes are correct as written.
Enable the mapping in the gate build rather than the project file, because the
documented reason it defaults off is runtime cost; shipped builds keep it off.
The clean step takes the same property so an incremental run cannot serve output
built without it. Probe `xamlautomationid` first and keep the data-automation-*
spellings for the older heads; drop the speculative plain-id probe.
Ref: Uno docs features/accessibility/automation-properties.md (release/stable/6.7).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The settings-navigation smoke was failing on the overflow menu, and the reason it was fragile is the same reason it was hard to read: it described a click sequence rather than a goal, and the parts it leaned on were implementation, not contract. Four couplings removed: - The overflow affordance was found by the Segoe Fluent private-use glyphs and , then re-targeted through Uno's internal `.uno-button` class. A new icon or a renamed class broke the gate with no behaviour change. It is now found by accessible name, which is what Uno publishes and what a screen reader uses. - Arrival was judged only by a multi-language prose alternation over body text. Such a pattern is satisfied by any one branch appearing anywhere on the page, so it cannot tell "arrived" from "that word is also here". The section must now report itself active - selected tab, heading, or aria-current - which is the state the navigation is supposed to produce. The caller's pattern is still honoured, but it is no longer the only evidence. - navigateToSettingsSection resized the window to force the overflow route and restored it in a `finally`, while the smoke performed the same resize and overflow click again. Two viewport mutations made their order load-bearing. The caller now owns its viewport; the helper owns route selection. - Locale is pinned to en-US when the context is created. Accessible names are localized by design, so an unpinned locale is what pushed every assertion into a language alternation in the first place. Rect-based activation is kept deliberately: Uno's semantic nodes are a visually hidden overlay beside the canvas, so Playwright's actionability checks refuse to click them. That is a renderer property, not a shortcut. navigateToSettingsSection keeps its signature - 24 call sites across six smokes depend on it - so this changes how it identifies and verifies, not its contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The WASM smokes were coordinate-geometry scrums: rectangle centers, Tab loops, Escape retries, hardcoded ComboBox indexes, six near-identical full-DOM scanners, and a bottom-right-button heuristic. All of it probed renderer internals that nothing promises. Uno's WASM semantic DOM (enabled by IsUiAutomationMappingEnabled) is the stable contract: nodes under #uno-semantics-root carry aria-label and xamlautomationid, and Uno programs each node's click to its peer's Invoke/Toggle/Selection/ExpandCollapse without hit testing - so program matic element.click() is the documented activation route, needing no scroll and no coordinate. Text boxes and sliders are real <input>s. - ui-affordances.mjs: rewritten as thin node-side drivers over one in-page semantic runtime (window.__salmoneggSmoke.semantic, injected by browser-app.mjs). AutomationIds win over labels; failures dump the semantic DOM instead of guessed rectangles. Deleted findVisibleNavigationTargetPoint, the index maps, and the keyboard fallbacks. - browser-app.mjs: inject the semantic runtime alongside the existing helper; findVisibleControl also honors xamlautomationid. - settings-shell.mjs: rebuilt on the new primitives; overflow route tries semantic first, keeps the rect scanner only as a fallback for the template-generated overflow button whose name is not in our resw. - gamepad smoke: Expander reveal prefers semantic toggle, keeps the real-mouse fallback (its unreliable-synthetic-click measurement predates the semantic contract); start-button click goes semantic. - ChatInputArea: real AutomationIds for Send/Cancel, replacing the bottom-right button heuristic. The named AutomationIds that the old heuristics guessed at are the same ones screen readers announce - testing through them tests behaviour. Co-Authored-By: Claude <noreply@anthropic.com>
The section-active predicate waited for the navigation item to carry aria-selected or aria-current. NavigationViewItem is rendered through Uno's generic semantic path, which publishes neither attribute on Skia WASM - so the wait could never succeed and burned its timeout before every failure. Arrival was already asserted where it is observable: the caller's bodyPattern pins strings that exist only on that section's page. Co-Authored-By: Claude <noreply@anthropic.com>
The smoke still queried [aria-label=<AutomationId>], the pre-6.7 DOM shape. Under IsUiAutomationMappingEnabled the id lands on xamlautomationid and the accessible name is the card title, so every lookup returned null and the gate failed before reaching the tip-card assertion. Locate the cards the same way every other smoke does, keep the localized-title assertion as the accessible-name contract, and drop the alpha/opacity checks that have no mirror in the semantic tree. Co-Authored-By: Claude <noreply@anthropic.com>
These buttons carry no AutomationProperties.Name, so their aria-label only coincidentally equals the automation id (the nameless fallback in the semantic DOM). xamlautomationid is the id contract the semantic library itself matches on, and it survives a Name being added later. Co-Authored-By: Claude <noreply@anthropic.com>
The cards' AutomationId is x:Bind-fed, and Uno's BrowserWasm semantic mapping bakes the automation id into the semantic element at node creation - before bindings resolve - and never revises it, so the tree still carries the button's x:Name fallback (HeroSuggestionButton) while the bound id never appears. Static ids survive the bake; dynamic ones never do. Windows UIA reads the bound id live, so the ViewModel ids remain valid for real consumers - they are just unobservable through this tree. The behaviour a screen reader consumer sees is a button announced by its localized title, so match by name and pin the match to role=button. Co-Authored-By: Claude <noreply@anthropic.com>
matchNode's label fallback accepts a textContent hit, and the hero card's title TextBlock renders before its button's Name binding lands - so the first poll settled on the title div. Add an optional role filter to matchNode (no role passed: unchanged for every existing caller) and pin the card waits and activation to role=button, letting the poll continue until the announced button is actually there. Co-Authored-By: Claude <noreply@anthropic.com>
…tic one CI showed the hero card's command never fires through the semantic activate path (element.click): the tip dialog never opens and the smoke times out on its copy. That is the same BrowserWasm synthetic-click gap the expander toggle and the gamepad refresh button already documented. Playwright's trusted click on the same semantic node the screen reader exposes ([role=button][aria-label=...]) is the closest stand-in for the user's gesture, and the old full-DOM version of this smoke was already passing exactly that way. Co-Authored-By: Claude <noreply@anthropic.com>
YoungSx
force-pushed
the
fix-nav-focus-issue-171
branch
from
September 4, 2026 14:11
0c2febe to
b7f5656
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #171
Summary
Creating a session from the Start page left the left-nav selection visual on a different session in the same project. Two independent causes, one upstream and one ours.
Upstream —
Uno.Sdk6.6.29 → 6.7.22. 6.7 is the first stable line containing unoplatform/uno#21132, which re-syncsNavigationViewwith the latest WinUI sources and fixes the container-recycling selection defect. Verified by containment rather than by release notes:compare/release/stable/6.7...bdee6d9ereportsahead_by=0, whilerelease/stable/6.6isdiverged. 6.8 is not available — NuGet only publishes6.8.0-dev.*andglobal.jsonsetsallowPrerelease: false.Ours — the selection projection lied, and it mutated the tree under itself.
ApplySelectionProjectionsuppressed theSelectedItemnotification whenever the newly projected item was reference-equal to the previously projected one. That guard is deliberate (republishing on every pass is what stranded the gray selection mask across rows) but it keyed on the wrong thing:SyncSessionsreuses the row view models on purpose, so inserting a newly created session at the top of its project changes the menu source while leaving the projection reference-equal. Every container below the insert then renders a different row, the control is never told, and aOneWaybinding gives no readback to notice. The contract the existing tests already describe is "stay quiet while the native menu source is stable" — so this tracks a structural revision ofItemsand of each project'sChildrenand re-publishes once per revision. Pane toggles and no-op refreshes stay quiet exactly as before.TryMaterializeSessionalso calledRebuildTreeCore()synchronously from inside selection change notifications, mutating the bound collections under the handler that raised them — andRebuildTreeCoreends inNormalizeSelectionAfterRebuild, so the path re-entered itself. It now goes through the existing coalescing scheduler.Changes
build(uno)WebView2control typechore(styles)fix(nav)WebView2needed qualifying because 6.7 pulls in the transitiveWebView2Aotpackage, whoseWebView2.dlldeclares a top-levelWebView2namespace. A namespace in the global namespace wins simple-name lookup over a using-imported type, so the bare name stopped resolving to the XAML control (CS0118).The NumberBox override is a copy of the framework template kept for the still-open unoplatform/uno#24021, pinned to an upstream commit that a compliance test asserts. Rather than bump the string, the upstream template was diffed between the 6.6.166 commit and 6.7: exactly one change,
PlaceholderTextContentPresentergainingAutomationProperties.AccessibilityView="Raw". Adopted, then re-anchored.Tested
Presentation.Coresuite: 3293 passed / 0 failednet10.0-desktopRelease: 0 warnings, 0 errorsNew tests:
RebuildTree_RepublishesSelectedItemBinding_WhenNewerSessionIsInsertedAboveSelection— asserts the projected instance is unchanged yet a notification still fires, which is precisely what an instance-keyed guard missesSessionSelectionWithoutNavRow_SchedulesRebuild_RatherThanMutatingTheBoundTreeInline— usesQueuedSynchronizationContextto assert the tree is untouched at selection time and only materializes after the queue drainsNot verified
net10.0-browserwasmwas not built. It failsUNOWA0001on this arm64 machine. Attributed, not assumed:WasmNativeWorkloadAvailableisfalseon both 6.6.29 and 6.7.22, and both bootstrap versions carry the diagnostic — so this predates the upgrade. Needs CI.AppWindow.Closingremark inMainPage.Shutdown.cscites a decompile of 6.6.166. Only the symbol's presence was re-checked on 6.7.103, not each host's value; the comment now says so rather than silently re-pointing at an unverified version.🤖 Generated with Claude Code