From 64877ee0ac0c861304d2dfca25d72746f1137252 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 23 Nov 2023 09:00:58 +0000 Subject: [PATCH 1/6] Disable Twemoji emoji font by default Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../legacy-light/css/_legacy-light.pcss | 15 ++++--- res/themes/light/css/_light.pcss | 13 +++--- .../tabs/user/AppearanceUserSettingsTab.tsx | 8 ++++ .../payloads/UpdateSystemFontPayload.ts | 5 +++ src/i18n/strings/en_EN.json | 1 + src/settings/Settings.tsx | 9 +++- .../controllers/SystemFontController.ts | 6 +-- .../controllers/UseSystemFontController.ts | 37 --------------- src/settings/watchers/FontWatcher.ts | 45 +++++++++++++------ 9 files changed, 72 insertions(+), 67 deletions(-) delete mode 100644 src/settings/controllers/UseSystemFontController.ts diff --git a/res/themes/legacy-light/css/_legacy-light.pcss b/res/themes/legacy-light/css/_legacy-light.pcss index f10e6743511..b1bb34c18ef 100644 --- a/res/themes/legacy-light/css/_legacy-light.pcss +++ b/res/themes/legacy-light/css/_legacy-light.pcss @@ -1,17 +1,20 @@ +:root { + /* This is set to Twemoji when the user opts into the bundled emoji font */ + --emoji-font-family: ""; +} + /* Nunito lacks combining diacritics, so these will fall through to the next font. Helevetica's diacritics sometimes do not combine nicely (on OSX, at least) and result in a huge horizontal mess. Arial empirically gets it right, hence prioritising Arial here. */ -/* We fall through to Twemoji for emoji rather than falling through - to native Emoji fonts (if any) to ensure cross-browser consistency */ /* Noto Color Emoji contains digits, in fixed-width, therefore causing digits in flowed text to stand out. TODO: Consider putting all emoji fonts to the end rather than the front. */ -$font-family: "Nunito", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Arial", "Helvetica", sans-serif, - "Noto Color Emoji"; +$font-family: "Nunito", var(--emoji-font-family), "Apple Color Emoji", "Segoe UI Emoji", "Arial", "Helvetica", + sans-serif, "Noto Color Emoji"; -$monospace-font-family: "Inconsolata", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Courier", monospace, - "Noto Color Emoji"; +$monospace-font-family: "Inconsolata", var(--emoji-font-family), "Apple Color Emoji", "Segoe UI Emoji", "Courier", + monospace, "Noto Color Emoji"; /* unified palette */ /* try to use these colors when possible */ diff --git a/res/themes/light/css/_light.pcss b/res/themes/light/css/_light.pcss index d3448a82c77..c47f1910ab3 100644 --- a/res/themes/light/css/_light.pcss +++ b/res/themes/light/css/_light.pcss @@ -1,17 +1,20 @@ +:root { + /* This is set to Twemoji when the user opts into the bundled emoji font */ + --emoji-font-family: ""; +} + /* Nunito and Inter lacks combining diacritics, so these will fall through to the next font. Helevetica's diacritics sometimes do not combine nicely (on OSX, at least) and result in a huge horizontal mess. Arial empirically gets it right, hence prioritising Arial here. */ -/* We fall through to Twemoji for emoji rather than falling through - to native Emoji fonts (if any) to ensure cross-browser consistency */ /* Noto Color Emoji contains digits, in fixed-width, therefore causing digits in flowed text to stand out. TODO: Consider putting all emoji fonts to the end rather than the front. */ -$font-family: "Inter", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Arial", "Helvetica", sans-serif, +$font-family: "Inter", var(--emoji-font-family), "Apple Color Emoji", "Segoe UI Emoji", "Arial", "Helvetica", sans-serif, "Noto Color Emoji"; -$monospace-font-family: "Inconsolata", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Courier", monospace, - "Noto Color Emoji"; +$monospace-font-family: "Inconsolata", var(--emoji-font-family), "Apple Color Emoji", "Segoe UI Emoji", "Courier", + monospace, "Noto Color Emoji"; /* Colors from Figma Compound https://www.figma.com/file/X4XTH9iS2KGJ2wFKDqkyed/Compound?node-id=559%3A120 */ /* ******************** */ diff --git a/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx b/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx index 343c0d26ec3..9408274e9bb 100644 --- a/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx @@ -38,6 +38,7 @@ import MatrixClientContext from "../../../../../contexts/MatrixClientContext"; interface IProps {} interface IState { + useBundledEmojiFont: boolean; useSystemFont: boolean; systemFont: string; showAdvanced: boolean; @@ -60,6 +61,7 @@ export default class AppearanceUserSettingsTab extends React.Component + this.setState({ useBundledEmojiFont: checked })} + /> ({ action: Action.UpdateSystemFont, + useBundledEmojiFont: SettingsStore.getValue("useBundledEmojiFont"), useSystemFont: SettingsStore.getValue("useSystemFont"), - font: newValue, + font: SettingsStore.getValue("systemFont"), }); } } diff --git a/src/settings/controllers/UseSystemFontController.ts b/src/settings/controllers/UseSystemFontController.ts deleted file mode 100644 index c3fe6e339eb..00000000000 --- a/src/settings/controllers/UseSystemFontController.ts +++ /dev/null @@ -1,37 +0,0 @@ -/* -Copyright 2020 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. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import SettingController from "./SettingController"; -import SettingsStore from "../SettingsStore"; -import dis from "../../dispatcher/dispatcher"; -import { UpdateSystemFontPayload } from "../../dispatcher/payloads/UpdateSystemFontPayload"; -import { Action } from "../../dispatcher/actions"; -import { SettingLevel } from "../SettingLevel"; - -export default class UseSystemFontController extends SettingController { - public constructor() { - super(); - } - - public onChange(level: SettingLevel, roomId: string, newValue: any): void { - // Dispatch font size change so that everything open responds to the change. - dis.dispatch({ - action: Action.UpdateSystemFont, - useSystemFont: newValue, - font: SettingsStore.getValue("systemFont"), - }); - } -} diff --git a/src/settings/watchers/FontWatcher.ts b/src/settings/watchers/FontWatcher.ts index 4af7a476c95..bb538d1809f 100644 --- a/src/settings/watchers/FontWatcher.ts +++ b/src/settings/watchers/FontWatcher.ts @@ -86,6 +86,7 @@ export class FontWatcher implements IWatcher { private updateFont(): void { this.setRootFontSize(SettingsStore.getValue("baseFontSizeV2")); this.setSystemFont({ + useBundledEmojiFont: SettingsStore.getValue("useBundledEmojiFont"), useSystemFont: SettingsStore.getValue("useSystemFont"), font: SettingsStore.getValue("systemFont"), }); @@ -102,6 +103,7 @@ export class FontWatcher implements IWatcher { // Clear font overrides when logging out this.setRootFontSize(FontWatcher.DEFAULT_SIZE); this.setSystemFont({ + useBundledEmojiFont: false, useSystemFont: false, font: "", }); @@ -121,31 +123,46 @@ export class FontWatcher implements IWatcher { }; public static readonly FONT_FAMILY_CUSTOM_PROPERTY = "--cpd-font-family-sans"; + public static readonly EMOJI_FONT_FAMILY_CUSTOM_PROPERTY = "--emoji-font-family"; + public static readonly BUNDLED_EMOJI_FONT = "Twemoji"; private setSystemFont = ({ + useBundledEmojiFont, useSystemFont, font, - }: Pick): void => { + }: Pick): void => { if (useSystemFont) { + let fontString = font + .split(",") + .map((font) => { + font = font.trim(); + if (!font.startsWith('"') && !font.endsWith('"')) { + font = `"${font}"`; + } + return font; + }) + .join(","); + + if (useBundledEmojiFont) { + fontString += ", " + FontWatcher.BUNDLED_EMOJI_FONT; + } + /** * Overrides the default font family from Compound * Make sure that fonts with spaces in their names get interpreted properly */ - document.body.style.setProperty( - FontWatcher.FONT_FAMILY_CUSTOM_PROPERTY, - font - .split(",") - .map((font) => { - font = font.trim(); - if (!font.startsWith('"') && !font.endsWith('"')) { - font = `"${font}"`; - } - return font; - }) - .join(","), - ); + document.body.style.setProperty(FontWatcher.FONT_FAMILY_CUSTOM_PROPERTY, fontString); } else { document.body.style.removeProperty(FontWatcher.FONT_FAMILY_CUSTOM_PROPERTY); + + if (useBundledEmojiFont) { + document.body.style.setProperty( + FontWatcher.EMOJI_FONT_FAMILY_CUSTOM_PROPERTY, + FontWatcher.BUNDLED_EMOJI_FONT, + ); + } else { + document.body.style.removeProperty(FontWatcher.EMOJI_FONT_FAMILY_CUSTOM_PROPERTY); + } } }; } From 9b399599d058eea8c081bffceb4954add44b1353 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 23 Nov 2023 09:09:15 +0000 Subject: [PATCH 2/6] Force Twemoji font in SAS Verification Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- res/css/views/verification/_VerificationShowSas.pcss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/res/css/views/verification/_VerificationShowSas.pcss b/res/css/views/verification/_VerificationShowSas.pcss index f601933cc51..00de29da0d2 100644 --- a/res/css/views/verification/_VerificationShowSas.pcss +++ b/res/css/views/verification/_VerificationShowSas.pcss @@ -49,6 +49,8 @@ limitations under the License. .mx_VerificationShowSas_emojiSas_emoji { font-size: $font-32px; + /* Force the Twemoji font for consistency with other clients */ + font-family: Twemoji; } .mx_VerificationShowSas_emojiSas_label { From 78ee39956986109a299d1ecca6bf777fb8fb0e79 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 23 Nov 2023 09:15:27 +0000 Subject: [PATCH 3/6] Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../verification/_VerificationShowSas.pcss | 4 +- .../controllers/SystemFontController-test.ts | 14 ++++--- .../UseSystemFontController-test.ts | 40 ------------------- 3 files changed, 11 insertions(+), 47 deletions(-) delete mode 100644 test/settings/controllers/UseSystemFontController-test.ts diff --git a/res/css/views/verification/_VerificationShowSas.pcss b/res/css/views/verification/_VerificationShowSas.pcss index 00de29da0d2..7e6443fe9f2 100644 --- a/res/css/views/verification/_VerificationShowSas.pcss +++ b/res/css/views/verification/_VerificationShowSas.pcss @@ -49,8 +49,8 @@ limitations under the License. .mx_VerificationShowSas_emojiSas_emoji { font-size: $font-32px; - /* Force the Twemoji font for consistency with other clients */ - font-family: Twemoji; + /* Use the Twemoji font for consistency with other clients */ + font-family: Twemoji, emoji; } .mx_VerificationShowSas_emojiSas_label { diff --git a/test/settings/controllers/SystemFontController-test.ts b/test/settings/controllers/SystemFontController-test.ts index c827557ca45..2450816ac4f 100644 --- a/test/settings/controllers/SystemFontController-test.ts +++ b/test/settings/controllers/SystemFontController-test.ts @@ -17,22 +17,26 @@ limitations under the License. import { Action } from "../../../src/dispatcher/actions"; import dis from "../../../src/dispatcher/dispatcher"; import SystemFontController from "../../../src/settings/controllers/SystemFontController"; -import { SettingLevel } from "../../../src/settings/SettingLevel"; import SettingsStore from "../../../src/settings/SettingsStore"; const dispatchSpy = jest.spyOn(dis, "dispatch"); describe("SystemFontController", () => { - it("dispatches a font size action on change", () => { - const getValueSpy = jest.spyOn(SettingsStore, "getValue").mockReturnValue(true); + it("dispatches a system font update action on change", () => { const controller = new SystemFontController(); - controller.onChange(SettingLevel.ACCOUNT, "$room:server", 12); + const getValueSpy = jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName) => { + if (settingName === "useBundledEmojiFont") return false; + if (settingName === "useSystemFont") return true; + if (settingName === "systemFont") return "Comic Sans MS"; + }); + controller.onChange(); expect(dispatchSpy).toHaveBeenCalledWith({ action: Action.UpdateSystemFont, + useBundledEmojiFont: false, useSystemFont: true, - font: 12, + font: "Comic Sans MS", }); expect(getValueSpy).toHaveBeenCalledWith("useSystemFont"); diff --git a/test/settings/controllers/UseSystemFontController-test.ts b/test/settings/controllers/UseSystemFontController-test.ts deleted file mode 100644 index 5b5003dce6f..00000000000 --- a/test/settings/controllers/UseSystemFontController-test.ts +++ /dev/null @@ -1,40 +0,0 @@ -/* -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. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import { Action } from "../../../src/dispatcher/actions"; -import dis from "../../../src/dispatcher/dispatcher"; -import UseSystemFontController from "../../../src/settings/controllers/UseSystemFontController"; -import { SettingLevel } from "../../../src/settings/SettingLevel"; -import SettingsStore from "../../../src/settings/SettingsStore"; - -const dispatchSpy = jest.spyOn(dis, "dispatch"); - -describe("UseSystemFontController", () => { - it("dispatches a font size action on change", () => { - const getValueSpy = jest.spyOn(SettingsStore, "getValue").mockReturnValue(12); - const controller = new UseSystemFontController(); - - controller.onChange(SettingLevel.ACCOUNT, "$room:server", true); - - expect(dispatchSpy).toHaveBeenCalledWith({ - action: Action.UpdateSystemFont, - useSystemFont: true, - font: 12, - }); - - expect(getValueSpy).toHaveBeenCalledWith("systemFont"); - }); -}); From 574c6ff42d58c9d652b4299e40c2778d0061cba6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 23 Nov 2023 09:25:26 +0000 Subject: [PATCH 4/6] Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- res/css/views/verification/_VerificationShowSas.pcss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/css/views/verification/_VerificationShowSas.pcss b/res/css/views/verification/_VerificationShowSas.pcss index 7e6443fe9f2..40588179b0e 100644 --- a/res/css/views/verification/_VerificationShowSas.pcss +++ b/res/css/views/verification/_VerificationShowSas.pcss @@ -50,7 +50,7 @@ limitations under the License. .mx_VerificationShowSas_emojiSas_emoji { font-size: $font-32px; /* Use the Twemoji font for consistency with other clients */ - font-family: Twemoji, emoji; + font-family: Twemoji, var(--cpd-font-family-sans); } .mx_VerificationShowSas_emojiSas_label { From c83f3091228f276e589c80ce6b5965685c5e44b6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 23 Nov 2023 10:51:05 +0000 Subject: [PATCH 5/6] Add tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/settings/watchers/FontWatcher-test.tsx | 27 +++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/settings/watchers/FontWatcher-test.tsx b/test/settings/watchers/FontWatcher-test.tsx index dd781994e38..c4fb593c5f7 100644 --- a/test/settings/watchers/FontWatcher-test.tsx +++ b/test/settings/watchers/FontWatcher-test.tsx @@ -34,6 +34,9 @@ async function setSystemFont(font: string): Promise { const getFontFamily = () => { return document.body.style.getPropertyValue(FontWatcher.FONT_FAMILY_CUSTOM_PROPERTY); }; +const getEmojiFontFamily = () => { + return document.body.style.getPropertyValue(FontWatcher.EMOJI_FONT_FAMILY_CUSTOM_PROPERTY); +}; describe("FontWatcher", function () { it("should load font on start()", async () => { @@ -85,6 +88,30 @@ describe("FontWatcher", function () { }); }); + describe("Sets bundled emoji font as expected", () => { + let fontWatcher: FontWatcher; + beforeEach(async () => { + fontWatcher = new FontWatcher(); + await fontWatcher.start(); + }); + afterEach(() => { + fontWatcher.stop(); + }); + + it("by default does not add Twemoji font", async () => { + expect(getEmojiFontFamily()).toMatchInlineSnapshot(`""`); + }); + it("adds Twemoji font when enabled", async () => { + await SettingsStore.setValue("useBundledEmojiFont", null, SettingLevel.DEVICE, true); + expect(getEmojiFontFamily()).toMatchInlineSnapshot(`""`); + }); + it("works in conjunction with useSystemFont", async () => { + await setSystemFont(`"Commodore 64"`); + await SettingsStore.setValue("useBundledEmojiFont", null, SettingLevel.DEVICE, true); + expect(getFontFamily()).toMatchInlineSnapshot(`""Commodore 64", Twemoji"`); + }); + }); + describe("Migrates baseFontSize", () => { let watcher: FontWatcher | undefined; From aff512f5a8e94b4a2c3ffc71c462ad79a79b6942 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 23 Nov 2023 12:38:44 +0000 Subject: [PATCH 6/6] Improve tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/settings/watchers/FontWatcher-test.tsx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/settings/watchers/FontWatcher-test.tsx b/test/settings/watchers/FontWatcher-test.tsx index c4fb593c5f7..6fb400508fe 100644 --- a/test/settings/watchers/FontWatcher-test.tsx +++ b/test/settings/watchers/FontWatcher-test.tsx @@ -24,9 +24,15 @@ import { Action } from "../../../src/dispatcher/actions"; import { untilDispatch } from "../../test-utils"; import defaultDispatcher from "../../../src/dispatcher/dispatcher"; -async function setSystemFont(font: string): Promise { +async function setSystemFont(font: string | false): Promise { + await SettingsStore.setValue("systemFont", null, SettingLevel.DEVICE, font || ""); 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 +} + +async function setUseBundledEmojiFont(use: boolean): Promise { + await SettingsStore.setValue("useBundledEmojiFont", null, SettingLevel.DEVICE, use); await untilDispatch(Action.UpdateSystemFont); await sleep(1); // await the FontWatcher doing its action } @@ -91,6 +97,7 @@ describe("FontWatcher", function () { describe("Sets bundled emoji font as expected", () => { let fontWatcher: FontWatcher; beforeEach(async () => { + await setSystemFont(false); fontWatcher = new FontWatcher(); await fontWatcher.start(); }); @@ -102,12 +109,12 @@ describe("FontWatcher", function () { expect(getEmojiFontFamily()).toMatchInlineSnapshot(`""`); }); it("adds Twemoji font when enabled", async () => { - await SettingsStore.setValue("useBundledEmojiFont", null, SettingLevel.DEVICE, true); - expect(getEmojiFontFamily()).toMatchInlineSnapshot(`""`); + await setUseBundledEmojiFont(true); + expect(getEmojiFontFamily()).toMatchInlineSnapshot(`"Twemoji"`); }); it("works in conjunction with useSystemFont", async () => { await setSystemFont(`"Commodore 64"`); - await SettingsStore.setValue("useBundledEmojiFont", null, SettingLevel.DEVICE, true); + await setUseBundledEmojiFont(true); expect(getFontFamily()).toMatchInlineSnapshot(`""Commodore 64", Twemoji"`); }); });