fix(samples): resolve the guest's own Application for seed colors - #1703
Draft
DerekRaven wants to merge 2 commits into
Draft
fix(samples): resolve the guest's own Application for seed colors#1703DerekRaven wants to merge 2 commits into
DerekRaven wants to merge 2 commits into
Conversation
The Seed Color page threw out of its own constructor in every ALC-hosted guest, so it could not be opened in either the Material or the Simple sample under ThemesSampleApp. - `SemanticThemeHelper` reads `Application.Current`, a process-wide static in the shared Uno.UI that is never assigned for a secondary ALC, so a hosted guest got the deliberately theme-free wrapper and `GetColorsOrThrow()` threw `InvalidOperationException`. - Heads now publish themselves on `NavigationHelper.CurrentApplication` (per-ALC, since SamplesApp.Shared is compiled into each head), matching the existing `NavigationHelper.MainWindow` hand-off. - `SeedColorSamplePage` resolves the theme from that instance via `application.GetTheme()` — the pattern doc/seed-colors.md already prescribes — and degrades to a no-op instead of throwing. - Hosting smoke now asserts each guest publishes its own Application and resolves its own theme; red before this change, green after. - Corrects the specs/03 claim that `BaseTheme` type identity is shared across the ALC boundary: `!Uno.Themes.WinUI` isolates it, so a host-side `GetTheme()` returns null silently. No change to the Uno.Themes public API. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clears the CS8618 introduced by the previous commit (one per sample head). - `NavigationHelper.CurrentApplication` is legitimately null until a head publishes itself, so declare it `Application?` rather than leaving a non-nullable property the compiler cannot prove initialised. - Capture it into a local in `Given_ApplicationExtensions` so the null check informs the compiler's nullable flow — `Assert.IsNotNull` does not. Verified: full rebuild is warning-free for every file this branch touches; runtime tests 175 total, 174 passed, 1 pre-existing [Ignore]. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
GitHub Issue (If applicable): relates to #1698 (see Relationship to #1698 below — it may warrant reopening)
PR Type
What kind of change does this PR introduce?
Description
The Seed Color page (Styles) could not be opened in any ALC-hosted guest — it threw out of its own constructor, in both the Material and the Simple sample running under
ThemesSampleApp:Observed 15 times across two guest sessions in a single run.
Root cause
SemanticThemeHelperresolves the theme throughApplication.Current(SemanticThemeHelper.cs:22).Uno.UIis shared across host and guests (=Uno.UIinGuestSharedAssemblies.txt), andApplication.Currentis deliberately never assigned for a secondary ALC (Application.Alc.cs), so a hosted guest reads the wrapper's application. The wrapper is deliberately theme-free (ThemesSampleApp/App.xaml), so noBaseThemeis ever found.Guests merge their theme into their own
Application.Resources(MaterialSampleApp/App.xaml), which the static helper never reads.SeedColorSamplePagecallsApplySeedColorunconditionally from its constructor, so the failure happens at construction — the page cannot even render.This is a samples-side bug, not a library bug:
Application.Currentis correct for standalone consumers, and the instance-based escape hatch already exists (ApplicationExtensions.GetTheme(this Application), added in #1699) and is already documented indoc/seed-colors.md. The sample simply never adopted its own published guidance.The fix
Follows the idiom this repo already uses to get per-head — therefore per-ALC — state into
SamplesApp.Shared: the head pushes, shared code pulls.NavigationHelper.MainWindowexists for exactly this reason, and all three heads already avoid the sibling trap forWindow.Currentwith a verbatim comment explaining the ALC hazard.Application.Currentis the identical trap; this extends the established fix to it.NavigationHelper.CurrentApplication— new hand-off besideMainWindow. Per-ALC becauseSamplesApp.Sharedis compiled into each head rather than shared as an assembly.InitializeComponent(), so the theme is already merged when shared code picks it up.SeedColorSamplePageresolves the theme from that instance viaapplication.GetTheme()and degrades to a no-op instead of throwing, so the page renders either way (AGENTS.md §8: prefer graceful degradation over throwing from resource-resolution paths).No change to the
Uno.Themespublic API surface.Relationship to #1698
#1698 ("SemanticThemeHelper cannot access BaseTheme for a nested ALC application") was closed as completed by #1699, which added
GetTheme(this Application). That API landed, but no consumer was migrated to it, so the user-visible symptom survived — this PR is what actually fixes it.Two further notes on #1698's premise, which was framed from the host side ("a host that holds the nested app's
Applicationcannot read that app's theme"):ThemesSampleApp, one of the two scenarios SemanticThemeHelper cannot access BaseTheme for a nested ALC application #1698 names.GuestSharedAssemblies.txtdeclares!Uno.Themes.WinUI— always isolate per-ALC, isolation wins over sharing — soBaseThemetype identity is not shared. A host-sideguestApp.GetTheme()matches the host's ownBaseThemetype against dictionaries built from the guest's, finds nothing, and returnsnullsilently.specs/03-seed-color-palette/seed-color-palette.mdasserted the opposite ("BaseThemetype identity is shared and the plainOfType<BaseTheme>match works across the ALC boundary"). Corrected here to state thatGetTheme()must be called from inside the guest's own ALC, and that host code must match by type name (asGuestHostingSmokenow does).Hot Design's guest hosting may or may not share
Uno.Themes.WinUI— if it isolates it too, the #1698 API cannot serve that scenario either, which is why #1698 may warrant reopening.Why CI did not catch this
GuestHostingSmoke.RunAsyncloops the catalog doing load → "is hosted" → reclamation check → unload. It never opens a sample page, so it proved guests load, not that guest pages work. That is the gap this bug walked through.This PR adds a
CheckGuestThemeIsReachablestep asserting each guest publishes its ownApplicationand that it resolves the guest's own theme, plus a minimalinternal GuestAppLoader.CurrentGuestAppfor the smoke to reach it.It deliberately asserts the invariant rather than constructing the page: building a guest visual tree in the smoke would add ALC roots and could destabilise the Release reclamation assertion, failing the
HostingSmoke_Desktopgate for reasons unrelated to theming. Systematic per-page coverage is noted as a follow-up.Verification
Red/fix/green on the real hosted failure (
--smoke, desktop under xvfb):RESULT: FAIL— "Simple did not publish NavigationHelper.CurrentApplication"RESULT: PASS— Material and Simple each resolve their own themeRuntime tests (
SimpleSampleApp, desktop): 175 total, 174 passed, 0 failed. The single non-pass isWhen_BaseThemeIsCollected_Then_HotReloadHandlerDoesNotResurrectIt, which carries a pre-existing[Ignore]onmaster. AddsWhen_RunningStandalone_Then_CurrentApplicationIsPublishedAndResolvesSameThemetoGiven_ApplicationExtensions(passes) — this guards the standalone hand-off; the hosted path is unreachable from the in-app runner (Application.Currentis the app under test by construction) and is covered by the smoke instead.Build:
ThemesSampleAppdesktop, 0 errors, no new warnings.Interactive: relaunched on desktop; Styles → Seed Color opens in both Material and Simple, the picker re-themes live, and no
No BaseThemeentry appears in the log.A note on the smoke's Cupertino line — "merges no BaseTheme (expected for this design)" — is not a defect: there is no
CupertinoThemetype at all (onlyMaterialThemeandSimpleThemederive fromBaseTheme), which is why the page declaresSupportedDesigns = { Material, Simple }. The check enforces only the hand-off that every guest owes.Follow-ups (not in this PR)
Given_ApplicationExtensionsassertsApplication.Currenthas aSimpleThememerged — correct standalone, but it would fail if the runtime tests were ever run inside a hosted guest (the runner is a reachable sample page). Needs a hosting-aware variant.Given_SeedColorPalette.csActiveThemeKeyreadsApplication.Current.RequestedTheme, i.e. the host's, whileRequestedThemeis pinned per guest at theAlcContentHostboundary — so it can select the wrong theme dictionary key under hosting.SamplePageper guest would catch this whole class of bug rather than one instance. Kept out here because it will likely surface unrelated pre-existing page failures needing their own triage.--sample=/?sample=selector, whichGuestAppDeepLinkalready documents as the intended guest extension point but which no guest-side reader exists for.PR Checklist
🤖 Generated with Claude Code