Skip to content

Commit 91d756d

Browse files
committed
fix(theme): apply component theme reliably so components are not unstyled
1 parent 507dbf4 commit 91d756d

2 files changed

Lines changed: 72 additions & 35 deletions

File tree

core/scripts/testing/scripts.js

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,27 @@ const DEFAULT_PALETTE = 'light';
118118
paletteName = DEFAULT_PALETTE;
119119
}
120120

121+
/*
122+
* Tracks when the asynchronous theme token loading has finished injecting
123+
* the component theme CSS. Tests wait on this (see the Playwright `goto`
124+
* helper) so screenshots are not taken before the themed CSS variables are
125+
* available, which would render components unstyled.
126+
*/
127+
window.__ionicTestThemeReady = false;
128+
121129
// Load theme tokens if the theme is valid
122130
const validThemes = ['ionic', 'ios', 'md'];
123131
if (themeName && validThemes.includes(themeName)) {
124-
loadThemeTokens(themeName, paletteName);
125-
} else if(themeName) {
126-
console.warn(
127-
`Unsupported theme "${themeName}". Supported themes are: ${validThemes.join(', ')}. Defaulting to ${DEFAULT_THEME}.`
128-
);
132+
loadThemeTokens(themeName, paletteName).finally(() => {
133+
window.__ionicTestThemeReady = true;
134+
});
135+
} else {
136+
if (themeName) {
137+
console.warn(
138+
`Unsupported theme "${themeName}". Supported themes are: ${validThemes.join(', ')}. Defaulting to ${DEFAULT_THEME}.`
139+
);
140+
}
141+
window.__ionicTestThemeReady = true;
129142
}
130143

131144
/**
@@ -148,6 +161,25 @@ const DEFAULT_PALETTE = 'light';
148161
return result;
149162
};
150163

164+
/*
165+
* Resolves once the Ionic Config instance (created by `initialize()` in
166+
* ionic-global.ts) is available. If the app has already loaded we resolve
167+
* immediately; otherwise we wait for the `appload` event, which fires after
168+
* `initialize()` has set up the config. This is event-driven rather than
169+
* polled, so there's no arbitrary timeout. JavaScript's single-threaded
170+
* execution guarantees `appload` cannot fire between the synchronous check
171+
* and `addEventListener`, so there is no missed-event race.
172+
*/
173+
function whenConfigReady() {
174+
return new Promise((resolve) => {
175+
if (window.testAppLoaded === true || window.Ionic?.config?.set) {
176+
resolve();
177+
} else {
178+
window.addEventListener('appload', () => resolve(), { once: true });
179+
}
180+
});
181+
}
182+
151183
// TODO(FW-6750): Determine if this function can be removed once the theme tokens can be imported directly into the test pages
152184
async function loadThemeTokens(themeName, paletteName) {
153185
try {
@@ -156,7 +188,6 @@ const DEFAULT_PALETTE = 'light';
156188
// Load the default tokens for the theme
157189
const defaultTokens = await import(`/themes/${themeName}/default.tokens.js`);
158190
let theme = defaultTokens.defaultTheme;
159-
160191
// Merge with existing theme to preserve any customizations
161192
if (customTheme) {
162193
theme = deepMerge(theme, customTheme);
@@ -175,39 +206,36 @@ const DEFAULT_PALETTE = 'light';
175206
theme.palette.highContrastDark.enabled = 'always';
176207
}
177208

178-
if (window.Ionic?.config?.set) {
179-
/**
180-
* New Page Load after Initial App Load or Playwright Test:
181-
*
182-
* If the Config instance exists, we must use the
183-
* `set()` method. This ensures the internal private Map inside
184-
* the `Config` class is updated with the loaded theme tokens.
185-
* Without this, components would read 'undefined' or 'base'
186-
* values from the stale Map when trying to access them through
187-
* methods like `config.get()`.
188-
*/
189-
window.Ionic.config.set('customTheme', theme);
190-
} else {
191-
/**
192-
* App Initialization or Browser Refresh:
193-
*
194-
* If the Config instance doesn't exist yet,
195-
* we attach the theme to the global Ionic object. The `initialize()`
196-
* method in `ionic-global.ts` will later merge this into the new
197-
* `Config` instance via `config.reset()`.
198-
*/
199-
window.Ionic = window.Ionic || {};
200-
window.Ionic.config = window.Ionic.config || {};
209+
window.Ionic = window.Ionic || {};
210+
window.Ionic.config = window.Ionic.config || {};
211+
212+
/**
213+
* App Initialization or Browser Refresh:
214+
*
215+
* If the Config instance doesn't exist yet, stash the theme on the
216+
* global Ionic object so `initialize()` in ionic-global.ts can merge it
217+
* into the new Config instance via `config.reset()`.
218+
*/
219+
if (!window.Ionic.config.set) {
201220
window.Ionic.config.customTheme = theme;
202221
}
203222

204223
/**
205-
* Re-applying the global theme is critical for Playwright tests.
206-
* Even if the config is set, the CSS variables for the specific theme
207-
* (e.g., md or ios) must be force-injected into the document head to
208-
* ensure visual assertions pass correctly.
224+
* Wait for the Config instance to be created by `initialize()`, then set
225+
* the theme and force-inject the global and component CSS ourselves.
226+
*
227+
* This avoids a race: if `initialize()` runs before this async import
228+
* resolves, it applies the base theme (which has no component tokens) and
229+
* replaces the global Ionic.config, orphaning the stash above. By always
230+
* applying here once the Config instance exists, the component CSS
231+
* variables (e.g. --ion-badge-*) are reliably injected regardless of
232+
* ordering, instead of flaky unstyled renders.
209233
*/
210-
if (window.Ionic?.config?.get && window.Ionic?.config?.set) {
234+
await whenConfigReady();
235+
236+
if (window.Ionic?.config?.set) {
237+
window.Ionic.config.set('customTheme', theme);
238+
211239
const themeModule = await import('/themes/utils/theme.js');
212240
themeModule.applyGlobalTheme(theme);
213241
themeModule.applyComponentsTheme(theme);

core/src/utils/test/playwright/page/utils/goto.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,16 @@ configs().forEach(({ config, title }) => {
9999
});
100100

101101
const result = await Promise.all([
102-
page.waitForFunction(() => (window as any).testAppLoaded === true, { timeout: 4750 }),
102+
/*
103+
* Wait for the app to load and for any asynchronous theme token loading to
104+
* finish injecting the component theme CSS. `__ionicTestThemeReady` is only
105+
* set by the test scripts that load themes; when it is undefined (e.g. pages
106+
* that don't run those scripts) the `!== false` check lets the test proceed.
107+
*/
108+
page.waitForFunction(
109+
() => (window as any).testAppLoaded === true && (window as any).__ionicTestThemeReady !== false,
110+
{ timeout: 4750 }
111+
),
103112
originalFn(formattedUrl, options),
104113
]);
105114

0 commit comments

Comments
 (0)