improve client and storage implemenation

Signed-off-by: Josh Romero <[email protected]>
joshuarrrr · Jan 5, 2024 · 0f57b88 · 0f57b88
1 parent 3290e4a
commit 0f57b88
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 39 deletions.
diff --git a/src/core/public/ui_settings/ui_settings_client.ts b/src/core/public/ui_settings/ui_settings_client.ts
@@ -56,7 +56,12 @@ export class UiSettingsClient implements IUiSettingsClient {
  constructor(params: UiSettingsClientParams) {
  this.api = params.api;
  this.defaults = cloneDeep(params.defaults);
- this.cache = defaultsDeep({}, this.defaults, cloneDeep(params.initialSettings));
+ this.cache = defaultsDeep(
+ {},
+ this.defaults,
+ cloneDeep(params.initialSettings),
+ this.getBrowserStoredSettings()
+ );
 
  params.done$.subscribe({
  complete: () => {
@@ -89,17 +94,10 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
  }
 
  const type = this.cache[key].type;
- const userValue = this.cache[key].preferBrowserSetting
- ? window.localStorage.getItem(`uiSettings:${key}`)
- : this.cache[key].userValue;
+ const userValue = this.cache[key].userValue;
  const defaultValue = defaultOverride !== undefined ? defaultOverride : this.cache[key].value;
  const value = userValue == null ? defaultValue : userValue;
 
- // TODO: is there a more elegant way to handle?
- if (typeof value === 'string' && type === 'boolean') {
- return JSON.parse(value);
- }
-
  if (type === 'json') {
  return JSON.parse(value);
  }
@@ -180,6 +178,28 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
  return this.updateErrors$.asObservable();
  }
 
+ private getBrowserStoredSettings() {
+ const uiSettingsJSON = window.localStorage.getItem('uiSettings') || '{}';
+ try {
+ return JSON.parse(uiSettingsJSON);
+ } catch (error) {
+ this.updateErrors$.next(error);
+ }
+ return {};
+ }
+
+ private setBrowserStoredSettings(key: string, newVal: any) {
+ const oldSettings = this.getBrowserStoredSettings();
+ const newSettings = cloneDeep(oldSettings);
+ if (newVal === null) {
+ delete newSettings[key];
+ } else {
+ newSettings[key] = { userValue: newVal };
+ }
+ window.localStorage.setItem(`uiSettings`, JSON.stringify(newSettings));
+ return { settings: newSettings };
+ }
+
  private assertUpdateAllowed(key: string) {
  if (this.isOverridden(key)) {
  throw new Error(
@@ -194,15 +214,6 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
  const declared = this.isDeclared(key);
  const defaults = this.defaults;
 
- if (this.cache[key].preferBrowserSetting) {
- try {
- window.localStorage.setItem(`uiSettings:${key}`, newVal);
- return true;
- } catch (error) {
- return false;
- }
- }
-
  const oldVal = declared ? this.cache[key].userValue : undefined;
 
  const unchanged = oldVal === newVal;
@@ -214,8 +225,10 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
  this.setLocally(key, newVal);
 
  try {
- const { settings } = (await this.api.batchSet(key, newVal)) || {};
- this.cache = defaultsDeep({}, defaults, settings);
+ const { settings } = this.cache[key]?.preferBrowserSetting
+ ? this.setBrowserStoredSettings(key, newVal)
+ : (await this.api.batchSet(key, newVal)) || {};
+ this.cache = defaultsDeep({}, defaults, this.getBrowserStoredSettings(), settings);
  this.saved$.next({ key, newValue: newVal, oldValue: initialVal });
  return true;
  } catch (error) {

diff --git a/src/legacy/ui/ui_render/bootstrap/template.js.hbs b/src/legacy/ui/ui_render/bootstrap/template.js.hbs
@@ -11,15 +11,8 @@ if (window.__osdStrictCsp__ && window.__osdCspNotEnforced__) {
  window.console.log("^ A single error about an inline script not firing due to content security policy is expected!");
  }
 
- // TODO: Should this be retrieved from window.__osdThemeTag__ instead?
- var browserDarkMode = window.localStorage.getItem('uiSettings:theme:darkMode');
-
- var rawDarkMode = browserDarkMode !== 'true' && browserDarkMode !== 'false' ? {{configDarkMode}} : browserDarkMode === 'true';
-
- // TODO: Should this be retrieved from window.__osdThemeTag__ instead?
- var rawThemeVersion = window.localStorage.getItem('uiSettings:theme:version') || '{{configThemeVersion}}'
-
- var themeVersion = rawThemeVersion === 'v7' ? 'v7' : 'v8';
+ var isDarkMode = window.__osdThemeTag__.endsWith('dark');
+ var themeVersion = window.__osdThemeTag__.startsWith('v7') ? 'v7' : 'v8';
 
  var loadingMessage = document.getElementById('osd_loading_message');
  loadingMessage.style.display = 'flex';
@@ -28,7 +21,7 @@ if (window.__osdStrictCsp__ && window.__osdCspNotEnforced__) {
  '{{regularBundlePath}}/osd-ui-shared-deps/{{UiSharedDeps.baseCssDistFilename}}'
  ];
 
- if (rawDarkMode) {
+ if (isDarkMode) {
  if (themeVersion === 'v7') {
  styleSheetPaths.push('{{regularBundlePath}}/osd-ui-shared-deps/{{UiSharedDeps.darkCssDistFilename}}');
  styleSheetPaths.push('{{basePath}}/node_modules/@osd/ui-framework/dist/kui_dark.css')

diff --git a/src/legacy/ui/ui_render/startup/template.js.hbs b/src/legacy/ui/ui_render/startup/template.js.hbs
@@ -1,9 +1,20 @@
-// TODO: fix boolean/string coerce issues
-var browserDarkMode = window.localStorage.getItem('uiSettings:theme:darkMode');
+var uiSettingsJSON = window.localStorage.getItem('uiSettings') || '{}';
 
-var rawDarkMode = browserDarkMode !== 'true' && browserDarkMode !== 'false' ? {{configDarkMode}} : browserDarkMode === 'true';
+var uiSettings = {};
+try {
+ uiSettings = JSON.parse(uiSettingsJSON) || {};
+} catch (error) {
+ console.log(error);
+}
 
-var rawThemeVersion = window.localStorage.getItem('uiSettings:theme:version') || '{{configThemeVersion}}'
+var browserDarkMode = uiSettings['theme:darkMode'] || {};
+var browserThemeVersion = uiSettings['theme:version'] || {};
+
+var rawDarkMode = typeof browserDarkMode.userValue !== 'boolean' ? {{configDarkMode}} : browserDarkMode.userValue
+
+debugger;
+
+var rawThemeVersion = browserThemeVersion.userValue || '{{configThemeVersion}}'
 
 // TODO: should this reflect config defaults instead?
 var darkMode = rawDarkMode ? 'dark' : 'light';

diff --git a/src/plugins/advanced_settings/public/header_user_menu.tsx b/src/plugins/advanced_settings/public/header_user_menu.tsx
@@ -91,6 +91,7 @@ export const HeaderUserMenu = () => {
  const {
  services: {
  http: { basePath },
+ uiSettings,
  },
  } = useOpenSearchDashboards<CoreStart>();
  const themeOptions = [
@@ -124,6 +125,10 @@ export const HeaderUserMenu = () => {
  const [screenMode, setScreenMode] = useState(
  darkMode ? screenModeOptions[1].value : screenModeOptions[0].value
  );
+ const allSettings = uiSettings.getAll();
+ const defaultTheme = allSettings['theme:version'].value;
+ const defaultScreenMode = allSettings['theme:darkMode'].value;
+ console.log(allSettings['theme:darkMode']);
 
  const onAvatarClick = () => {
  setPopover(!isPopoverOpen);
@@ -313,10 +318,18 @@ export const HeaderUserMenu = () => {
  </EuiLink>
  </EuiCallOut>
  <EuiSpacer />
- <EuiFormRow label="Theme version" helpText="Default: Next (preview)">
+ <EuiFormRow label="Theme version" helpText={`Default: ${defaultTheme}`}>
  <EuiSelect options={themeOptions} value={theme} onChange={onThemeChange} />
  </EuiFormRow>
- <EuiFormRow label="Screen mode" helpText="Default: Dark mode">
+ <EuiFormRow
+ label="Screen mode"
+ helpText={`Default: ${
+ screenModeOptions.find((t) => {
+ const defaultValue = defaultScreenMode ? 'dark' : 'light';
+ return defaultValue === t.value;
+ })?.text
+ }`}
+ >
  <EuiSelect
  options={screenModeOptions}
  value={screenMode}

diff --git a/src/plugins/advanced_settings/public/header_user_theme_menu.tsx b/src/plugins/advanced_settings/public/header_user_theme_menu.tsx
@@ -27,6 +27,7 @@ export const HeaderUserThemeMenu = () => {
  const {
  services: {
  http: { basePath },
+ uiSettings,
  },
  } = useOpenSearchDashboards<CoreStart>();
  // TODO: move to central location?
@@ -62,6 +63,10 @@ export const HeaderUserThemeMenu = () => {
  const [screenMode, setScreenMode] = useState(
  darkMode ? screenModeOptions[1].value : screenModeOptions[0].value
  );
+ const allSettings = uiSettings.getAll();
+ const defaultTheme = allSettings['theme:version'].value;
+ const defaultScreenMode = allSettings['theme:darkMode'].value;
+ console.log(allSettings['theme:darkMode']);
 
  const onButtonClick = () => {
  setPopover(!isPopoverOpen);
@@ -112,7 +117,6 @@ export const HeaderUserThemeMenu = () => {
  );
 
  // TODO: make i18n, check all translation ids
- // TODO: get configured defaults
  // TODO: fix focus behavior
  const appearanceContent = (
  <div style={{ maxWidth: 300 }}>
@@ -130,10 +134,18 @@ export const HeaderUserThemeMenu = () => {
  </EuiLink>
  </EuiCallOut>
  <EuiSpacer />
- <EuiFormRow label="Theme version" helpText="Default: Next (preview)">
+ <EuiFormRow label="Theme version" helpText={`Default: ${defaultTheme}`}>
  <EuiSelect options={themeOptions} value={theme} onChange={onThemeChange} />
  </EuiFormRow>
- <EuiFormRow label="Screen mode" helpText="Default: Dark mode">
+ <EuiFormRow
+ label="Screen mode"
+ helpText={`Default: ${
+ screenModeOptions.find((t) => {
+ const defaultValue = defaultScreenMode ? 'dark' : 'light';
+ return defaultValue === t.value;
+ })?.text
+ }`}
+ >
  <EuiSelect options={screenModeOptions} value={screenMode} onChange={onScreenModeChange} />
  </EuiFormRow>
  <EuiFlexGroup>