Skip to content

fix(samples): resolve the guest's own Application for seed colors - #1703

Draft
DerekRaven wants to merge 2 commits into
masterfrom
dev/dr/seed-color-alc-hosting
Draft

fix(samples): resolve the guest's own Application for seed colors#1703
DerekRaven wants to merge 2 commits into
masterfrom
dev/dr/seed-color-alc-hosting

Conversation

@DerekRaven

Copy link
Copy Markdown
Contributor

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?

  • Bugfix

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:

SeedColorSamplePage..ctor()
  → SelectedIndex = … → SeedColorModeCombo_SelectionChanged
  → ApplySeedColor → SemanticThemeHelper.set_SeedColorMode → GetColorsOrThrow()
  → InvalidOperationException: No BaseTheme (MaterialTheme, SimpleTheme, etc.)
    found in Application.Current.Resources.MergedDictionaries.

Observed 15 times across two guest sessions in a single run.

Root cause

SemanticThemeHelper resolves the theme through Application.Current (SemanticThemeHelper.cs:22). Uno.UI is shared across host and guests (=Uno.UI in GuestSharedAssemblies.txt), and Application.Current is 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 no BaseTheme is ever found.

Guests merge their theme into their own Application.Resources (MaterialSampleApp/App.xaml), which the static helper never reads. SeedColorSamplePage calls ApplySeedColor unconditionally 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.Current is correct for standalone consumers, and the instance-based escape hatch already exists (ApplicationExtensions.GetTheme(this Application), added in #1699) and is already documented in doc/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.MainWindow exists for exactly this reason, and all three heads already avoid the sibling trap for Window.Current with a verbatim comment explaining the ALC hazard. Application.Current is the identical trap; this extends the established fix to it.

  • NavigationHelper.CurrentApplication — new hand-off beside MainWindow. Per-ALC because SamplesApp.Shared is compiled into each head rather than shared as an assembly.
  • All three heads publish themselves after InitializeComponent(), so the theme is already merged when shared code picks it up.
  • SeedColorSamplePage resolves the theme from that instance via application.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.Themes public 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 Application cannot read that app's theme"):

  • That host-side path still does not work for ThemesSampleApp, one of the two scenarios SemanticThemeHelper cannot access BaseTheme for a nested ALC application #1698 names. GuestSharedAssemblies.txt declares !Uno.Themes.WinUI — always isolate per-ALC, isolation wins over sharing — so BaseTheme type identity is not shared. A host-side guestApp.GetTheme() matches the host's own BaseTheme type against dictionaries built from the guest's, finds nothing, and returns null silently.
  • specs/03-seed-color-palette/seed-color-palette.md asserted the opposite ("BaseTheme type identity is shared and the plain OfType<BaseTheme> match works across the ALC boundary"). Corrected here to state that GetTheme() must be called from inside the guest's own ALC, and that host code must match by type name (as GuestHostingSmoke now 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.RunAsync loops 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 CheckGuestThemeIsReachable step asserting each guest publishes its own Application and that it resolves the guest's own theme, plus a minimal internal GuestAppLoader.CurrentGuestApp for 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_Desktop gate 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):

State Verdict
Head registrations reverted RESULT: FAIL"Simple did not publish NavigationHelper.CurrentApplication"
Fix applied RESULT: PASS — Material and Simple each resolve their own theme

Runtime tests (SimpleSampleApp, desktop): 175 total, 174 passed, 0 failed. The single non-pass is When_BaseThemeIsCollected_Then_HotReloadHandlerDoesNotResurrectIt, which carries a pre-existing [Ignore] on master. Adds When_RunningStandalone_Then_CurrentApplicationIsPublishedAndResolvesSameTheme to Given_ApplicationExtensions (passes) — this guards the standalone hand-off; the hosted path is unreachable from the in-app runner (Application.Current is the app under test by construction) and is covered by the smoke instead.

Build: ThemesSampleApp desktop, 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 BaseTheme entry 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 CupertinoTheme type at all (only MaterialTheme and SimpleTheme derive from BaseTheme), which is why the page declares SupportedDesigns = { Material, Simple }. The check enforces only the hand-off that every guest owes.

Follow-ups (not in this PR)

  • Given_ApplicationExtensions asserts Application.Current has a SimpleTheme merged — 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.cs ActiveThemeKey reads Application.Current.RequestedTheme, i.e. the host's, while RequestedTheme is pinned per guest at the AlcContentHost boundary — so it can select the wrong theme dictionary key under hosting.
  • Systematise the smoke's page coverage — constructing every registered SamplePage per 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.
  • Implement the anticipated --sample= / ?sample= selector, which GuestAppDeepLink already documents as the intended guest extension point but which no guest-side reader exists for.

PR Checklist

  • Commits follow the Conventional Commits specification
  • Tested the changes where applicable:
    • UWP
    • iOS
    • Android
    • WASM
    • MacOS
    • Desktop (Skia/X11 — smoke + runtime tests + interactive)

Platform coverage is deliberately narrow: the change is desktop-verified only. The guest-hosting wrapper supports desktop and browserwasm only, and WASM was not exercised here — the in-browser smoke (?smoke) has a harness but no CI driver yet. The edits are platform-neutral C# in shared sample code, so no platform-specific risk is expected, but WASM remains unverified.

🤖 Generated with Claude Code

DerekRaven and others added 2 commits August 21, 2026 11:21
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant