[GH-314] Export native app user settings on change (#380)

* [GH-314] Export native app user settings on change

This switches from exporting the native app user settings on window close to exporting
on settings change. This way the settings export remains independent of native application
life-cycle events.

This is a stop-gap towards enabling settings export on the native Linux app. The latter
does not have an easy way to catch window close events.

Relates to: #314

* Disable no-shadow rule to prevent false-positive

* Verify allowed localStorage keys

* Fix import order/spacing

* Treat JSON parsing errors as failed import

* Read known keys from the correct type 🤦

* Extend logging with imported keys and always include _current_ user settings

* Fixing eslint

Co-authored-by: Hossein <hahmadia@users.noreply.github.com>
Co-authored-by: Jesús Espino <jespinog@gmail.com>

mac/Focalboard/ViewController.swift

@@ -41,11 +41,6 @@ class ViewController:
 		self.view.window?.makeFirstResponder(self.webView)
 	}
 
-    override func viewWillDisappear() {
-        super.viewWillDisappear()
-        persistUserSettings()
-    }
-
 	override var representedObject: Any? {
 		didSet {
 			// Update the view, if already loaded.
@@ -77,34 +72,6 @@ class ViewController:
 		}
 	}
 
-	private func persistUserSettings() {
-		let semaphore = DispatchSemaphore(value: 0)
-
-		webView.evaluateJavaScript("Focalboard.exportUserSettingsBlob();") { result, error in
-			defer { semaphore.signal() }
-			guard let blob = result as? String else {
-				NSLog("Failed to export user settings: \(error?.localizedDescription ?? "?")")
-				return
-			}
-			UserDefaults.standard.set(blob, forKey: "localStorage")
-			NSLog("Persisted user settings: \(Data(base64Encoded: blob).flatMap { String(data: $0, encoding: .utf8) } ?? blob)")
-		}
-
-		// During shutdown the system grants us about 5 seconds to clean up and store user data
-		let timeout = DispatchTime.now() + .seconds(3)
-		var result: DispatchTimeoutResult?
-
-		// Busy wait because evaluateJavaScript can only be called from *and* signals on the main thread
-		while (result != .success && .now() < timeout) {
-			result = semaphore.wait(timeout: .now())
-			RunLoop.current.run(mode: .default, before: Date())
-		}
-
-		if result == .timedOut {
-			NSLog("Timed out trying to persist user settings")
-		}
-	}
-
 	private func updateSessionTokenAndUserSettings() {
 		let appDelegate = NSApplication.shared.delegate as! AppDelegate
 		let sessionTokenScript = WKUserScript(
@@ -276,11 +243,29 @@ class ViewController:
 	}
 
 	func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) {
-		guard let body = message.body as? [String: String], let type = body["type"], let blob = body["settingsBlob"] else {
+		guard
+			let body = message.body as? [AnyHashable: Any],
+			let type = body["type"] as? String,
+			let blob = body["settingsBlob"] as? String
+		else {
 			NSLog("Received unexpected script message \(message.body)")
 			return
 		}
-		NSLog("Received script message \(type): \(Data(base64Encoded: blob).flatMap { String(data: $0, encoding: .utf8) } ?? blob)")
+		NSLog("Received script message \(type)")
+		switch type {
+		case "didImportUserSettings":
+			NSLog("Imported user settings keys \(body["keys"] ?? "?")")
+		case "didNotImportUserSettings":
+			break
+		case "didChangeUserSettings":
+			UserDefaults.standard.set(blob, forKey: "localStorage")
+			NSLog("Persisted user settings after change for key \(body["key"] ?? "?")")
+		default:
+			NSLog("Received script message of unknown type \(type)")
+		}
+		if let settings = Data(base64Encoded: blob).flatMap({ try? JSONSerialization.jsonObject(with: $0, options: []) }) {
+			NSLog("Current user settings: \(settings)")
+		}
 	}
 }
 

webapp/src/app.tsx

@@ -22,15 +22,12 @@ import LoginPage from './pages/loginPage'
 import RegisterPage from './pages/registerPage'
 import {Utils} from './utils'
 import wsClient from './wsclient'
