diff --git a/public/app/core/components/SharedPreferences/SharedPreferences.test.tsx b/public/app/core/components/SharedPreferences/SharedPreferences.test.tsx index 84346d2ed5238..e43f6109f6032 100644 --- a/public/app/core/components/SharedPreferences/SharedPreferences.test.tsx +++ b/public/app/core/components/SharedPreferences/SharedPreferences.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor, within } from '@testing-library/react'; +import { act, cleanup, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { comboboxTestSetup } from 'test/helpers/comboboxTestSetup'; import { getSelectParent, selectOptionInTest } from 'test/helpers/selectOptionInTest'; @@ -7,6 +7,14 @@ import { Preferences as UserPreferencesDTO } from '@grafana/schema/src/raw/prefe import SharedPreferences from './SharedPreferences'; +// @PERCONA +// Mock getAppEvents at the module level +let mockGetAppEvents = jest.fn(); +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getAppEvents: () => mockGetAppEvents(), +})); + const selectComboboxOptionInTest = async (input: HTMLElement, optionOrOptions: string) => { await userEvent.click(input); const option = await screen.findByRole('option', { name: optionOrOptions }); @@ -206,4 +214,98 @@ describe('SharedPreferences', () => { await userEvent.click(screen.getByText('Save')); expect(mockReload).toHaveBeenCalled(); }); + + // @PERCONA + describe('ThemeChangedEvent subscription', () => { + let mockEventBus: { + subscribe: jest.Mock; + unsubscribe: jest.Mock; + }; + + beforeEach(() => { + // Cleanup the render from the parent beforeEach + cleanup(); + + mockEventBus = { + subscribe: jest.fn(), + unsubscribe: jest.fn(), + }; + mockGetAppEvents.mockReturnValue(mockEventBus); + }); + + it('subscribes to ThemeChangedEvent on mount', async () => { + render(); + await waitFor(() => expect(mockPrefsLoad).toHaveBeenCalled()); + + expect(mockEventBus.subscribe).toHaveBeenCalledWith( + expect.anything(), // ThemeChangedEvent + expect.any(Function) + ); + }); + + it('updates theme state when ThemeChangedEvent is received with dark theme', async () => { + let eventHandler: ((evt: any) => void) | undefined; + mockEventBus.subscribe.mockImplementation((_event, handler) => { + eventHandler = handler; + return { unsubscribe: mockEventBus.unsubscribe }; + }); + + render(); + await waitFor(() => expect(mockPrefsLoad).toHaveBeenCalled()); + + // Wait for the form to be fully loaded and verify initial theme is light + const themeSelect = await screen.findByRole('combobox', { name: /Interface theme/i }); + expect(themeSelect).toHaveValue('Light'); + + // Simulate ThemeChangedEvent with dark theme using proper GrafanaTheme2 structure + act(() => { + eventHandler?.({ payload: { colors: { mode: 'dark' } } }); + }); + + await waitFor(() => { + expect(themeSelect).toHaveValue('Dark'); + }); + }); + + it('updates theme state when ThemeChangedEvent is received with light theme', async () => { + // Reset mockPrefsLoad to return dark theme for this test + mockPrefsLoad.mockResolvedValueOnce({ ...mockPreferences, theme: 'dark' }); + + let eventHandler: ((evt: any) => void) | undefined; + mockEventBus.subscribe.mockImplementation((_event, handler) => { + eventHandler = handler; + return { unsubscribe: mockEventBus.unsubscribe }; + }); + + render(); + await waitFor(() => expect(mockPrefsLoad).toHaveBeenCalled()); + + // Wait for the form to be fully loaded and verify initial theme is dark + const themeSelect = await screen.findByRole('combobox', { name: /Interface theme/i }); + await waitFor(() => { + expect(themeSelect).toHaveValue('Dark'); + }); + + // Simulate ThemeChangedEvent with light theme using proper GrafanaTheme2 structure + act(() => { + eventHandler?.({ payload: { colors: { mode: 'light' } } }); + }); + + await waitFor(() => { + expect(themeSelect).toHaveValue('Light'); + }); + }); + + it('unsubscribes from ThemeChangedEvent on unmount', async () => { + const unsubscribeMock = jest.fn(); + mockEventBus.subscribe.mockReturnValue({ unsubscribe: unsubscribeMock }); + + const { unmount } = render(); + await waitFor(() => expect(mockPrefsLoad).toHaveBeenCalled()); + + unmount(); + + expect(unsubscribeMock).toHaveBeenCalled(); + }); + }); }); diff --git a/public/app/core/components/SharedPreferences/SharedPreferences.tsx b/public/app/core/components/SharedPreferences/SharedPreferences.tsx index dcc1a250133f6..3a66c00bb96b7 100644 --- a/public/app/core/components/SharedPreferences/SharedPreferences.tsx +++ b/public/app/core/components/SharedPreferences/SharedPreferences.tsx @@ -1,10 +1,17 @@ import { css } from '@emotion/css'; import { PureComponent } from 'react'; import * as React from 'react'; +import type { Unsubscribable } from 'rxjs'; -import { FeatureState, getBuiltInThemes, ThemeRegistryItem } from '@grafana/data'; +import { + FeatureState, + getBuiltInThemes, + ThemeRegistryItem, + type GrafanaTheme2, + type BusEventWithPayload, +} from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { config, reportInteraction } from '@grafana/runtime'; +import { config, reportInteraction, getAppEvents, ThemeChangedEvent } from '@grafana/runtime'; import { Preferences as UserPreferencesDTO } from '@grafana/schema/src/raw/preferences/x/preferences_types.gen'; import { Button, @@ -26,6 +33,7 @@ import { t, Trans } from 'app/core/internationalization'; import { LANGUAGES, PSEUDO_LOCALE } from 'app/core/internationalization/constants'; import { PreferencesService } from 'app/core/services/PreferencesService'; import { changeTheme } from 'app/core/services/theme'; + export interface Props { resourceUri: string; disabled?: boolean; @@ -67,6 +75,8 @@ function getLanguageOptions(): ComboboxOption[] { export class SharedPreferences extends PureComponent { service: PreferencesService; themeOptions: ComboboxOption[]; + // @PERCONA: Subscription to ThemeChangedEvent for syncing dropdown with theme changes + private themeChangedSub?: Unsubscribable; constructor(props: Props) { super(props); @@ -121,6 +131,33 @@ export class SharedPreferences extends PureComponent { queryHistory: prefs.queryHistory, navbar: prefs.navbar, }); + + // @PERCONA: Subscribe to theme changes to keep the dropdown in sync with the actual theme. + // This ensures the dropdown reflects theme changes from any source (system preferences, + // other UI components, etc.), not just from this component's dropdown selection. + const eventBus = getAppEvents(); + if (eventBus && typeof eventBus.subscribe === 'function') { + this.themeChangedSub = eventBus.subscribe(ThemeChangedEvent, (evt: BusEventWithPayload) => { + try { + const newTheme = evt.payload; + const mode = newTheme.colors.mode; + + if (this.state.theme !== mode) { + this.setState({ theme: mode }); + } + } catch (err) { + console.warn('[SharedPreferences] Failed to sync theme from ThemeChangedEvent:', err); + } + }); + } + } + + componentWillUnmount() { + try { + this.themeChangedSub?.unsubscribe(); + } catch (err) { + console.warn('[SharedPreferences] Failed to unsubscribe ThemeChangedEvent:', err); + } } onSubmitForm = async (event: React.FormEvent) => { @@ -136,6 +173,7 @@ export class SharedPreferences extends PureComponent { onThemeChanged = (value: ComboboxOption) => { this.setState({ theme: value.value }); + reportInteraction('grafana_preferences_theme_changed', { toTheme: value.value, preferenceType: this.props.preferenceType, diff --git a/public/app/core/services/theme.ts b/public/app/core/services/theme.ts index 6368aa07f7697..b2796a42ec830 100644 --- a/public/app/core/services/theme.ts +++ b/public/app/core/services/theme.ts @@ -12,14 +12,18 @@ export async function changeTheme(themeId: string, runtimeOnly?: boolean) { const newTheme = getThemeById(themeId); - appEvents.publish(new ThemeChangedEvent(newTheme)); - // Add css file for new theme if (oldTheme.colors.mode !== newTheme.colors.mode) { const newCssLink = document.createElement('link'); newCssLink.rel = 'stylesheet'; newCssLink.href = config.bootData.assets[newTheme.colors.mode]; - newCssLink.onload = () => { + + // @PERCONA: Publish ThemeChangedEvent AFTER CSS loads to ensure UI components (like theme dropdown) + // update synchronously with the actual rendered theme. This prevents visual lag where + // the dropdown updates immediately but the page content updates later. + const publishThemeChange = () => { + appEvents.publish(new ThemeChangedEvent(newTheme)); + // Remove old css file const bodyLinks = document.getElementsByTagName('link'); for (let i = 0; i < bodyLinks.length; i++) { @@ -33,7 +37,16 @@ export async function changeTheme(themeId: string, runtimeOnly?: boolean) { } } }; + + newCssLink.onload = publishThemeChange; + // @PERCONA: Ensure event is published even if CSS fails to load (network error, ad blocker, etc.) + // to prevent UI from getting stuck in inconsistent state + newCssLink.onerror = publishThemeChange; + document.head.insertBefore(newCssLink, document.head.firstChild); + } else { + // Same mode (e.g., light -> light with different variant), publish event immediately + appEvents.publish(new ThemeChangedEvent(newTheme)); } if (runtimeOnly) {