From d75e2f19c57b186ba6b79495654f476a2e827e2c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 26 May 2022 09:56:53 +0100 Subject: [PATCH] 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 --- src/BasePlatform.ts | 2 +- src/DeviceListener.ts | 2 +- src/Lifecycle.ts | 8 +-- src/components/structures/MatrixChat.tsx | 4 +- src/dispatcher/actions.ts | 12 +++- src/settings/Settings.tsx | 3 +- src/settings/watchers/FontWatcher.ts | 35 +++++++++--- src/stores/LifecycleStore.ts | 2 +- src/stores/ReadyWatchingStore.ts | 3 +- src/stores/RoomViewStore.tsx | 2 +- test/settings/watchers/FontWatcher-test.tsx | 63 +++++++++++++++------ 11 files changed, 97 insertions(+), 39 deletions(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index d1f4ba397a..9de1122430 100644 --- a/src/BasePlatform.ts +++ b/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; } diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index adba51d135..c9a6d0386b 100644 --- a/src/DeviceListener.ts +++ b/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(); }; diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 5d864cc1cc..2932779d93 100644 --- a/src/Lifecycle.ts +++ b/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 { */ export async function onLoggedOut(): Promise { _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"); diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index b7e89b08a4..a41f8000a6 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -743,7 +743,7 @@ export default class MatrixChat extends React.PureComponent { 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 { case 'on_client_not_viable': this.onSoftLogout(); break; - case 'on_logged_out': + case Action.OnLoggedOut: this.onLoggedOut(); break; case 'will_start_client': diff --git a/src/dispatcher/actions.ts b/src/dispatcher/actions.ts index 27f0b423c0..95c0807573 100644 --- a/src/dispatcher/actions.ts +++ b/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", } diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index e3d5b0afce..09987c4386 100644 --- a/src/settings/Settings.tsx +++ b/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": { diff --git a/src/settings/watchers/FontWatcher.ts b/src/settings/watchers/FontWatcher.ts index 88d2a70081..cacbcb6862 100644 --- a/src/settings/watchers/FontWatcher.ts +++ b/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,11 +37,7 @@ 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); } @@ -46,15 +45,33 @@ export class FontWatcher implements IWatcher { 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(":root").style.fontSize = toPx(fontSize); }; - private setSystemFont = ({ useSystemFont, font }) => { + private setSystemFont = ({ useSystemFont, font }: Pick) => { if (useSystemFont) { // Make sure that fonts with spaces in their names get interpreted properly document.body.style.fontFamily = font diff --git a/src/stores/LifecycleStore.ts b/src/stores/LifecycleStore.ts index 618f4b4162..5d10ed3e39 100644 --- a/src/stores/LifecycleStore.ts +++ b/src/stores/LifecycleStore.ts @@ -71,7 +71,7 @@ class LifecycleStore extends Store { break; } case 'on_client_not_viable': - case 'on_logged_out': + case Action.OnLoggedOut: this.reset(); break; } diff --git a/src/stores/ReadyWatchingStore.ts b/src/stores/ReadyWatchingStore.ts index c44664a08e..a142693e62 100644 --- a/src/stores/ReadyWatchingStore.ts +++ b/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; diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index f9871c5752..d8259097a2 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -241,7 +241,7 @@ export class RoomViewStore extends Store { break; } case 'on_client_not_viable': - case 'on_logged_out': + case Action.OnLoggedOut: this.reset(); break; case 'reply_to_event': diff --git a/test/settings/watchers/FontWatcher-test.tsx b/test/settings/watchers/FontWatcher-test.tsx index 95f3085330..25aba6c2dd 100644 --- a/test/settings/watchers/FontWatcher-test.tsx +++ b/test/settings/watchers/FontWatcher-test.tsx @@ -1,5 +1,6 @@ /* Copyright 2022 r00ster91 +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 { + 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"`); + }); }); });