[BUG] Allow user Theme Selection retain theme selection

changelogs/fragments/7787.yml

@@ -0,0 +1,2 @@
+fix:
+- [BUG] Allow user Theme Selection retain theme selection ([#7787](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7787))

src/core/public/ui_settings/types.ts

@@ -54,6 +54,24 @@ export interface IUiSettingsClient {
    */
   get: <T = any>(key: string, defaultOverride?: T) => T;
 
+  /**
+   * Gets the value for a specific uiSetting, considering browser-stored settings and advanced settings.
+   * This method returns an object containing the resolved value and individual setting values.
+   *
+   * @param key - The key of the uiSetting to retrieve
+   * @param defaultOverride - An optional default value to use if the setting is not declared
+   * @returns An object containing the resolved value and additional setting information
+   * @throws Error if the setting is not declared and no defaultOverride is provided
+   */
+  getWithBrowserSettings<T = any>(
+    key: string,
+    defaultOverride?: T
+  ): {
+    advancedSettingValue: T | undefined;
+    browserValue: T | undefined;
+    defaultValue: T;
+  };
+
   /**
    * Gets an observable of the current value for a config key, and all updates to that config
    * key in the future. Providing a `defaultOverride` argument behaves the same as it does in #get()

src/core/public/ui_settings/ui_settings_client.test.ts

@@ -34,20 +34,45 @@ import { materialize, take, toArray } from 'rxjs/operators';
 import { UiSettingsClient } from './ui_settings_client';
 
 let done$: Subject<unknown>;
+let mockStorage: { [key: string]: string };
 
-function setup(options: { defaults?: any; initialSettings?: any } = {}) {
+function setup(options: { defaults?: any; initialSettings?: any; localStorage?: any } = {}) {
   const {
     defaults = {
       dateFormat: { value: 'Browser' },
       aLongNumeral: { value: `${BigInt(Number.MAX_SAFE_INTEGER) + 11n}`, type: 'number' },
+      'theme:enableUserControl': { value: false },
     },
     initialSettings = {},
+    localStorage = {},
   } = options;
 
   const batchSet = jest.fn(() => ({
     settings: {},
   }));
   done$ = new Subject();
+
+  // Mock localStorage
+  mockStorage = { ...localStorage };
+  const localStorageMock = {
+    getItem: jest.fn((key) => mockStorage[key]),
+    setItem: jest.fn((key, value) => {
+      mockStorage[key] = value.toString();
+    }),
+    removeItem: jest.fn((key) => {
+      delete mockStorage[key];
+    }),
+    clear: jest.fn(() => {
+      Object.keys(mockStorage).forEach((key) => delete mockStorage[key]);
+    }),
+  };
+  Object.defineProperty(window, 'localStorage', { value: localStorageMock });
+
+  // Initialize localStorage with provided values
+  if (localStorage.uiSettings) {
+    window.localStorage.setItem('uiSettings', localStorage.uiSettings);
+  }
+
   const client = new UiSettingsClient({
     defaults,
     initialSettings,
@@ -57,11 +82,88 @@ function setup(options: { defaults?: any; initialSettings?: any } = {}) {
     done$,
   });
 
-  return { client, batchSet };
+  return { client, batchSet, localStorage: localStorageMock };
 }
 
+beforeEach(() => {
+  mockStorage = {};
+});
+
 afterEach(() => {
   done$.complete();
+  window.localStorage.clear();
+  jest.resetAllMocks();
+});
+
+describe('#getWithBrowserSettings', () => {
+  beforeEach(() => {
+    window.localStorage.clear();
+  });
+
+  afterEach(() => {
+    window.localStorage.clear();
+    done$.complete();
+    jest.resetAllMocks();
+  });
+
+  it('returns correct values when user control is enabled', () => {
+    const { client } = setup({
+      defaults: {
+        'theme:enableUserControl': { value: true },
+        testSetting: { value: 'defaultValue' },
+      },
+      initialSettings: {
+        testSetting: { userValue: 'advancedValue' },
+      },
+      localStorage: {
+        uiSettings: JSON.stringify({
+          testSetting: { userValue: 'browserValue' },
+        }),
+      },
+    });
+
+    const result = client.getWithBrowserSettings('testSetting');
+    expect(result).toEqual({
+      advancedSettingValue: 'advancedValue',
+      browserValue: 'browserValue',
+      defaultValue: 'defaultValue',
+    });
+  });
+
+  it('returns correct values when user control is disabled', () => {
+    const { client } = setup({
+      defaults: {
+        'theme:enableUserControl': { value: false },
+        testSetting: { value: 'defaultValue' },
+      },
+      initialSettings: {
+        testSetting: { userValue: 'advancedValue' },
+      },
+    });
+
+    const result = client.getWithBrowserSettings('testSetting');
+    expect(result).toEqual({
+      advancedSettingValue: 'advancedValue',
+      browserValue: undefined,
+      defaultValue: 'defaultValue',
+    });
+  });
+
+  it('returns correct values for a setting with only a default value', () => {
+    const { client } = setup({
+      defaults: {
+        'theme:enableUserControl': { value: true },
+        testSetting: { value: 'defaultValue' },
+      },
+    });
+
+    const result = client.getWithBrowserSettings('testSetting');
+    expect(result).toEqual({
+      advancedSettingValue: undefined,
+      browserValue: undefined,
+      defaultValue: 'defaultValue',
+    });
+  });
 });
 
 describe('#getDefault', () => {

src/core/public/ui_settings/ui_settings_client.ts

@@ -52,17 +52,18 @@
   private readonly api: UiSettingsApi;
   private readonly defaults: Record<string, PublicUiSettingsParams>;
   private cache: Record<string, PublicUiSettingsParams & UserProvidedValues>;
+  private browserStoredSettings: Record<string, any> = {};
 
   constructor(params: UiSettingsClientParams) {
     this.api = params.api;
     this.defaults = cloneDeep(params.defaults);
     this.cache = defaultsDeep({}, this.defaults, cloneDeep(params.initialSettings));
 
-    if (
+    const userControl =
       this.cache['theme:enableUserControl']?.userValue ??
-      this.cache['theme:enableUserControl']?.value
-    ) {
-      this.cache = defaultsDeep(this.cache, this.getBrowserStoredSettings());
+      this.cache['theme:enableUserControl']?.value;
+    if (userControl) {
+      this.browserStoredSettings = this.getBrowserStoredSettings();
     }
 
     params.done$.subscribe({
@@ -114,6 +115,43 @@
     return this.resolveValue(value, type);
   }
 
+  getWithBrowserSettings<T = any>(
+    key: string,
+    defaultOverride?: T
+  ): {
+    advancedSettingValue: T | undefined;
+    browserValue: T | undefined;
+    defaultValue: T;
+  } {
+    const declared = this.isDeclared(key);
+
+    if (!declared && defaultOverride === undefined) {
+      throw new Error(
+        `Unexpected \`IUiSettingsClient.getWithBrowserSettings("${key}")\` call on unrecognized configuration setting "${key}".`
+      );
+    }
+
+    const type = this.cache[key]?.type;
+    const advancedSettingValue = this.cache[key]?.userValue;
+    const browserValue = this.browserStoredSettings[key]?.userValue;
+    const defaultValue = defaultOverride !== undefined ? defaultOverride : this.cache[key]?.value;
+
+    // Resolve the value based on the type
+    const resolveValue = (val: any): T => this.resolveValue(val, type);
+
+    const resolvedAdvancedValue =
+      advancedSettingValue !== undefined ? resolveValue(advancedSettingValue) : undefined;
+    const resolvedBrowserValue =
+      browserValue !== undefined ? resolveValue(browserValue) : undefined;
+    const resolvedDefaultValue = resolveValue(defaultValue);
+
+    return {
+      advancedSettingValue: resolvedAdvancedValue,
+      browserValue: resolvedBrowserValue,
+      defaultValue: resolvedDefaultValue,
+    };
+  }
+
   get$<T = any>(key: string, defaultOverride?: T) {
     return concat(
       defer(() => of(this.get(key, defaultOverride))),
@@ -230,8 +268,15 @@
 
     const declared = this.isDeclared(key);
     const defaults = this.defaults;
-
-    const oldVal = declared ? this.cache[key].userValue : undefined;
+    const userControl =
+      this.cache['theme:enableUserControl']?.userValue ??
+      this.cache['theme:enableUserControl']?.value;
+    const oldVal = userControl
+      ? this.getBrowserStoredSettings()[key]?.userValue ??
+        (declared ? this.cache[key].userValue : undefined)
+      : declared
+      ? this.cache[key].userValue
+      : undefined;
 
     const unchanged = oldVal === newVal;
     if (unchanged) {
@@ -242,16 +287,15 @@
     this.setLocally(key, newVal);
 
     try {
-      if (
-        this.cache['theme:enableUserControl']?.userValue ??
-        this.cache['theme:enableUserControl']?.value
-      ) {
-        const { settings } = this.cache[key]?.preferBrowserSetting
-          ? this.setBrowserStoredSettings(key, newVal)
-          : (await this.api.batchSet(key, newVal)) || {};
-        this.cache = defaultsDeep({}, defaults, this.getBrowserStoredSettings(), settings);
+      if (userControl) {
+        if (this.cache[key]?.preferBrowserSetting) {
+          this.setBrowserStoredSettings(key, newVal);
+        } else {
+          const { settings = {} } = (await this.api.batchSet(key, newVal)) || {};
+          this.cache = defaultsDeep({}, this.cache, settings);
+        }
       } else {
-        const { settings } = (await this.api.batchSet(key, newVal)) || {};
+        const { settings = {} } = (await this.api.batchSet(key, newVal)) || {};
         this.cache = defaultsDeep({}, defaults, settings);
       }
       this.saved$.next({ key, newValue: newVal, oldValue: initialVal });

src/core/public/ui_settings/ui_settings_service.mock.ts

@@ -49,6 +49,7 @@ const createSetupContractMock = () => {
     getUpdate$: jest.fn(),
     getSaved$: jest.fn(),
     getUpdateErrors$: jest.fn(),
+    getWithBrowserSettings: jest.fn(),
   };
   setupContract.get$.mockReturnValue(new Rx.Subject<any>());
   setupContract.getUpdate$.mockReturnValue(new Rx.Subject<any>());

src/core/server/ui_settings/settings/theme.ts

@@ -47,7 +47,7 @@ export const getThemeSettings = (): Record<string, UiSettingsParams> => {
       name: i18n.translate('core.ui_settings.params.enableUserControlTitle', {
         defaultMessage: 'Enable user control',
       }),
-      value: true,
+      value: false,
       description: i18n.translate('core.ui_settings.params.enableUserControlText', {
         defaultMessage: `Enable users to control theming and dark or light mode via "Appearance" control in top navigation. When true, those settings can no longer be set globally by administrators.`,
       }),