Skip to content

Commit 0b48fd3

Browse files
PMM-14560: Improve theme switching synchronization
- Publish ThemeChangedEvent after CSS loads for better sync - Subscribe to ThemeChangedEvent in SharedPreferences dropdown - Add CSS load error handling to prevent stuck UI state - Fix TypeScript types to use GrafanaTheme2 properly Fixes visual lag between theme dropdown and content rendering.
1 parent 8d413aa commit 0b48fd3

File tree

2 files changed

+54
-9
lines changed

2 files changed

+54
-9
lines changed

public/app/core/components/SharedPreferences/SharedPreferences.tsx

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import { css } from '@emotion/css';
22
import { PureComponent } from 'react';
33
import * as React from 'react';
4+
import type { Unsubscribable } from 'rxjs';
45

5-
import { FeatureState, getBuiltInThemes, ThemeRegistryItem } from '@grafana/data';
6+
import { FeatureState, getBuiltInThemes, ThemeRegistryItem, type GrafanaTheme2 } from '@grafana/data';
67
import { selectors } from '@grafana/e2e-selectors';
7-
import { config, reportInteraction } from '@grafana/runtime';
8+
import { config, reportInteraction, getAppEvents, ThemeChangedEvent, type BusEventWithPayload } from '@grafana/runtime';
89
import { Preferences as UserPreferencesDTO } from '@grafana/schema/src/raw/preferences/x/preferences_types.gen';
910
import {
1011
Button,
@@ -26,6 +27,7 @@ import { t, Trans } from 'app/core/internationalization';
2627
import { LANGUAGES, PSEUDO_LOCALE } from 'app/core/internationalization/constants';
2728
import { PreferencesService } from 'app/core/services/PreferencesService';
2829
import { changeTheme } from 'app/core/services/theme';
30+
2931
export interface Props {
3032
resourceUri: string;
3133
disabled?: boolean;
@@ -67,6 +69,7 @@ function getLanguageOptions(): ComboboxOption[] {
6769
export class SharedPreferences extends PureComponent<Props, State> {
6870
service: PreferencesService;
6971
themeOptions: ComboboxOption[];
72+
private themeChangedSub?: Unsubscribable;
7073

7174
constructor(props: Props) {
7275
super(props);
@@ -121,6 +124,33 @@ export class SharedPreferences extends PureComponent<Props, State> {
121124
queryHistory: prefs.queryHistory,
122125
navbar: prefs.navbar,
123126
});
127+
128+
// Subscribe to theme changes to keep the dropdown in sync with the actual theme.
129+
// This ensures the dropdown reflects theme changes from any source (system preferences,
130+
// other UI components, etc.), not just from this component's dropdown selection.
131+
const eventBus = getAppEvents();
132+
if (eventBus && typeof eventBus.subscribe === 'function') {
133+
this.themeChangedSub = eventBus.subscribe(ThemeChangedEvent, (evt: BusEventWithPayload<GrafanaTheme2>) => {
134+
try {
135+
const newTheme = evt.payload;
136+
const mode = newTheme.colors.mode;
137+
138+
if (this.state.theme !== mode) {
139+
this.setState({ theme: mode });
140+
}
141+
} catch (err) {
142+
console.warn('[SharedPreferences] Failed to sync theme from ThemeChangedEvent:', err);
143+
}
144+
});
145+
}
146+
}
147+
148+
componentWillUnmount() {
149+
try {
150+
this.themeChangedSub?.unsubscribe();
151+
} catch (err) {
152+
console.warn('[SharedPreferences] Failed to unsubscribe ThemeChangedEvent:', err);
153+
}
124154
}
125155

126156
onSubmitForm = async (event: React.FormEvent<HTMLFormElement>) => {
@@ -135,7 +165,9 @@ export class SharedPreferences extends PureComponent<Props, State> {
135165
};
136166

137167
onThemeChanged = (value: ComboboxOption<string>) => {
168+
// Update state immediately so the form has the correct value when saving
138169
this.setState({ theme: value.value });
170+
139171
reportInteraction('grafana_preferences_theme_changed', {
140172
toTheme: value.value,
141173
preferenceType: this.props.preferenceType,
@@ -144,6 +176,9 @@ export class SharedPreferences extends PureComponent<Props, State> {
144176
if (value.value) {
145177
changeTheme(value.value, true);
146178
}
179+
// Note: setState is called twice - once here for immediate form state update,
180+
// and again via ThemeChangedEvent subscription after CSS loads. This is intentional
181+
// to ensure form correctness while also maintaining sync with rendered theme.
147182
};
148183

149184
onTimeZoneChanged = (timezone?: string) => {

public/app/core/services/theme.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,38 @@ export async function changeTheme(themeId: string, runtimeOnly?: boolean) {
1212

1313
const newTheme = getThemeById(themeId);
1414

15-
appEvents.publish(new ThemeChangedEvent(newTheme));
16-
1715
// Add css file for new theme
1816
if (oldTheme.colors.mode !== newTheme.colors.mode) {
1917
const newCssLink = document.createElement('link');
2018
newCssLink.rel = 'stylesheet';
2119
newCssLink.href = config.bootData.assets[newTheme.colors.mode];
22-
newCssLink.onload = () => {
23-
// Remove old css file
20+
21+
// Publish ThemeChangedEvent AFTER CSS loads to ensure UI components (like theme dropdown)
22+
// update synchronously with the actual rendered theme. This prevents visual lag where
23+
// the dropdown updates immediately but the page content updates later.
24+
const publishThemeChange = () => {
25+
appEvents.publish(new ThemeChangedEvent(newTheme));
26+
27+
// Remove old css file only after new one is loaded to prevent flickering
2428
const bodyLinks = document.getElementsByTagName('link');
2529
for (let i = 0; i < bodyLinks.length; i++) {
2630
const link = bodyLinks[i];
2731

2832
if (link.href && link.href.includes(`build/grafana.${oldTheme.colors.mode}`)) {
29-
// Remove existing link once the new css has loaded to avoid flickering
30-
// If we add new css at the same time we remove current one the page will be rendered without css
31-
// As the new css file is loading
3233
link.remove();
3334
}
3435
}
3536
};
37+
38+
newCssLink.onload = publishThemeChange;
39+
// Ensure event is published even if CSS fails to load (network error, ad blocker, etc.)
40+
// to prevent UI from getting stuck in inconsistent state
41+
newCssLink.onerror = publishThemeChange;
42+
3643
document.head.insertBefore(newCssLink, document.head.firstChild);
44+
} else {
45+
// Same mode (e.g., light -> light with different variant), publish event immediately
46+
appEvents.publish(new ThemeChangedEvent(newTheme));
3747
}
3848

3949
if (runtimeOnly) {

0 commit comments

Comments
 (0)