diff --git a/apps/web/src/dispatcher/actions.ts b/apps/web/src/dispatcher/actions.ts index cafee196ff..a1ab565cb3 100644 --- a/apps/web/src/dispatcher/actions.ts +++ b/apps/web/src/dispatcher/actions.ts @@ -108,11 +108,6 @@ export enum Action { */ ToggleSpacePanel = "toggle_space_panel", - /** - * Sets the apps root font size. Should be used with UpdateFontSizePayload - */ - MigrateBaseFontSize = "migrate_base_font_size", - /** * Sets the apps root font size delta. Should be used with UpdateFontSizeDeltaPayload * It will add the delta to the current font size. diff --git a/apps/web/src/settings/Settings.tsx b/apps/web/src/settings/Settings.tsx index d463957392..9b6314d34e 100644 --- a/apps/web/src/settings/Settings.tsx +++ b/apps/web/src/settings/Settings.tsx @@ -238,8 +238,6 @@ export interface Settings { "mjolnirPersonalRoom": IBaseSetting; "RoomList.backgroundImage": IBaseSetting; "sendReadReceipts": IBaseSetting; - "baseFontSize": IBaseSetting<"" | number>; - "baseFontSizeV2": IBaseSetting<"" | number>; "fontSizeDelta": IBaseSetting; "useCustomFontSize": IBaseSetting; "MessageComposerInput.suggestEmoji": IBaseSetting; @@ -623,15 +621,6 @@ export const SETTINGS: Settings = { shouldWarn: true, default: false, }, - /** - * @deprecated in favor of {@link fontSizeDelta} - */ - "baseFontSize": { - displayName: _td("settings|appearance|font_size"), - supportedLevels: LEVELS_ACCOUNT_SETTINGS, - default: "", - controller: new FontSizeController(), - }, "feature_render_reaction_images": { isFeature: true, labsGroup: LabGroup.Messaging, @@ -658,21 +647,8 @@ export const SETTINGS: Settings = { isFeature: true, default: false, }, - /** - * With the transition to Compound we are moving to a base font size - * of 16px. We're taking the opportunity to move away from the `baseFontSize` - * setting that had a 5px offset. - * @deprecated in favor {@link fontSizeDelta} - */ - "baseFontSizeV2": { - displayName: _td("settings|appearance|font_size"), - supportedLevels: [SettingLevel.DEVICE], - default: "", - controller: new FontSizeController(), - }, /** * This delta is added to the browser default font size - * Moving from `baseFontSizeV2` to `fontSizeDelta` to replace the default 16px to --cpd-font-size-root (browser default font size) + fontSizeDelta */ "fontSizeDelta": { displayName: _td("settings|appearance|font_size"), diff --git a/apps/web/src/settings/controllers/FontSizeController.test.ts b/apps/web/src/settings/controllers/FontSizeController.test.ts index 32240f19e3..7d36ab1d90 100644 --- a/apps/web/src/settings/controllers/FontSizeController.test.ts +++ b/apps/web/src/settings/controllers/FontSizeController.test.ts @@ -13,7 +13,7 @@ import dis from "../../dispatcher/dispatcher"; import FontSizeController from "./FontSizeController"; import { SettingLevel } from "../SettingLevel"; -const dispatchSpy = vi.spyOn(dis, "fire"); +const dispatchSpy = vi.spyOn(dis, "dispatch"); describe("FontSizeController", () => { it("dispatches a font size action on change", () => { @@ -21,6 +21,6 @@ describe("FontSizeController", () => { controller.onChange(SettingLevel.ACCOUNT, "$room:server", 12); - expect(dispatchSpy).toHaveBeenCalledWith(Action.MigrateBaseFontSize); + expect(dispatchSpy).toHaveBeenCalledWith({ action: Action.UpdateFontSizeDelta, delta: 12 }); }); }); diff --git a/apps/web/src/settings/controllers/FontSizeController.ts b/apps/web/src/settings/controllers/FontSizeController.ts index 140369c2b8..fa70012fcb 100644 --- a/apps/web/src/settings/controllers/FontSizeController.ts +++ b/apps/web/src/settings/controllers/FontSizeController.ts @@ -10,7 +10,7 @@ import SettingController from "./SettingController"; import dis from "../../dispatcher/dispatcher"; import { type UpdateFontSizeDeltaPayload } from "../../dispatcher/payloads/UpdateFontSizeDeltaPayload"; import { Action } from "../../dispatcher/actions"; -import { SettingLevel } from "../SettingLevel"; +import { type SettingLevel } from "../SettingLevel"; export default class FontSizeController extends SettingController { public constructor() { @@ -18,13 +18,7 @@ export default class FontSizeController extends SettingController { } public onChange(level: SettingLevel, roomId: string, newValue: any): void { - // In a distant past, `baseFontSize` was set on the account and config - // level. This can be accessed only after the initial sync. If we end up - // discovering that a logged in user has this kind of setting, we want to - // trigger another migration of the base font size. - if (level === SettingLevel.ACCOUNT || level === SettingLevel.CONFIG) { - dis.fire(Action.MigrateBaseFontSize); - } else if (newValue !== "") { + if (newValue !== "") { // Dispatch font size change so that everything open responds to the change. dis.dispatch({ action: Action.UpdateFontSizeDelta, diff --git a/apps/web/src/settings/watchers/FontWatcher.ts b/apps/web/src/settings/watchers/FontWatcher.ts index 9a36d8f73e..ee6813a7a4 100644 --- a/apps/web/src/settings/watchers/FontWatcher.ts +++ b/apps/web/src/settings/watchers/FontWatcher.ts @@ -11,7 +11,6 @@ import SettingsStore from "../SettingsStore"; import type IWatcher from "./Watcher"; import { toPx } from "../../utils/units"; import { Action } from "../../dispatcher/actions"; -import { SettingLevel } from "../SettingLevel"; import { type UpdateSystemFontPayload } from "../../dispatcher/payloads/UpdateSystemFontPayload"; import { type ActionPayload } from "../../dispatcher/payloads"; @@ -33,97 +32,6 @@ export class FontWatcher implements IWatcher { public async start(): Promise { this.updateFont(); this.dispatcherRef = dis.register(this.onAction); - /** - * baseFontSize is an account level setting which is loaded after the initial - * sync. Hence why we can't do that in the `constructor` - */ - await this.migrateBaseFontSize(); - } - - /** - * Migrate the base font size from the V1 and V2 version to the V3 version - * @private - */ - private async migrateBaseFontSize(): Promise { - await this.migrateBaseFontV1toFontSizeDelta(); - await this.migrateBaseFontV2toFontSizeDelta(); - } - - /** - * Migrating from the V1 version of the base font size to the new delta system. - * The delta system is using the default browser font size as a base - * Everything will become slightly larger, and getting rid of the `SIZE_DIFF` - * weirdness for locally persisted values - * @private - */ - private async migrateBaseFontV1toFontSizeDelta(): Promise { - const legacyBaseFontSize = SettingsStore.getValue("baseFontSize"); - // No baseFontV1 found, nothing to migrate - if (!legacyBaseFontSize) return; - - console.log( - "Migrating base font size -> base font size V2 -> font size delta for Compound, current value", - legacyBaseFontSize, - ); - - // Compute the V1 to V2 version before migrating to fontSizeDelta - const baseFontSizeV2 = this.computeBaseFontSizeV1toV2(legacyBaseFontSize); - - // Compute the difference between the V2 and the fontSizeDelta - const delta = this.computeFontSizeDeltaFromV2BaseFontSize(baseFontSizeV2); - - await SettingsStore.setValue("fontSizeDelta", null, SettingLevel.DEVICE, delta); - await SettingsStore.setValue("baseFontSize", null, SettingLevel.DEVICE, 0); - console.log("Migration complete, deleting legacy `baseFontSize`"); - } - - /** - * Migrating from the V2 version of the base font size to the new delta system - * @private - */ - private async migrateBaseFontV2toFontSizeDelta(): Promise { - const legacyBaseFontV2Size = SettingsStore.getValue("baseFontSizeV2"); - // No baseFontV2 found, nothing to migrate - if (!legacyBaseFontV2Size) return; - - console.log("Migrating base font size V2 for Compound, current value", legacyBaseFontV2Size); - - // Compute the difference between the V2 and the fontSizeDelta - const delta = this.computeFontSizeDeltaFromV2BaseFontSize(legacyBaseFontV2Size); - - await SettingsStore.setValue("fontSizeDelta", null, SettingLevel.DEVICE, delta); - await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 0); - console.log("Migration complete, deleting legacy `baseFontSizeV2`"); - } - - /** - * Compute the V2 font size from the V1 font size - * @param legacyBaseFontSize - * @private - */ - private computeBaseFontSizeV1toV2(legacyBaseFontSize: number): number { - // For some odd reason, the persisted value in user storage has an offset - // of 5 pixels for all values stored under `baseFontSize` - const LEGACY_SIZE_DIFF = 5; - - // Compound uses a base font size of `16px`, whereas the old Element - // styles based their calculations off a `15px` root font size. - const ROOT_FONT_SIZE_INCREASE = 1; - - // Compute the font size of the V2 version before migrating to V3 - return legacyBaseFontSize + ROOT_FONT_SIZE_INCREASE + LEGACY_SIZE_DIFF; - } - - /** - * Compute the difference between the V2 font size and the default browser font size - * @param legacyBaseFontV2Size - * @private - */ - private computeFontSizeDeltaFromV2BaseFontSize(legacyBaseFontV2Size: number): number { - const browserDefaultFontSize = FontWatcher.getRootFontSize(); - - // Compute the difference between the V2 font size and the default browser font size - return legacyBaseFontV2Size - browserDefaultFontSize; } /** @@ -157,9 +65,7 @@ export class FontWatcher implements IWatcher { } private onAction = (payload: ActionPayload): void => { - if (payload.action === Action.MigrateBaseFontSize) { - this.migrateBaseFontSize(); - } else if (payload.action === Action.UpdateFontSizeDelta) { + if (payload.action === Action.UpdateFontSizeDelta) { this.setRootFontSize(payload.delta); } else if (payload.action === Action.UpdateSystemFont) { this.setSystemFont(payload as UpdateSystemFontPayload); diff --git a/apps/web/test/unit-tests/settings/watchers/FontWatcher-test.tsx b/apps/web/test/unit-tests/settings/watchers/FontWatcher-test.tsx index 7e7aab6931..20e2b24681 100644 --- a/apps/web/test/unit-tests/settings/watchers/FontWatcher-test.tsx +++ b/apps/web/test/unit-tests/settings/watchers/FontWatcher-test.tsx @@ -112,66 +112,6 @@ describe("FontWatcher", function () { }); }); - describe("Migrates baseFontSize", () => { - let watcher: FontWatcher | undefined; - - beforeEach(() => { - document.documentElement.style.fontSize = "14px"; - watcher = new FontWatcher(); - }); - - afterEach(() => { - watcher!.stop(); - }); - - it("should not run the migration", async () => { - await watcher!.start(); - expect(SettingsStore.getValue("fontSizeDelta")).toBe(0); - }); - - it("should migrate from V1 font size to V3", async () => { - await SettingsStore.setValue("baseFontSize", null, SettingLevel.DEVICE, 13); - await watcher!.start(); - // 13px (V1 font size) + 5px (V1 offset) + 1px (root font size increase) - 16px (default browser font size) = 3px - expect(SettingsStore.getValue("fontSizeDelta")).toBe(3); - // baseFontSize should be cleared - expect(SettingsStore.getValue("baseFontSize")).toBe(0); - }); - - it("should migrate from V2 font size to V3 using browser font size", async () => { - await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 18); - await watcher!.start(); - // 18px - 16px (default browser font size) = 2px - expect(SettingsStore.getValue("fontSizeDelta")).toBe(2); - // baseFontSize should be cleared - expect(SettingsStore.getValue("baseFontSizeV2")).toBe(0); - }); - - it("should migrate from V2 font size to V3 using fallback font size", async () => { - document.documentElement.style.fontSize = ""; - await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 18); - await watcher!.start(); - // 18px - 16px (fallback) = 2px - expect(SettingsStore.getValue("fontSizeDelta")).toBe(2); - // baseFontSize should be cleared - expect(SettingsStore.getValue("baseFontSizeV2")).toBe(0); - }); - - it("should trigger migration when dispatched", async () => { - await watcher!.start(); - - await SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, 18); - defaultDispatcher.fire(Action.MigrateBaseFontSize); - - await waitFor(() => { - // 18px - 16px (default browser font size) = 2px - expect(SettingsStore.getValue("fontSizeDelta")).toBe(2); - // baseFontSizeV2 should be cleared - expect(SettingsStore.getValue("baseFontSizeV2")).toBe(0); - }); - }); - }); - it("should update root font size with positive delta", async () => { await new FontWatcher().start(); diff --git a/scripts/find-unused-settings.ts b/scripts/find-unused-settings.ts old mode 100644 new mode 100755 index 16296b7d2e..bef674ea76 --- a/scripts/find-unused-settings.ts +++ b/scripts/find-unused-settings.ts @@ -22,10 +22,15 @@ const ROOT = path.resolve(__dirname, ".."); const SETTINGS_DIR = path.join(ROOT, "apps/web/src/settings"); const SETTINGS_FILE = path.join(SETTINGS_DIR, "Settings.tsx"); -// Only the settings *definitions* directory should be excluded from the usage search - -// there are plenty of other directories literally named "settings" (e.g. -// apps/web/src/components/views/settings/) which hold real usages and must stay included. -const SETTINGS_DIR_RELATIVE = path.relative(ROOT, SETTINGS_DIR); +const EXCLUDE_GLOBS = [ + // Only the settings *definitions* directory should be excluded from the usage search - + // there are plenty of other directories literally named "settings" (e.g. + // apps/web/src/components/views/settings/) which hold real usages and must stay included. + "apps/web/src/settings/**", + "*/*/test/**", + "*-test.*", + "*.test.*", +].map((pattern) => path.relative(ROOT, pattern)); // See https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message const SETTINGS_FILE_RELATIVE = path.relative(ROOT, SETTINGS_FILE); @@ -151,12 +156,9 @@ function findControllerText(settingValue: ts.Expression, sourceFile: ts.SourceFi */ function isUsedOutsideSettings(searchTerms: string[]): boolean { const patternArgs = searchTerms.flatMap((term) => ["-e", term]); + const excludeArgs = EXCLUDE_GLOBS.map((term) => `:(exclude)${term}`); try { - execFileSync( - "git", - ["grep", "-F", "-q", ...patternArgs, "--", "apps", `:(exclude)${SETTINGS_DIR_RELATIVE}/**`], - { cwd: ROOT }, - ); + execFileSync("git", ["grep", "-F", "-q", ...patternArgs, "--", "apps", ...excludeArgs], { cwd: ROOT }); return true; } catch (e) { if (typeof (e as { status?: number }).status === "number") return false; @@ -205,7 +207,7 @@ function reportUnused(unused: DeclaredSetting[]): void { for (const { name, line } of unused) { console.error(` Settings.tsx:${line}: "${name}"`); if (process.env.GITHUB_ACTIONS === "true") { - printAnnotation(line, `Setting "${name}" is declared but never used outside ${SETTINGS_DIR_RELATIVE}/`); + printAnnotation(line, `Setting "${name}" is declared but never used outside ${EXCLUDE_GLOBS}`); } } }