-import {importNativeAppSettings} from './nativeApp'
 import {fetchMe, getLoggedIn} from './store/users'
 import {getLanguage, fetchLanguage} from './store/language'
 import {setGlobalError, getGlobalError} from './store/globalError'
 import {useAppSelector, useAppDispatch} from './store/hooks'
 
 const App = React.memo((): JSX.Element => {
-    importNativeAppSettings()
-
     const language = useAppSelector<string>(getLanguage)
     const loggedIn = useAppSelector<boolean|null>(getLoggedIn)
     const globalError = useAppSelector<string>(getGlobalError)

webapp/src/i18n.tsx

@@ -13,6 +13,8 @@ import messages_tr from '../i18n/tr.json'
 import messages_zhHant from '../i18n/zh_Hant.json'
 import messages_zhHans from '../i18n/zh_Hans.json'
 
+import {UserSettings} from './userSettings'
+
 const supportedLanguages = ['de', 'fr', 'ja', 'nl', 'ru', 'es', 'oc', 'tr', 'zh-cn', 'zh-tw']
 
 export function getMessages(lang: string): {[key: string]: string} {
@@ -44,7 +46,7 @@ export function getMessages(lang: string): {[key: string]: string} {
 }
 
 export function getCurrentLanguage(): string {
-    let lang = localStorage.getItem('language')
+    let lang = UserSettings.language
     if (!lang) {
         if (supportedLanguages.includes(navigator.language)) {
             lang = navigator.language
@@ -58,5 +60,5 @@ export function getCurrentLanguage(): string {
 }
 
 export function storeLanguage(lang: string): void {
-    localStorage.setItem('language', lang)
+    UserSettings.language = lang
 }

webapp/src/main.tsx

@@ -3,9 +3,12 @@
 import React from 'react'
 import ReactDOM from 'react-dom'
 import {Provider as ReduxProvider} from 'react-redux'
+import {store as emojiMartStore} from 'emoji-mart'
 
 import App from './app'
 import {initThemes} from './theme'
+import {importNativeAppSettings} from './nativeApp'
+import {UserSettings} from './userSettings'
 
 import './styles/variables.scss'
 import './styles/main.scss'
@@ -13,6 +16,9 @@ import './styles/labels.scss'
 
 import store from './store'
 
+emojiMartStore.setHandlers({getter: UserSettings.getEmojiMartSetting, setter: UserSettings.setEmojiMartSetting})
+importNativeAppSettings()
+
 initThemes()
 ReactDOM.render(
     (

webapp/src/nativeApp.ts

@@ -1,7 +1,7 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import {importUserSettingsBlob} from './userSettings'
+import {exportUserSettingsBlob, importUserSettingsBlob} from './userSettings'
 
 declare interface INativeApp {
     settingsBlob: string | null;
@@ -13,20 +13,16 @@ export function importNativeAppSettings() {
     if (typeof NativeApp === 'undefined' || !NativeApp.settingsBlob) {
         return
     }
-    const success = importUserSettingsBlob(NativeApp.settingsBlob)
-    const messageType = success ? 'didImportUserSettings' : 'didNotImportUserSettings'
-    postWebKitMessage({type: messageType, settingsBlob: NativeApp.settingsBlob})
+    const importedKeys = importUserSettingsBlob(NativeApp.settingsBlob)
+    const messageType = importedKeys.length ? 'didImportUserSettings' : 'didNotImportUserSettings'
+    postWebKitMessage({type: messageType, settingsBlob: exportUserSettingsBlob(), keys: importedKeys})
     NativeApp.settingsBlob = null
 }
 
-function postWebKitMessage(message: any) {
-    const webkit = (window as any).webkit
-    if (typeof webkit === 'undefined') {
-        return
-    }
-    const handler = webkit.messageHandlers.nativeApp
-    if (typeof handler === 'undefined') {
-        return
-    }
-    handler.postMessage(message)
+export function notifySettingsChanged(key: string) {
+    postWebKitMessage({type: 'didChangeUserSettings', settingsBlob: exportUserSettingsBlob(), key})
+}
+
+function postWebKitMessage(message: any) {
+    (window as any).webkit?.messageHandlers.nativeApp?.postMessage(message)
 }

webapp/src/pages/boardPage.tsx

@@ -26,6 +26,7 @@ import {updateContents} from '../store/contents'
 import {updateComments} from '../store/comments'
 import {initialLoad, initialReadOnlyLoad} from '../store/initialLoad'
 import {useAppSelector, useAppDispatch} from '../store/hooks'
+import {UserSettings} from '../userSettings'
 
 type Props = {
     readonly?: boolean
@@ -70,8 +71,8 @@ const BoardPage = (props: Props) => {
 
         if (!boardId) {
             // Load last viewed boardView
-            const lastBoardId = localStorage.getItem('lastBoardId') || undefined
-            const lastViewId = localStorage.getItem('lastViewId') || undefined
+            const lastBoardId = UserSettings.lastBoardId || undefined
+            const lastViewId = UserSettings.lastViewId || undefined
             if (lastBoardId) {
                 let newPath = generatePath(match.path, {...match.params, boardId: lastBoardId})
                 if (lastViewId) {
@@ -90,8 +91,8 @@ const BoardPage = (props: Props) => {
             return
         }
 
-        localStorage.setItem('lastBoardId', boardId || '')
-        localStorage.setItem('lastViewId', viewId || '')
+        UserSettings.lastBoardId = boardId || ''
+        UserSettings.lastViewId = viewId || ''
         dispatch(setCurrentBoard(boardId || ''))
         dispatch(setCurrentView(viewId || ''))
     }, [match.params.boardId, match.params.viewId, history, boardViews])

webapp/src/theme.ts

@@ -9,6 +9,8 @@ import {Utils} from './utils'
 
 let activeThemeName: string
 
+import {UserSettings} from './userSettings'
+
 export type Theme = {
     mainBg: string,
     mainFg: string,
@@ -111,9 +113,9 @@ export function setTheme(theme: Theme | null): Theme {
     let consolidatedTheme = defaultTheme
     if (theme) {
         consolidatedTheme = {...defaultTheme, ...theme}
-        localStorage.setItem('theme', JSON.stringify(consolidatedTheme))
+        UserSettings.theme = JSON.stringify(consolidatedTheme)
     } else {
-        localStorage.setItem('theme', '')
+        UserSettings.theme = ''
         const darkThemeMq = window.matchMedia('(prefers-color-scheme: dark)')
         if (darkThemeMq.matches) {
             consolidatedTheme = {...defaultTheme, ...darkTheme}
@@ -205,7 +207,7 @@ function setActiveThemeName(consolidatedTheme: Theme, theme: Theme | null) {
 }
 
 export function loadTheme(): Theme {
-    const themeStr = localStorage.getItem('theme')
+    const themeStr = UserSettings.theme
     if (themeStr) {
         try {
             const theme = JSON.parse(themeStr)
@@ -223,7 +225,7 @@ export function loadTheme(): Theme {
 export function initThemes(): void {
     const darkThemeMq = window.matchMedia('(prefers-color-scheme: dark)')
     const changeHandler = () => {
-        const themeStr = localStorage.getItem('theme')
+        const themeStr = UserSettings.theme
         if (!themeStr) {
             setTheme(null)
         }

webapp/src/userSettings.ts

@@ -1,53 +1,135 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
+import {notifySettingsChanged} from './nativeApp'
+import {Utils} from './utils'
+
+// eslint-disable-next-line no-shadow
+enum UserSettingKey {
+    Language = 'language',
+    Theme = 'theme',
+    LastBoardId = 'lastBoardId',
+    LastViewId = 'lastViewId',
+    EmojiMartSkin = 'emoji-mart.skin',
+    EmojiMartLast = 'emoji-mart.last',
+    EmojiMartFrequently = 'emoji-mart.frequently',
+    RandomIcons = 'randomIcons'
+}
+
 export class UserSettings {
+    static get(key: UserSettingKey): string | null {
+        return localStorage.getItem(key)
+    }
+
+    static set(key: UserSettingKey, value: string | null) {
+        if (!Object.values(UserSettingKey).includes(key)) {
+            return
+        }
+        if (value === null) {
+            localStorage.removeItem(key)
+        } else {
+            localStorage.setItem(key, value)
+        }
+        notifySettingsChanged(key)
+    }
+
+    static get language(): string | null {
+        return UserSettings.get(UserSettingKey.Language)
+    }
+
+    static set language(newValue: string | null) {
+        UserSettings.set(UserSettingKey.Language, newValue)
+    }
+
+    static get theme(): string | null {
+        return UserSettings.get(UserSettingKey.Theme)
+    }
+
+    static set theme(newValue: string | null) {
+        UserSettings.set(UserSettingKey.Theme, newValue)
+    }
+
+    static get lastBoardId(): string | null {
+        return UserSettings.get(UserSettingKey.LastBoardId)
+    }
+
+    static set lastBoardId(newValue: string | null) {
+        UserSettings.set(UserSettingKey.LastBoardId, newValue)
+    }
+
+    static get lastViewId(): string | null {
+        return UserSettings.get(UserSettingKey.LastViewId)
+    }
+
+    static set lastViewId(newValue: string | null) {
+        UserSettings.set(UserSettingKey.LastViewId, newValue)
+    }
+
     static get prefillRandomIcons(): boolean {
-        return localStorage.getItem('randomIcons') !== 'false'
+        return UserSettings.get(UserSettingKey.RandomIcons) !== 'false'
     }
 
     static set prefillRandomIcons(newValue: boolean) {
-        localStorage.setItem('randomIcons', JSON.stringify(newValue))
+        UserSettings.set(UserSettingKey.RandomIcons, JSON.stringify(newValue))
+    }
+
+    static getEmojiMartSetting(key: string): any {
+        const prefixed = `emoji-mart.${key}`
+        Utils.assert((Object as any).values(UserSettingKey).includes(prefixed))
+        const json = UserSettings.get(prefixed as UserSettingKey)
+        return json ? JSON.parse(json) : null
+    }
+
+    static setEmojiMartSetting(key: string, value: any) {
+        const prefixed = `emoji-mart.${key}`
+        Utils.assert((Object as any).values(UserSettingKey).includes(prefixed))
+        UserSettings.set(prefixed as UserSettingKey, JSON.stringify(value))
     }
 }
 
-const keys = ['language', 'theme', 'lastBoardId', 'lastViewId', 'emoji-mart.last', 'emoji-mart.frequently', 'randomIcons']
-
 export function exportUserSettingsBlob(): string {
     return window.btoa(exportUserSettings())
 }
 
 function exportUserSettings(): string {
+    const keys = Object.values(UserSettingKey)
     const settings = Object.fromEntries(keys.map((key) => [key, localStorage.getItem(key)]))
     settings.timestamp = `${Date.now()}`
     return JSON.stringify(settings)
 }
 
-export function importUserSettingsBlob(blob: string): boolean {
+export function importUserSettingsBlob(blob: string): string[] {
     return importUserSettings(window.atob(blob))
 }
 
-function importUserSettings(json: string): boolean {
+function importUserSettings(json: string): string[] {
     const settings = parseUserSettings(json)
+    if (!settings) {
+        return []
+    }
     const timestamp = settings.timestamp
     const lastTimestamp = localStorage.getItem('timestamp')
     if (!timestamp || (lastTimestamp && Number(timestamp) <= Number(lastTimestamp))) {
-        return false
+        return []
     }
+    const importedKeys = []
     for (const [key, value] of Object.entries(settings)) {
-        if (value) {
-            localStorage.setItem(key, value as string)
-        } else {
-            localStorage.removeItem(key)
+        if (Object.values(UserSettingKey).includes(key as UserSettingKey)) {
+            if (value) {
+                localStorage.setItem(key, value as string)
+            } else {
+                localStorage.removeItem(key)
+            }
+            importedKeys.push(key)
         }
     }
-    return true
+    return importedKeys
 }
 
 function parseUserSettings(json: string): any {
     try {
         return JSON.parse(json)
     } catch (e) {
-        return {}
+        return undefined
     }
 }