Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit 6d55ce0

Browse files
floriandurosflorianduros
authored
Use browser's font size instead of hardcoded 16px as root font size (#12246)
* WIP Use browser font size instead of hardcoded 16px * Add font migration to v3 * Remove custom font size input * Use a dropdown instead of a slider * Add margin to the font size dropdown * Fix `UpdateFontSizeDelta` action typo * Fix `fontScale`in `Call.ts` * Rename `baseFontSizeV3` to `fontSizeDelta` * Update playwright test * Add `default` next to the browser font size * Remove remaining `TODO` * Remove falsy `private` * Improve doc * Update snapshots after develop merge * Remove commented import
1 parent 36a8d50 commit 6d55ce0

File tree

17 files changed

+456
-369
lines changed

17 files changed

+456
-369
lines changed

‎playwright/e2e/settings/appearance-user-settings-tab.spec.ts‎

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -73,48 +73,18 @@ test.describe("Appearance user settings tab", () => {
7373
await expect(page.locator(".mx_RoomView_body[data-layout='bubble']")).toBeVisible();
7474
});
7575

76-
test("should support changing font size by clicking the font slider", async ({ page, app, user }) => {
76+
test("should support changing font size by using the font size dropdown", async ({ page, app, user }) => {
7777
await app.settings.openUserSettings("Appearance");
7878

7979
const tab = page.getByTestId("mx_AppearanceUserSettingsTab");
80-
const fontSliderSection = tab.locator(".mx_FontScalingPanel_fontSlider");
80+
const fontDropdown = tab.locator(".mx_FontScalingPanel_Dropdown");
81+
await expect(fontDropdown.getByLabel("Font size")).toBeVisible();
8182

82-
await expect(fontSliderSection.getByLabel("Font size")).toBeVisible();
83+
// Default browser font size is 16px and the select value is 0
84+
// -4 value is 12px
85+
await fontDropdown.getByLabel("Font size").selectOption({ value: "-4" });
8386

84-
const slider = fontSliderSection.getByRole("slider");
85-
// Click the left position of the slider
86-
await slider.click({ position: { x: 0, y: 10 } });
87-
88-
const MIN_FONT_SIZE = 11;
89-
// Assert that the smallest font size is selected
90-
await expect(fontSliderSection.locator(`input[value='${MIN_FONT_SIZE}']`)).toBeVisible();
91-
await expect(
92-
fontSliderSection.locator("output .mx_Slider_selection_label", { hasText: String(MIN_FONT_SIZE) }),
93-
).toBeVisible();
94-
95-
await expect(fontSliderSection).toMatchScreenshot(`font-slider-${MIN_FONT_SIZE}.png`);
96-
97-
// Click the right position of the slider
98-
await slider.click({ position: { x: 572, y: 10 } });
99-
100-
const MAX_FONT_SIZE = 21;
101-
// Assert that the largest font size is selected
102-
await expect(fontSliderSection.locator(`input[value='${MAX_FONT_SIZE}']`)).toBeVisible();
103-
await expect(
104-
fontSliderSection.locator("output .mx_Slider_selection_label", { hasText: String(MAX_FONT_SIZE) }),
105-
).toBeVisible();
106-
107-
await expect(fontSliderSection).toMatchScreenshot(`font-slider-${MAX_FONT_SIZE}.png`);
108-
});
109-
110-
test("should disable font size slider when custom font size is used", async ({ page, app, user }) => {
111-
await app.settings.openUserSettings("Appearance");
112-
113-
const panel = page.getByTestId("mx_FontScalingPanel");
114-
await panel.locator("label", { hasText: "Use custom size" }).click();
115-
116-
// Assert that the font slider is disabled
117-
await expect(panel.locator(".mx_FontScalingPanel_fontSlider input[disabled]")).toBeVisible();
87+
await expect(page).toMatchScreenshot("window-12px.png");
11888
});
11989

12090
test("should support enabling compact group (modern) layout", async ({ page, app, user }) => {

‎playwright/snapshots/settings/appearance-user-settings-tab.spec.ts/appearance-tab-linux.png‎

-772 Bytes
Loading

‎playwright/snapshots/settings/appearance-user-settings-tab.spec.ts/window-12px-linux.png‎

68.9 KB
Loading

‎res/css/views/settings/_FontScalingPanel.pcss‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,8 @@ limitations under the License.
3636
}
3737
}
3838

39-
.mx_FontScalingPanel_fontSlider {
40-
display: flex;
41-
align-items: center;
42-
padding: 15px $spacing-20 35px;
43-
background: $panels;
44-
border-radius: 10px;
45-
font-size: $font-10px;
46-
47-
.mx_FontScalingPanel_fontSlider_smallText,
48-
.mx_FontScalingPanel_fontSlider_largeText {
49-
font-weight: 500;
50-
}
51-
52-
.mx_FontScalingPanel_fontSlider_smallText {
53-
font-size: $font-15px;
54-
padding-inline-end: $spacing-20;
55-
}
56-
57-
.mx_FontScalingPanel_fontSlider_largeText {
58-
font-size: $font-18px;
59-
padding-inline-start: $spacing-20;
60-
}
39+
.mx_FontScalingPanel_Dropdown {
40+
width: 120px;
41+
/* Override default mx_Field margin */
42+
margin-bottom: var(--cpd-space-2x) !important;
6143
}

‎src/components/views/settings/FontScalingPanel.tsx‎

Lines changed: 41 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,26 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
import React, { ChangeEvent } from "react";
17+
import React from "react";
1818

1919
import EventTilePreview from "../elements/EventTilePreview";
20-
import Field from "../elements/Field";
21-
import SettingsFlag from "../elements/SettingsFlag";
2220
import SettingsStore from "../../../settings/SettingsStore";
23-
import Slider from "../elements/Slider";
24-
import { FontWatcher } from "../../../settings/watchers/FontWatcher";
25-
import { IValidationResult, IFieldState } from "../elements/Validation";
2621
import { Layout } from "../../../settings/enums/Layout";
2722
import { MatrixClientPeg } from "../../../MatrixClientPeg";
2823
import { SettingLevel } from "../../../settings/SettingLevel";
2924
import { _t } from "../../../languageHandler";
30-
import { clamp } from "../../../utils/numbers";
3125
import SettingsSubsection from "./shared/SettingsSubsection";
26+
import Field from "../elements/Field";
27+
import { FontWatcher } from "../../../settings/watchers/FontWatcher";
3228

3329
interface IProps {}
3430

3531
interface IState {
32+
browserFontSize: number;
3633
// String displaying the current selected fontSize.
37-
// Needs to be string for things like '17.' without
34+
// Needs to be string for things like '1.' without
3835
// trailing 0s.
39-
fontSize: string;
36+
fontSizeDelta: number;
4037
useCustomFontSize: boolean;
4138
layout: Layout;
4239
// User profile data for the message preview
@@ -47,14 +44,19 @@ interface IState {
4744

4845
export default class FontScalingPanel extends React.Component<IProps, IState> {
4946
private readonly MESSAGE_PREVIEW_TEXT = _t("common|preview_message");
47+
/**
48+
* Font sizes available (in px)
49+
*/
50+
private readonly sizes = [9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 20, 22, 24, 26, 28, 30, 32, 34, 36];
5051
private layoutWatcherRef?: string;
5152
private unmounted = false;
5253

5354
public constructor(props: IProps) {
5455
super(props);
5556

5657
this.state = {
57-
fontSize: SettingsStore.getValue("baseFontSizeV2", null).toString(),
58+
fontSizeDelta: SettingsStore.getValue<number>("fontSizeDelta", null),
59+
browserFontSize: FontWatcher.getBrowserDefaultFontSize(),
5860
useCustomFontSize: SettingsStore.getValue("useCustomFontSize"),
5961
layout: SettingsStore.getValue("layout"),
6062
};
@@ -90,30 +92,22 @@ export default class FontScalingPanel extends React.Component<IProps, IState> {
9092
}
9193
}
9294

93-
private onFontSizeChanged = (size: number): void => {
94-
this.setState({ fontSize: size.toString() });
95-
SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, size);
95+
/**
96+
* Save the new font size
97+
* @param delta
98+
*/
99+
private onFontSizeChanged = async (delta: string): Promise<void> => {
100+
const parsedDelta = parseInt(delta, 10) || 0;
101+
this.setState({ fontSizeDelta: parsedDelta });
102+
await SettingsStore.setValue("fontSizeDelta", null, SettingLevel.DEVICE, parsedDelta);
96103
};
97104

98-
private onValidateFontSize = async ({ value }: Pick<IFieldState, "value">): Promise<IValidationResult> => {
99-
const parsedSize = parseFloat(value!);
100-
const min = FontWatcher.MIN_SIZE;
101-
const max = FontWatcher.MAX_SIZE;
102-
103-
if (isNaN(parsedSize)) {
104-
return { valid: false, feedback: _t("settings|appearance|font_size_nan") };
105-
}
106-
107-
if (!(min <= parsedSize && parsedSize <= max)) {
108-
return {
109-
valid: false,
110-
feedback: _t("settings|appearance|font_size_limit", { min, max }),
111-
};
112-
}
113-
114-
SettingsStore.setValue("baseFontSizeV2", null, SettingLevel.DEVICE, parseInt(value!, 10));
115-
116-
return { valid: true, feedback: _t("settings|appearance|font_size_valid", { min, max }) };
105+
/**
106+
* Compute the difference between the selected font size and the browser font size
107+
* @param fontSize
108+
*/
109+
private computeDeltaFontSize = (fontSize: number): number => {
110+
return fontSize - this.state.browserFontSize;
117111
};
118112

119113
public render(): React.ReactNode {
@@ -123,6 +117,21 @@ export default class FontScalingPanel extends React.Component<IProps, IState> {
123117
stretchContent
124118
data-testid="mx_FontScalingPanel"
125119
>
120+
<Field
121+
element="select"
122+
className="mx_FontScalingPanel_Dropdown"
123+
label={_t("settings|appearance|font_size")}
124+
value={this.state.fontSizeDelta.toString()}
125+
onChange={(e) => this.onFontSizeChanged(e.target.value)}
126+
>
127+
{this.sizes.map((size) => (
128+
<option key={size} value={this.computeDeltaFontSize(size)}>
129+
{size === this.state.browserFontSize
130+
? _t("settings|appearance|font_size_default", { fontSize: size })
131+
: size}
132+
</option>
133+
))}
134+
</Field>
126135
<EventTilePreview
127136
className="mx_FontScalingPanel_preview"
128137
message={this.MESSAGE_PREVIEW_TEXT}
@@ -131,49 +140,6 @@ export default class FontScalingPanel extends React.Component<IProps, IState> {
131140
displayName={this.state.displayName}
132141
avatarUrl={this.state.avatarUrl}
133142
/>
134-
<div className="mx_FontScalingPanel_fontSlider">
135-
<div className="mx_FontScalingPanel_fontSlider_smallText">Aa</div>
136-
<Slider
137-
min={FontWatcher.MIN_SIZE}
138-
max={FontWatcher.MAX_SIZE}
139-
step={1}
140-
value={parseInt(this.state.fontSize, 10)}
141-
onChange={this.onFontSizeChanged}
142-
displayFunc={(_) => ""}
143-
disabled={this.state.useCustomFontSize}
144-
label={_t("settings|appearance|font_size")}
145-
/>
146-
<div className="mx_FontScalingPanel_fontSlider_largeText">Aa</div>
147-
</div>
148-
149-
<SettingsFlag
150-
name="useCustomFontSize"
151-
level={SettingLevel.ACCOUNT}
152-
onChange={(checked) => {
153-
this.setState({ useCustomFontSize: checked });
154-
if (!checked) {
155-
const size = parseInt(this.state.fontSize, 10);
156-
const clamped = clamp(size, FontWatcher.MIN_SIZE, FontWatcher.MAX_SIZE);
157-
if (clamped !== size) {
158-
this.onFontSizeChanged(clamped);
159-
}
160-
}
161-
}}
162-
useCheckbox={true}
163-
/>
164-
165-
<Field
166-
type="number"
167-
label={_t("settings|appearance|font_size")}
168-
autoComplete="off"
169-
placeholder={this.state.fontSize.toString()}
170-
value={this.state.fontSize.toString()}
171-
id="font_size_field"
172-
onValidate={this.onValidateFontSize}
173-
onChange={(value: ChangeEvent<HTMLInputElement>) => this.setState({ fontSize: value.target.value })}
174-
disabled={!this.state.useCustomFontSize}
175-
className="mx_AppearanceUserSettingsTab_checkboxControlledField"
176-
/>
177143
</SettingsSubsection>
178144
);
179145
}

‎src/dispatcher/actions.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,11 @@ export enum Action {
107107
MigrateBaseFontSize = "migrate_base_font_size",
108108

109109
/**
110-
* Sets the apps root font size. Should be used with UpdateFontSizePayload
110+
* Sets the apps root font size delta. Should be used with UpdateFontSizeDeltaPayload
111+
* It will add the delta to the current font size.
112+
* The delta should be between {@link FontWatcher.MIN_DELTA} and {@link FontWatcher.MAX_DELTA}.
111113
*/
112-
UpdateFontSize = "update_font_size",
114+
UpdateFontSizeDelta = "update_font_size_delta",
113115

114116
/**
115117
* Sets a system font. Should be used with UpdateSystemFontPayload

‎src/dispatcher/payloads/UpdateFontSizePayload.ts‎ renamed to ‎src/dispatcher/payloads/UpdateFontSizeDeltaPayload.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ limitations under the License.
1717
import { ActionPayload } from "../payloads";
1818
import { Action } from "../actions";
1919

20-
export interface UpdateFontSizePayload extends ActionPayload {
21-
action: Action.UpdateFontSize;
20+
export interface UpdateFontSizeDeltaPayload extends ActionPayload {
21+
action: Action.UpdateFontSizeDelta;
2222

2323
/**
24-
* The font size to set the root to
24+
* The delta is added to the current font size.
25+
* The delta should be between {@link FontWatcher.MIN_DELTA} and {@link FontWatcher.MAX_DELTA}.
2526
*/
26-
size: number;
27+
delta: number;
2728
}

‎src/i18n/strings/en_EN.json‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,9 +2422,7 @@
24222422
"custom_theme_success": "Theme added!",
24232423
"custom_theme_url": "Custom theme URL",
24242424
"font_size": "Font size",
2425-
"font_size_limit": "Custom font size can only be between %(min)s pt and %(max)s pt",
2426-
"font_size_nan": "Size must be a number",
2427-
"font_size_valid": "Use between %(min)s pt and %(max)s pt",
2425+
"font_size_default": "%(fontSize)s (default)",
24282426
"heading": "Customise your appearance",
24292427
"image_size_default": "Default",
24302428
"image_size_large": "Large",

‎src/models/Call.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ import WidgetStore from "../stores/WidgetStore";
5252
import { WidgetMessagingStore, WidgetMessagingStoreEvent } from "../stores/widgets/WidgetMessagingStore";
5353
import ActiveWidgetStore, { ActiveWidgetStoreEvent } from "../stores/ActiveWidgetStore";
5454
import { getCurrentLanguage } from "../languageHandler";
55-
import { FontWatcher } from "../settings/watchers/FontWatcher";
5655
import { PosthogAnalytics } from "../PosthogAnalytics";
5756
import { UPDATE_EVENT } from "../stores/AsyncStore";
5857
import { getJoinedNonFunctionalMembers } from "../utils/room/getJoinedNonFunctionalMembers";
5958
import { isVideoRoom } from "../utils/video-rooms";
59+
import { FontWatcher } from "../settings/watchers/FontWatcher";
6060

6161
const TIMEOUT_MS = 16000;
6262

@@ -687,7 +687,8 @@ export class ElementCall extends Call {
687687
roomId: roomId,
688688
baseUrl: client.baseUrl,
689689
lang: getCurrentLanguage().replace("_", "-"),
690-
fontScale: `${(SettingsStore.getValue("baseFontSizeV2") ?? 16) / FontWatcher.DEFAULT_SIZE}`,
690+
fontScale: (FontWatcher.getRootFontSize() / FontWatcher.getBrowserDefaultFontSize()).toString(),
691+
691692
analyticsID,
692693
});
693694

‎src/settings/Settings.tsx‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,9 @@ export const SETTINGS: { [setting: string]: ISetting } = {
511511
supportedLevels: [SettingLevel.CONFIG],
512512
default: 0,
513513
},
514+
/**
515+
* @deprecated in favor of {@link fontSizeDelta}
516+
*/
514517
"baseFontSize": {
515518
displayName: _td("settings|appearance|font_size"),
516519
supportedLevels: LEVELS_ACCOUNT_SETTINGS,
@@ -530,12 +533,22 @@ export const SETTINGS: { [setting: string]: ISetting } = {
530533
* With the transition to Compound we are moving to a base font size
531534
* of 16px. We're taking the opportunity to move away from the `baseFontSize`
532535
* setting that had a 5px offset.
533-
*
536+
* @deprecated in favor {@link fontSizeDelta}
534537
*/
535538
"baseFontSizeV2": {
536539
displayName: _td("settings|appearance|font_size"),
537540
supportedLevels: [SettingLevel.DEVICE],
538-
default: FontWatcher.DEFAULT_SIZE,
541+
default: "",
542+
controller: new FontSizeController(),
543+
},
544+
/**
545+
* This delta is added to the browser default font size
546+
* Moving from `baseFontSizeV2` to `fontSizeDelta` to replace the default 16px to --cpd-font-size-root (browser default font size) + fontSizeDelta
547+
*/
548+
"fontSizeDelta": {
549+
displayName: _td("settings|appearance|font_size"),
550+
supportedLevels: [SettingLevel.DEVICE],
551+
default: FontWatcher.DEFAULT_DELTA,
539552
controller: new FontSizeController(),
540553
},
541554
"useCustomFontSize": {

0 commit comments

Comments
 (0)