Default to system emoji font (#11925)

* Disable Twemoji emoji font by default Signed-off-by: Michael Telatynski <[email protected]> * Force Twemoji font in SAS Verification Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Add tests Signed-off-by: Michael Telatynski <[email protected]> * Improve tests Signed-off-by: Michael Telatynski <[email protected]> --------- Signed-off-by: Michael Telatynski <[email protected]>
matrix-org · Nov 23, 2023 · ee485ff · ee485ff
1 parent a670530
commit ee485ff
Show file tree

Hide file tree

Showing 13 changed files with 119 additions and 114 deletions.
diff --git 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;
+    /* Use the Twemoji font for consistency with other clients */
+    font-family: Twemoji, var(--cpd-font-family-sans);
 }
 
 .mx_VerificationShowSas_emojiSas_label {

diff --git 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
@@ -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
@@ -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<IProps, I
         super(props);
 
         this.state = {
+            useBundledEmojiFont: SettingsStore.getValue("useBundledEmojiFont"),
             useSystemFont: SettingsStore.getValue("useSystemFont"),
             systemFont: SettingsStore.getValue("systemFont"),
             showAdvanced: false,
@@ -111,6 +113,12 @@ export default class AppearanceUserSettingsTab extends React.Component<IProps, I
                 <>
                     <SettingsFlag name="useCompactLayout" level={SettingLevel.DEVICE} useCheckbox={true} />
 
+                    <SettingsFlag
+                        name="useBundledEmojiFont"
+                        level={SettingLevel.DEVICE}
+                        useCheckbox={true}
+                        onChange={(checked) => this.setState({ useBundledEmojiFont: checked })}
+                    />
                     <SettingsFlag
                         name="useSystemFont"
                         level={SettingLevel.DEVICE}

diff --git a/src/dispatcher/payloads/UpdateSystemFontPayload.ts b/src/dispatcher/payloads/UpdateSystemFontPayload.ts
@@ -20,6 +20,11 @@ import { Action } from "../actions";
 export interface UpdateSystemFontPayload extends ActionPayload {
     action: Action.UpdateSystemFont;
 
+    /**
+     * Specify whether to use the bundled emoji font or the system font
+     */
+    useBundledEmojiFont: boolean;
+
     /**
      * Specify whether to use a system font or the stylesheet font
      */

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -2400,6 +2400,7 @@
         "all_rooms_home_description": "All rooms you're in will appear in Home.",
         "always_show_message_timestamps": "Always show message timestamps",
         "appearance": {
+            "bundled_emoji_font": "Use bundled emoji font",
             "custom_font": "Use a system font",
             "custom_font_description": "Set the name of a font installed on your system & %(brand)s will attempt to use it.",
             "custom_font_name": "System font name",

diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx
@@ -28,7 +28,6 @@ import PushToMatrixClientController from "./controllers/PushToMatrixClientContro
 import ReloadOnChangeController from "./controllers/ReloadOnChangeController";
 import FontSizeController from "./controllers/FontSizeController";
 import SystemFontController from "./controllers/SystemFontController";
-import UseSystemFontController from "./controllers/UseSystemFontController";
 import { SettingLevel } from "./SettingLevel";
 import SettingController from "./controllers/SettingController";
 import { IS_MAC } from "../Keyboard";
@@ -712,11 +711,17 @@ export const SETTINGS: { [setting: string]: ISetting } = {
         default: true,
         displayName: _td("settings|appearance|match_system_theme"),
     },
+    "useBundledEmojiFont": {
+        supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
+        default: false,
+        displayName: _td("settings|appearance|bundled_emoji_font"),
+        controller: new SystemFontController(),
+    },
     "useSystemFont": {
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
         default: false,
         displayName: _td("settings|appearance|custom_font"),
-        controller: new UseSystemFontController(),
+        controller: new SystemFontController(),
     },
     "systemFont": {
         supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,

diff --git a/src/settings/controllers/SystemFontController.ts b/src/settings/controllers/SystemFontController.ts
@@ -19,19 +19,19 @@ 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 SystemFontController extends SettingController {
     public constructor() {
         super();
     }
 
-    public onChange(level: SettingLevel, roomId: string, newValue: any): void {
+    public onChange(): void {
         // Dispatch font size change so that everything open responds to the change.
         dis.dispatch<UpdateSystemFontPayload>({
             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
diff --git 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<UpdateSystemFontPayload, "useSystemFont" | "font">): void => {
+    }: Pick<UpdateSystemFontPayload, "useBundledEmojiFont" | "useSystemFont" | "font">): 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);
+            }
         }
     };
 }
diff --git 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