mirror of https://github.com/vector-im/riot-web
				
				
				
			Default to system emoji font (#11925)
* Disable Twemoji emoji font by default Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Force Twemoji font in SAS Verification Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>pull/28788/head^2
							parent
							
								
									a6705304aa
								
							
						
					
					
						commit
						ee485ffcfd
					
				|  | @ -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 { | ||||
|  |  | |||
|  | @ -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 */ | ||||
|  |  | |||
|  | @ -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 */ | ||||
| /* ******************** */ | ||||
|  |  | |||
|  | @ -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} | ||||
|  |  | |||
|  | @ -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 | ||||
|      */ | ||||
|  |  | |||
|  | @ -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", | ||||
|  |  | |||
|  | @ -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, | ||||
|  |  | |||
|  | @ -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"), | ||||
|         }); | ||||
|     } | ||||
| } | ||||
|  |  | |||
|  | @ -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<UpdateSystemFontPayload>({ | ||||
|             action: Action.UpdateSystemFont, | ||||
|             useSystemFont: newValue, | ||||
|             font: SettingsStore.getValue("systemFont"), | ||||
|         }); | ||||
|     } | ||||
| } | ||||
|  | @ -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); | ||||
|             } | ||||
|         } | ||||
|     }; | ||||
| } | ||||
|  |  | |||
|  | @ -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"); | ||||
|  |  | |||
|  | @ -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"); | ||||
|     }); | ||||
| }); | ||||
|  | @ -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<void> { | ||||
| async function setSystemFont(font: string | false): Promise<void> { | ||||
|     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<void> { | ||||
|     await SettingsStore.setValue("useBundledEmojiFont", null, SettingLevel.DEVICE, use); | ||||
|     await untilDispatch(Action.UpdateSystemFont); | ||||
|     await sleep(1); // await the FontWatcher doing its action
 | ||||
| } | ||||
|  | @ -34,6 +40,9 @@ async function setSystemFont(font: string): Promise<void> { | |||
| 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 +94,31 @@ describe("FontWatcher", function () { | |||
|         }); | ||||
|     }); | ||||
| 
 | ||||
|     describe("Sets bundled emoji font as expected", () => { | ||||
|         let fontWatcher: FontWatcher; | ||||
|         beforeEach(async () => { | ||||
|             await setSystemFont(false); | ||||
|             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 setUseBundledEmojiFont(true); | ||||
|             expect(getEmojiFontFamily()).toMatchInlineSnapshot(`"Twemoji"`); | ||||
|         }); | ||||
|         it("works in conjunction with useSystemFont", async () => { | ||||
|             await setSystemFont(`"Commodore 64"`); | ||||
|             await setUseBundledEmojiFont(true); | ||||
|             expect(getFontFamily()).toMatchInlineSnapshot(`""Commodore 64", Twemoji"`); | ||||
|         }); | ||||
|     }); | ||||
| 
 | ||||
|     describe("Migrates baseFontSize", () => { | ||||
|         let watcher: FontWatcher | undefined; | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 Michael Telatynski
						Michael Telatynski