Skip to content

Commit

Permalink
[BUG] Allow user Theme Selection retain theme selection
Browse files Browse the repository at this point in the history
Issue Resolve:
#7689

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
  • Loading branch information
ananzh committed Aug 22, 2024
1 parent aed03fa commit 67cadf3
Show file tree
Hide file tree
Showing 8 changed files with 231 additions and 58 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions src/core/public/ui_settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
106 changes: 104 additions & 2 deletions src/core/public/ui_settings/ui_settings_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand Down
74 changes: 59 additions & 15 deletions src/core/public/ui_settings/ui_settings_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,18 @@ export class UiSettingsClient implements IUiSettingsClient {
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({
Expand Down Expand Up @@ -114,6 +115,43 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
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))),
Expand Down Expand Up @@ -230,8 +268,15 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r

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) {
Expand All @@ -242,16 +287,15 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
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 });
Expand Down
1 change: 1 addition & 0 deletions src/core/public/ui_settings/ui_settings_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/settings/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`,
}),
Expand Down
Loading

0 comments on commit 67cadf3

Please sign in to comment.