Fix Saving Configuration Bug

Author: gregueiras

browser/main/Main.js

@@ -149,10 +149,11 @@ class Main extends React.Component {
     const { dispatch, config } = this.props
 
     this.refreshTheme = setInterval(() => {
-      chooseTheme(ConfigManager.get().ui)
+      const conf = ConfigManager.get()
+      chooseTheme(conf)
     }, 5 * 1000)
 
-    chooseTheme(config.ui)
+    chooseTheme(config)
     applyTheme(config.ui.theme)
 
     if (getLocales().indexOf(config.ui.language) !== -1) {

browser/main/lib/ConfigManager.js

@@ -2,7 +2,6 @@ import _ from 'lodash'
 import RcParser from 'browser/lib/RcParser'
 import i18n from 'browser/lib/i18n'
 import ee from 'browser/main/lib/eventEmitter'
-import { chooseTheme, applyTheme } from 'browser/main/lib/ThemeManager'
 
 const OSX = global.process.platform === 'darwin'
 const win = global.process.platform === 'win32'
@@ -204,9 +203,6 @@ function set(updates) {
   if (!validate(newConfig)) throw new Error('INVALID CONFIG')
   _save(newConfig)
 
-  chooseTheme(newConfig.ui)
-  applyTheme(newConfig.ui.theme)
-
   i18n.setLocale(newConfig.ui.language)
 
   let editorTheme = document.getElementById('editorTheme')

browser/main/lib/ThemeManager.js

@@ -1,4 +1,11 @@
-const chooseTheme = ui => {
+import ConfigManager from 'browser/main/lib/ConfigManager'
+
+const saveChanges = newConfig => {
+  ConfigManager.set(newConfig)
+}
+
+const chooseTheme = config => {
+  const { ui } = config
   if (!ui.enableScheduleTheme) {
     return
   }
@@ -21,11 +28,13 @@ const chooseTheme = ui => {
       ui.defaultTheme = ui.theme
       ui.theme = ui.scheduledTheme
       applyTheme(ui.theme)
+      saveChanges(config)
     }
   } else {
     if (ui.theme !== ui.defaultTheme) {
       ui.theme = ui.defaultTheme
       applyTheme(ui.theme)
+      saveChanges(config)
     }
   }
 }
@@ -40,6 +49,11 @@ const applyTheme = theme => {
   ]
   if (supportedThemes.indexOf(theme) !== -1) {
     document.body.setAttribute('data-theme', theme)
+    if (document.body.querySelector('.MarkdownPreview')) {
+      document.body
+        .querySelector('.MarkdownPreview')
+        .contentDocument.body.setAttribute('data-theme', theme)
+    }
   } else {
     document.body.setAttribute('data-theme', 'default')
   }

browser/main/modals/PreferencesModal/UiTab.js

@@ -13,6 +13,7 @@ import i18n from 'browser/lib/i18n'
 import { getLanguages } from 'browser/lib/Languages'
 import normalizeEditorFontFamily from 'browser/lib/normalizeEditorFontFamily'
 import uiThemes from 'browser/lib/ui-themes'
+import { chooseTheme, applyTheme } from 'browser/main/lib/ThemeManager'
 
 const OSX = global.process.platform === 'darwin'
 
@@ -194,6 +195,9 @@ class UiTab extends React.Component {
       preview: this.state.config.preview
     }
 
+    chooseTheme(newConfig)
+    applyTheme(newConfig.ui.theme)
+
     ConfigManager.set(newConfig)
 
     store.dispatch({

tests/lib/themeManager-test.js

@@ -0,0 +1,103 @@
+/**
+ * @fileoverview Unit test for browser/main/lib/ThemeManager.js
+ */
+const { chooseTheme, applyTheme } = require('browser/main/lib/ThemeManager')
+jest.mock('../../browser/main/lib/ConfigManager', () => {
+  return {
+    set: () => {}
+  }
+})
+
+const originalDate = Date
+let context = {}
+
+beforeAll(() => {
+  const constantDate = new Date('2017-11-27T14:33:42Z')
+  global.Date = class extends Date {
+    constructor() {
+      super()
+      return constantDate
+    }
+  }
+})
+
+beforeEach(() => {
+  context = {
+    ui: {
+      theme: 'white',
+      scheduledTheme: 'dark',
+      enableScheduleTheme: true,
+      defaultTheme: 'monokai'
+    }
+  }
+})
+
+afterAll(() => {
+  global.Date = originalDate
+})
+
+test("enableScheduleTheme is false, theme shouldn't change", () => {
+  context.ui.enableScheduleTheme = false
+
+  const beforeTheme = context.ui.theme
+  chooseTheme(context)
+  const afterTheme = context.ui.theme
+
+  expect(afterTheme).toBe(beforeTheme)
+})
+
+// NOT IN SCHEDULE
+test("scheduleEnd is bigger than scheduleStart and not in schedule, theme shouldn't change", () => {
+  const beforeTheme = context.ui.defaultTheme
+  context.ui.scheduleStart = 720 // 12:00
+  context.ui.scheduleEnd = 870 // 14:30
+  chooseTheme(context)
+  const afterTheme = context.ui.theme
+
+  expect(afterTheme).toBe(beforeTheme)
+})
+
+test("scheduleStart is bigger than scheduleEnd and not in schedule, theme shouldn't change", () => {
+  const beforeTheme = context.ui.defaultTheme
+  context.ui.scheduleStart = 960 // 16:00
+  context.ui.scheduleEnd = 600 // 10:00
+  chooseTheme(context)
+  const afterTheme = context.ui.theme
+
+  expect(afterTheme).toBe(beforeTheme)
+})
+
+// IN SCHEDULE
+test('scheduleEnd is bigger than scheduleStart and in schedule, theme should change', () => {
+  const beforeTheme = context.ui.scheduledTheme
+  context.ui.scheduleStart = 720 // 12:00
+  context.ui.scheduleEnd = 900 // 15:00
+  chooseTheme(context)
+  const afterTheme = context.ui.theme
+
+  expect(afterTheme).toBe(beforeTheme)
+})
+
+test('scheduleStart is bigger than scheduleEnd and in schedule, theme should change', () => {
+  const beforeTheme = context.ui.scheduledTheme
+  context.ui.scheduleStart = 1200 // 20:00
+  context.ui.scheduleEnd = 900 // 15:00
+  chooseTheme(context)
+  const afterTheme = context.ui.theme
+
+  expect(afterTheme).toBe(beforeTheme)
+})
+
+test("theme to apply is not a supported theme, theme shouldn't change", () => {
+  applyTheme('notATheme')
+  const afterTheme = document.body.dataset.theme
+
+  expect(afterTheme).toBe('default')
+})
+
+test('theme to apply is a supported theme, theme should change', () => {
+  applyTheme(context.ui.defaultTheme)
+  const afterTheme = document.body.dataset.theme
+
+  expect(afterTheme).toBe(context.ui.defaultTheme)
+})