Fix font not resetting when logging out (#8670)

* Fix font not resetting when logging out

* Adopt on_logged_in and on_logged_out into DispatcherAction

* Add tests

* Add copyright

Author: t3chguy

src/BasePlatform.ts

@@ -70,7 +70,7 @@ export default abstract class BasePlatform {
     protected onAction = (payload: ActionPayload) => {
         switch (payload.action) {
             case 'on_client_not_viable':
-            case 'on_logged_out':
+            case Action.OnLoggedOut:
                 this.setNotificationCount(0);
                 break;
         }

src/DeviceListener.ts

@@ -192,7 +192,7 @@ export default class DeviceListener {
     };
 
     private onAction = ({ action }: ActionPayload) => {
-        if (action !== "on_logged_in") return;
+        if (action !== Action.OnLoggedIn) return;
         this.recheck();
     };
 

src/Lifecycle.ts

@@ -632,7 +632,7 @@ async function doSetLoggedIn(
         logger.warn("No local storage available: can't persist session!");
     }
 
-    dis.dispatch({ action: 'on_logged_in' });
+    dis.fire(Action.OnLoggedIn);
 
     await startMatrixClient(/*startSyncing=*/!softLogout);
     return client;
@@ -857,15 +857,15 @@ async function startMatrixClient(startSyncing = true): Promise<void> {
  */
 export async function onLoggedOut(): Promise<void> {
     _isLoggingOut = false;
-    // Ensure that we dispatch a view change **before** stopping the client so
+    // Ensure that we dispatch a view change **before** stopping the client,
     // so that React components unmount first. This avoids React soft crashes
     // that can occur when components try to use a null client.
-    dis.dispatch({ action: 'on_logged_out' }, true);
+    dis.fire(Action.OnLoggedOut, true);
     stopMatrixClient();
     await clearStorage({ deleteEverything: true });
     LifecycleCustomisations.onLoggedOutAndStorageCleared?.();
 
-    // Do this last so we can make sure all storage has been cleared and all
+    // Do this last, so we can make sure all storage has been cleared and all
     // customisations got the memo.
     if (SdkConfig.get().logout_redirect_url) {
         logger.log("Redirecting to external provider to finish logout");

src/components/structures/MatrixChat.tsx

@@ -743,7 +743,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
             case Action.OpenDialPad:
                 Modal.createTrackedDialog('Dial pad', '', DialPadModal, {}, "mx_Dialog_dialPadWrapper");
                 break;
-            case 'on_logged_in':
+            case Action.OnLoggedIn:
                 if (
                     // Skip this handling for token login as that always calls onLoggedIn itself
                     !this.tokenLogin &&
@@ -759,7 +759,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
             case 'on_client_not_viable':
                 this.onSoftLogout();
                 break;
-            case 'on_logged_out':
+            case Action.OnLoggedOut:
                 this.onLoggedOut();
                 break;
             case 'will_start_client':

src/dispatcher/actions.ts

@@ -317,5 +317,15 @@ export enum Action {
     /**
      * Show current room topic
      */
-    ShowRoomTopic = "show_room_topic"
+    ShowRoomTopic = "show_room_topic",
+
+    /**
+     * Fired when the client was logged out. No additional payload information required.
+     */
+    OnLoggedOut = "on_logged_out",
+
+    /**
+     * Fired when the client was logged in. No additional payload information required.
+     */
+    OnLoggedIn = "on_logged_in",
 }

src/settings/Settings.tsx

@@ -42,6 +42,7 @@ import { ImageSize } from "./enums/ImageSize";
 import { MetaSpace } from "../stores/spaces";
 import SdkConfig from "../SdkConfig";
 import ThreadBetaController from './controllers/ThreadBetaController';
+import { FontWatcher } from "./watchers/FontWatcher";
 
 // These are just a bunch of helper arrays to avoid copy/pasting a bunch of times
 const LEVELS_ROOM_SETTINGS = [
@@ -420,7 +421,7 @@ export const SETTINGS: {[setting: string]: ISetting} = {
     "baseFontSize": {
         displayName: _td("Font size"),
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
-        default: 10,
+        default: FontWatcher.DEFAULT_SIZE,
         controller: new FontSizeController(),
     },
     "useCustomFontSize": {

src/settings/watchers/FontWatcher.ts

@@ -20,9 +20,12 @@ import IWatcher from "./Watcher";
 import { toPx } from '../../utils/units';
 import { Action } from '../../dispatcher/actions';
 import { SettingLevel } from "../SettingLevel";
+import { UpdateSystemFontPayload } from "../../dispatcher/payloads/UpdateSystemFontPayload";
+import { ActionPayload } from "../../dispatcher/payloads";
 
 export class FontWatcher implements IWatcher {
     public static readonly MIN_SIZE = 8;
+    public static readonly DEFAULT_SIZE = 10;
     public static readonly MAX_SIZE = 15;
     // Externally we tell the user the font is size 15. Internally we use 10.
     public static readonly SIZE_DIFF = 5;
@@ -34,27 +37,41 @@ export class FontWatcher implements IWatcher {
     }
 
     public start() {
-        this.setRootFontSize(SettingsStore.getValue("baseFontSize"));
-        this.setSystemFont({
-            useSystemFont: SettingsStore.getValue("useSystemFont"),
-            font: SettingsStore.getValue("systemFont"),
-        });
+        this.updateFont();
         this.dispatcherRef = dis.register(this.onAction);
     }
 
     public stop() {
         dis.unregister(this.dispatcherRef);
     }
 
-    private onAction = (payload) => {
+    private updateFont() {
+        this.setRootFontSize(SettingsStore.getValue("baseFontSize"));
+        this.setSystemFont({
+            useSystemFont: SettingsStore.getValue("useSystemFont"),
+            font: SettingsStore.getValue("systemFont"),
+        });
+    }
+
+    private onAction = (payload: ActionPayload) => {
         if (payload.action === Action.UpdateFontSize) {
             this.setRootFontSize(payload.size);
         } else if (payload.action === Action.UpdateSystemFont) {
-            this.setSystemFont(payload);
+            this.setSystemFont(payload as UpdateSystemFontPayload);
+        } else if (payload.action === Action.OnLoggedOut) {
+            // Clear font overrides when logging out
+            this.setRootFontSize(FontWatcher.DEFAULT_SIZE);
+            this.setSystemFont({
+                useSystemFont: false,
+                font: "",
+            });
+        } else if (payload.action === Action.OnLoggedIn) {
+            // Font size can be saved on the account, so grab value when logging in
+            this.updateFont();
         }
     };
 
-    private setRootFontSize = (size) => {
+    private setRootFontSize = (size: number) => {
         const fontSize = Math.max(Math.min(FontWatcher.MAX_SIZE, size), FontWatcher.MIN_SIZE);
 
         if (fontSize !== size) {
@@ -63,7 +80,7 @@ export class FontWatcher implements IWatcher {
         document.querySelector<HTMLElement>(":root").style.fontSize = toPx(fontSize);
     };
 
-    private setSystemFont = ({ useSystemFont, font }) => {
+    private setSystemFont = ({ useSystemFont, font }: Pick<UpdateSystemFontPayload, "useSystemFont" | "font">) => {
         if (useSystemFont) {
             // Make sure that fonts with spaces in their names get interpreted properly
             document.body.style.fontFamily = font

src/stores/LifecycleStore.ts

@@ -71,7 +71,7 @@ class LifecycleStore extends Store<ActionPayload> {
                 break;
             }
             case 'on_client_not_viable':
-            case 'on_logged_out':
+            case Action.OnLoggedOut:
                 this.reset();
                 break;
         }

src/stores/ReadyWatchingStore.ts

@@ -22,6 +22,7 @@ import { EventEmitter } from "events";
 import { MatrixClientPeg } from "../MatrixClientPeg";
 import { ActionPayload } from "../dispatcher/payloads";
 import { IDestroyable } from "../utils/IDestroyable";
+import { Action } from "../dispatcher/actions";
 
 export abstract class ReadyWatchingStore extends EventEmitter implements IDestroyable {
     protected matrixClient: MatrixClient;
@@ -83,7 +84,7 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro
                 this.matrixClient = payload.matrixClient;
                 await this.onReady();
             }
-        } else if (payload.action === 'on_client_not_viable' || payload.action === 'on_logged_out') {
+        } else if (payload.action === 'on_client_not_viable' || payload.action === Action.OnLoggedOut) {
             if (this.matrixClient) {
                 await this.onNotReady();
                 this.matrixClient = null;

src/stores/RoomViewStore.tsx

@@ -241,7 +241,7 @@ export class RoomViewStore extends Store<ActionPayload> {
                 break;
             }
             case 'on_client_not_viable':
-            case 'on_logged_out':
+            case Action.OnLoggedOut:
                 this.reset();
                 break;
             case 'reply_to_event':

test/settings/watchers/FontWatcher-test.tsx

@@ -1,5 +1,6 @@
 /*
 Copyright 2022 r00ster91 <[email protected]>
+Copyright 2022 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -21,34 +22,62 @@ import { SettingLevel } from '../../../src/settings/SettingLevel';
 import { FontWatcher } from "../../../src/settings/watchers/FontWatcher";
 import { Action } from "../../../src/dispatcher/actions";
 import { untilDispatch } from "../../test-utils";
+import defaultDispatcher from "../../../src/dispatcher/dispatcher";
 
 async function setSystemFont(font: string): Promise<void> {
+    await SettingsStore.setValue("useSystemFont", null, SettingLevel.DEVICE, !!font);
     await SettingsStore.setValue("systemFont", null, SettingLevel.DEVICE, font);
     await untilDispatch(Action.UpdateSystemFont);
     await sleep(1); // await the FontWatcher doing its action
 }
 
 describe('FontWatcher', function() {
-    let fontWatcher: FontWatcher;
-    beforeEach(() => {
-        fontWatcher = new FontWatcher();
-        fontWatcher.start();
-        return SettingsStore.setValue("useSystemFont", null, SettingLevel.DEVICE, true);
-    });
-    afterEach(() => {
-        fontWatcher.stop();
+    it("should load font on start()", async () => {
+        const watcher = new FontWatcher();
+        await setSystemFont("Font Name");
+        expect(document.body.style.fontFamily).toBe("");
+        watcher.start();
+        expect(document.body.style.fontFamily).toBe('"Font Name"');
     });
 
-    it('encloses the fonts by double quotes and sets them as the system font', async () => {
-        await setSystemFont("Fira Sans Thin, Commodore 64");
-        expect(document.body.style.fontFamily).toBe(`"Fira Sans Thin","Commodore 64"`);
+    it("should load font on Action.OnLoggedIn", async () => {
+        await setSystemFont("Font Name");
+        new FontWatcher().start();
+        document.body.style.fontFamily = ""; // clear the fontFamily which was set by start which we tested already
+        defaultDispatcher.fire(Action.OnLoggedIn, true);
+        expect(document.body.style.fontFamily).toBe('"Font Name"');
     });
-    it('does not add double quotes if already present and sets the font as the system font', async () => {
-        await setSystemFont(`"Commodore 64"`);
-        expect(document.body.style.fontFamily).toBe(`"Commodore 64"`);
+
+    it("should reset font on Action.OnLoggedOut", async () => {
+        await setSystemFont("Font Name");
+        const watcher = new FontWatcher();
+        watcher.start();
+        expect(document.body.style.fontFamily).toBe('"Font Name"');
+        defaultDispatcher.fire(Action.OnLoggedOut, true);
+        expect(document.body.style.fontFamily).toBe("");
     });
-    it('trims whitespace, encloses the fonts by double quotes, and sets them as the system font', async () => {
-        await setSystemFont(`  Fira Code  ,  "Commodore 64" `);
-        expect(document.body.style.fontFamily).toBe(`"Fira Code","Commodore 64"`);
+
+    describe("Sets font as expected", () => {
+        let fontWatcher: FontWatcher;
+        beforeEach(() => {
+            fontWatcher = new FontWatcher();
+            fontWatcher.start();
+        });
+        afterEach(() => {
+            fontWatcher.stop();
+        });
+
+        it('encloses the fonts by double quotes and sets them as the system font', async () => {
+            await setSystemFont("Fira Sans Thin, Commodore 64");
+            expect(document.body.style.fontFamily).toBe(`"Fira Sans Thin","Commodore 64"`);
+        });
+        it('does not add double quotes if already present and sets the font as the system font', async () => {
+            await setSystemFont(`"Commodore 64"`);
+            expect(document.body.style.fontFamily).toBe(`"Commodore 64"`);
+        });
+        it('trims whitespace, encloses the fonts by double quotes, and sets them as the system font', async () => {
+            await setSystemFont(`  Fira Code  ,  "Commodore 64" `);
+            expect(document.body.style.fontFamily).toBe(`"Fira Code","Commodore 64"`);
+        });
     });
 });