fix: Actually use and persist default settings

src/util/settings_persister.js

@@ -11,7 +11,9 @@ import generateId from '../lib/id_generator';
 export const isLocalStorageAvailable = () => {
   try {
     localStorage.setItem('test', 'test');
-    return localStorage.getItem('test') === 'test';
+    const localStorageRes = localStorage.getItem('test') === 'test';
+    localStorage.removeItem('test');
+    return localStorageRes && localStorage;
   } catch (e) {
     return false;
   }
@@ -139,6 +141,7 @@ export const persistableFields = [
     category: 'base',
     name: 'colorScheme',
     type: 'string',
+    default: 'Light',
     shouldStoreInConfig: true,
   },
   {
@@ -155,27 +158,29 @@ export const readOpennessState = () => {
   return !!opennessStateJSONString ? JSON.parse(opennessStateJSONString) : null;
 };
 
-const getFieldsToPersist = (state, fields) =>
-  fields
+const getFieldsToPersist = (state, fields) => {
+  return fields
     .filter((field) => !field.depreacted)
     .filter((field) => field.category === 'org')
     .map((field) => field.name)
     .map((field) => [field, state.org.present.get(field)])
     .concat(
       persistableFields
         .filter((field) => field.category !== 'org')
-        .map((field) =>
-          field.type === 'json'
+        .map((field) => {
+          return field.type === 'json'
             ? [
                 field.name,
                 JSON.stringify(state[field.category].get(field.name) || field.default || {}),
               ]
-            : [field.name, state[field.category].get(field.name)]
-        )
+            : [field.name, state[field.category].get(field.name) || field.default];
+        })
     );
+};
 
-const getConfigFileContents = (fieldsToPersist) =>
-  JSON.stringify(_.fromPairs(fieldsToPersist), null, 2);
+const getConfigFileContents = (fieldsToPersist) => {
+  return JSON.stringify(_.fromPairs(fieldsToPersist), null, 2);
+};
 
 export const applyCategorySettingsFromConfig = (state, config, category) => {
   persistableFields
@@ -258,7 +263,8 @@ export const readInitialState = () => {
     initialState.org.present = initialState.org.present.set('opennessState', fromJS(opennessState));
   }
 
-  // Cache the config file contents locally so we don't overwrite on initial page load.
+  // Cache the config file contents locally so we don't overwrite on
+  // initial page load.
   window.previousSettingsFileContents = getConfigFileContents(
     getFieldsToPersist(initialState, persistableFields)
   );
@@ -310,7 +316,9 @@ export const subscribeToChanges = (store) => {
 
       const fieldsToPersist = getFieldsToPersist(state, persistableFields);
 
-      fieldsToPersist.forEach(([name, value]) => localStorage.setItem(name, value));
+      fieldsToPersist.forEach(([name, value]) => {
+        if (name && value) localStorage.setItem(name, value);
+      });
 
       if (state.base.get('shouldStoreSettingsInSyncBackend')) {
         const settingsFileContents = getConfigFileContents(fieldsToPersist);

src/util/settings_persister.test.js

@@ -0,0 +1,24 @@
+import Store from '../store';
+import { readInitialState, subscribeToChanges } from './settings_persister';
+
+describe('Settings persister', () => {
+  let store;
+  beforeEach(() => {
+    const initialState = readInitialState();
+    store = Store(initialState);
+    subscribeToChanges(store)();
+  });
+
+  afterEach(() => {
+    localStorage.clear();
+  });
+
+  test('Do not persist nonsense like like "false" for settings without default', () => {
+    expect(localStorage.getItem('showClockDisplay')).not.toBe('false');
+    expect(localStorage.getItem('showClockDisplay')).toBe(null);
+  });
+
+  test('Does persist given default values, for example colorScheme', () => {
+    expect(localStorage.getItem('colorScheme')).toBe('Light');
+  });
+});