From 14d5b33f234197990074e02d86e60146e532aa0f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 29 Mar 2023 12:42:57 +0100 Subject: [PATCH 01/11] Clamp font size when disabling "Use custom size" --- .../views/settings/FontScalingPanel.tsx | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/components/views/settings/FontScalingPanel.tsx b/src/components/views/settings/FontScalingPanel.tsx index f8aa9186bb7..e233fe5e38a 100644 --- a/src/components/views/settings/FontScalingPanel.tsx +++ b/src/components/views/settings/FontScalingPanel.tsx @@ -27,6 +27,7 @@ import { Layout } from "../../../settings/enums/Layout"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; import { SettingLevel } from "../../../settings/SettingLevel"; import { _t } from "../../../languageHandler"; +import { clamp } from "../../../utils/numbers"; interface IProps {} @@ -103,6 +104,8 @@ export default class FontScalingPanel extends React.Component { }; public render(): React.ReactNode { + const sizes = [13, 14, 15, 16, 18]; + return (
{_t("Font size")} @@ -117,7 +120,7 @@ export default class FontScalingPanel extends React.Component {
Aa
""} @@ -129,7 +132,22 @@ export default class FontScalingPanel extends React.Component { this.setState({ useCustomFontSize: checked })} + onChange={(checked) => { + this.setState({ useCustomFontSize: checked }); + if (!checked) { + const size = parseInt(this.state.fontSize, 10); + const clamped = clamp(size, sizes[0], sizes[sizes.length - 1]); + if (clamped !== size) { + this.setState({ fontSize: clamped.toString() }); + SettingsStore.setValue( + "baseFontSize", + null, + SettingLevel.DEVICE, + clamped - FontWatcher.SIZE_DIFF, + ); + } + } + }} useCheckbox={true} /> From 5fad0e69c76296f3a3a18533b344fe4c80ea386f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 29 Mar 2023 13:03:11 +0100 Subject: [PATCH 02/11] Switch Slider to use a semantic input range element --- res/css/views/elements/_Slider.pcss | 187 ++++++++++-------- src/components/views/elements/Slider.tsx | 140 ++++--------- .../views/settings/FontScalingPanel.tsx | 10 +- 3 files changed, 144 insertions(+), 193 deletions(-) diff --git a/res/css/views/elements/_Slider.pcss b/res/css/views/elements/_Slider.pcss index d7d7dd7168e..a37d6f4b0cc 100644 --- a/res/css/views/elements/_Slider.pcss +++ b/res/css/views/elements/_Slider.pcss @@ -16,92 +16,107 @@ limitations under the License. .mx_Slider { position: relative; - margin: 0px; - flex-grow: 1; -} - -.mx_Slider_dotContainer { - display: flex; - flex-direction: row; - justify-content: space-between; -} - -.mx_Slider_bar { - display: flex; - box-sizing: border-box; - position: absolute; - height: 1em; - width: 100%; - padding: 0 0.5em; /* half the width of a dot. */ - align-items: center; -} - -.mx_Slider_bar > hr { - width: 100%; - height: 0.4em; - background-color: $slider-background-color; - border: 0; -} - -.mx_Slider_selection { - display: flex; - align-items: center; - width: calc(100% - 1em); /* 2 * half the width of a dot */ - height: 1em; - position: absolute; - pointer-events: none; -} - -.mx_Slider_selectionDot { - position: absolute; - width: $slider-selection-dot-size; - height: $slider-selection-dot-size; - background-color: $accent; - border-radius: 50%; - z-index: 10; -} - -.mx_Slider_selectionText { - color: $muted-fg-color; - font-size: $font-14px; - position: relative; - text-align: center; - top: 30px; - width: 100%; -} - -.mx_Slider_selection > hr { margin: 0; - border: 0.2em solid $accent; -} - -.mx_Slider_dot { - height: $slider-dot-size; - width: $slider-dot-size; - border-radius: 50%; - background-color: $slider-background-color; - z-index: 0; -} - -.mx_Slider_dotActive { - background-color: $accent; -} - -.mx_Slider_dotValue { - display: flex; - flex-direction: column; - align-items: center; - color: $slider-background-color; -} - -/* The following is a hack to center the labels without adding */ -/* any width to the slider's dots. */ -.mx_Slider_labelContainer { - width: 1em; -} + flex-grow: 1; -.mx_Slider_label { - position: relative; - width: fit-content; - left: -50%; + input[type=range] { + height: 2.4em; + appearance: none; + width: 100%; + background: none; + font-size: 1em; // set base multiplier for em units applied later + + --active-color: $accent; + + &:disabled { + cursor: not-allowed; + --active-color: $slider-background-color; + } + + &:focus { + outline: none; + } + + &::-webkit-slider-runnable-track { + width: 100%; + height: 0.4em; + background: $slider-background-color; + border-radius: 5px; + border: 0 solid #000000; + } + &::-webkit-slider-thumb { + border: 0 solid #000000; + width: $slider-selection-dot-size; + height: $slider-selection-dot-size; + background: var(--active-color); + border-radius: 50%; + -webkit-appearance: none; + margin-top: calc(2px + 1.2em - $slider-selection-dot-size); + } + &:focus::-webkit-slider-runnable-track { + background: $slider-background-color; + } + + &::-moz-range-track { + width: 100%; + height: 0.4em; + background: $slider-background-color; + border-radius: 5px; + border: 0 solid #000000; + } + &::-moz-range-progress { + height: 0.4em; + background: var(--active-color); + border-radius: 5px; + border: 0 solid #000000; + } + &::-moz-range-thumb { + border: 0 solid #000000; + width: $slider-selection-dot-size; + height: $slider-selection-dot-size; + background: var(--active-color); + border-radius: 50%; + } + + &::-ms-track { + width: 100%; + height: 0.4em; + background: transparent; + border-color: transparent; + color: transparent; + } + &::-ms-fill-lower, &::-ms-fill-upper { + background: $slider-background-color; + border: 0 solid #000000; + border-radius: 10px; + } + &::-ms-thumb { + margin-top: 1px; + width: $slider-selection-dot-size; + height: $slider-selection-dot-size; + background: var(--active-color); + border-radius: 50%; + } + &:focus::-ms-fill-upper { + background: $slider-background-color; + } + &::-ms-fill-lower, &:focus::-ms-fill-lower { + background: var(--active-color) + } + } + + output { + position: absolute; + left: 50%; + transform: translateX(-50%); + + font-size: 1em; // set base multiplier for em units applied later + text-align: center; + top: 3em; + + .mx_Slider_selection_label { + color: $muted-fg-color; + font-size: $font-14px; + } + } } diff --git a/src/components/views/elements/Slider.tsx b/src/components/views/elements/Slider.tsx index c8abeab6205..cb8037b796a 100644 --- a/src/components/views/elements/Slider.tsx +++ b/src/components/views/elements/Slider.tsx @@ -15,137 +15,71 @@ limitations under the License. */ import * as React from "react"; +import { ChangeEvent } from "react"; interface IProps { // A callback for the selected value - onSelectionChange: (value: number) => void; + onChange: (value: number) => void; // The current value of the slider value: number; - // The range and values of the slider - // Currently only supports an ascending, constant interval range - values: number[]; + // The min and max of the slider + min: number; + max: number; + // The step size of the slider, can be a number or "any" + step: number | "any"; - // A function for formatting the the values + // A function for formatting the values displayFunc: (value: number) => string; // Whether the slider is disabled disabled: boolean; } -export default class Slider extends React.Component { - // offset is a terrible inverse approximation. - // if the values represents some function f(x) = y where x is the - // index of the array and y = values[x] then offset(f, y) = x - // s.t f(x) = y. - // it assumes a monotonic function and interpolates linearly between - // y values. - // Offset is used for finding the location of a value on a - // non linear slider. - private offset(values: number[], value: number): number { - // the index of the first number greater than value. - const closest = values.reduce((prev, curr) => { - return value > curr ? prev + 1 : prev; - }, 0); - - // Off the left - if (closest === 0) { - return 0; - } - - // Off the right - if (closest === values.length) { - return 100; - } - - // Now - const closestLessValue = values[closest - 1]; - const closestGreaterValue = values[closest]; - - const intervalWidth = 1 / (values.length - 1); +const THUMB_SIZE = 2.4; // em - const linearInterpolation = (value - closestLessValue) / (closestGreaterValue - closestLessValue); - - return 100 * (closest - 1 + linearInterpolation) * intervalWidth; +export default class Slider extends React.Component { + private get position(): number { + const { min, max, value } = this.props; + return Number(((value - min) * 100) / (max - min)); } - public render(): React.ReactNode { - const dots = this.props.values.map((v) => ( - {} : () => this.props.onSelectionChange(v)} - key={v} - disabled={this.props.disabled} - /> - )); + private onChange = (ev: ChangeEvent): void => { + this.props.onChange(parseInt(ev.target.value, 10)); + }; + public render(): React.ReactNode { let selection: JSX.Element | undefined; if (!this.props.disabled) { - const offset = this.offset(this.props.values, this.props.value); + const position = this.position; selection = ( -
-
-
{this.props.value}
-
-
-
+ + {this.props.value} + ); } return (
-
-
-
{} : this.onClick.bind(this)} /> - {selection} -
-
{dots}
-
+ + {selection}
); } - - public onClick(event: React.MouseEvent): void { - const width = (event.target as HTMLElement).clientWidth; - // nativeEvent is safe to use because https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/offsetX - // is supported by all modern browsers - const relativeClick = event.nativeEvent.offsetX / width; - const nearestValue = this.props.values[Math.round(relativeClick * (this.props.values.length - 1))]; - this.props.onSelectionChange(nearestValue); - } -} - -interface IDotProps { - // Callback for behavior onclick - onClick: () => void; - - // Whether the dot should appear active - active: boolean; - - // The label on the dot - label: string; - - // Whether the slider is disabled - disabled: boolean; -} - -class Dot extends React.PureComponent { - public render(): React.ReactNode { - let className = "mx_Slider_dot"; - if (!this.props.disabled && this.props.active) { - className += " mx_Slider_dotActive"; - } - - return ( - -
-
-
{this.props.label}
-
- - ); - } } diff --git a/src/components/views/settings/FontScalingPanel.tsx b/src/components/views/settings/FontScalingPanel.tsx index e233fe5e38a..136a096a604 100644 --- a/src/components/views/settings/FontScalingPanel.tsx +++ b/src/components/views/settings/FontScalingPanel.tsx @@ -104,7 +104,8 @@ export default class FontScalingPanel extends React.Component { }; public render(): React.ReactNode { - const sizes = [13, 14, 15, 16, 18]; + const min = 13; + const max = 18; return (
@@ -120,9 +121,10 @@ export default class FontScalingPanel extends React.Component {
Aa
""} disabled={this.state.useCustomFontSize} /> @@ -136,7 +138,7 @@ export default class FontScalingPanel extends React.Component { this.setState({ useCustomFontSize: checked }); if (!checked) { const size = parseInt(this.state.fontSize, 10); - const clamped = clamp(size, sizes[0], sizes[sizes.length - 1]); + const clamped = clamp(size, min, max); if (clamped !== size) { this.setState({ fontSize: clamped.toString() }); SettingsStore.setValue( From c794428397bacc953bdd5cb6d05519a51189fb12 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 29 Mar 2023 13:07:22 +0100 Subject: [PATCH 03/11] Iterate --- src/components/views/settings/FontScalingPanel.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/settings/FontScalingPanel.tsx b/src/components/views/settings/FontScalingPanel.tsx index 136a096a604..832230325d2 100644 --- a/src/components/views/settings/FontScalingPanel.tsx +++ b/src/components/views/settings/FontScalingPanel.tsx @@ -123,6 +123,7 @@ export default class FontScalingPanel extends React.Component { ""} From e84038f43a34697fd7b042ae3c8c99e2d9727c3e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 29 Mar 2023 13:11:09 +0100 Subject: [PATCH 04/11] delint --- res/css/views/elements/_Slider.pcss | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/res/css/views/elements/_Slider.pcss b/res/css/views/elements/_Slider.pcss index a37d6f4b0cc..ad3a90c9cae 100644 --- a/res/css/views/elements/_Slider.pcss +++ b/res/css/views/elements/_Slider.pcss @@ -19,7 +19,7 @@ limitations under the License. margin: 0; flex-grow: 1; - input[type=range] { + input[type="range"] { height: 2.4em; appearance: none; width: 100%; @@ -85,7 +85,8 @@ limitations under the License. border-color: transparent; color: transparent; } - &::-ms-fill-lower, &::-ms-fill-upper { + &::-ms-fill-lower, + &::-ms-fill-upper { background: $slider-background-color; border: 0 solid #000000; border-radius: 10px; @@ -100,8 +101,9 @@ limitations under the License. &:focus::-ms-fill-upper { background: $slider-background-color; } - &::-ms-fill-lower, &:focus::-ms-fill-lower { - background: var(--active-color) + &::-ms-fill-lower, + &:focus::-ms-fill-lower { + background: var(--active-color); } } From 227c8e2fcc3e8e8b02b061054b51ba8470a437ab Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 29 Mar 2023 13:38:36 +0100 Subject: [PATCH 05/11] delint --- res/css/views/elements/_Slider.pcss | 1 + 1 file changed, 1 insertion(+) diff --git a/res/css/views/elements/_Slider.pcss b/res/css/views/elements/_Slider.pcss index ad3a90c9cae..fb3777556d9 100644 --- a/res/css/views/elements/_Slider.pcss +++ b/res/css/views/elements/_Slider.pcss @@ -30,6 +30,7 @@ limitations under the License. &:disabled { cursor: not-allowed; + --active-color: $slider-background-color; } From 64fa5436f90a486efd8752691aa26ad753e7b35d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 29 Mar 2023 13:39:42 +0100 Subject: [PATCH 06/11] snapshot --- .../FontScalingPanel-test.tsx.snap | 114 +++--------------- 1 file changed, 17 insertions(+), 97 deletions(-) diff --git a/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap b/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap index 8644bda9875..ce9be9f7959 100644 --- a/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap +++ b/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap @@ -36,104 +36,24 @@ exports[`FontScalingPanel renders the font scaling UI 1`] = `
-
-
-
-
-
-
- 15 -
-
-
-
-
-
+ + - -
-
-
-
- - -
-
-
-
- - -
-
-
-
- - -
-
-
-
- - -
-
-
-
- -
-
+ 15 + +
Date: Wed, 29 Mar 2023 14:22:59 +0100 Subject: [PATCH 07/11] Iterate --- .../views/settings/FontScalingPanel.tsx | 8 +------- .../views/settings/FontScalingPanel-test.tsx | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/components/views/settings/FontScalingPanel.tsx b/src/components/views/settings/FontScalingPanel.tsx index 832230325d2..49fe5576718 100644 --- a/src/components/views/settings/FontScalingPanel.tsx +++ b/src/components/views/settings/FontScalingPanel.tsx @@ -141,13 +141,7 @@ export default class FontScalingPanel extends React.Component { const size = parseInt(this.state.fontSize, 10); const clamped = clamp(size, min, max); if (clamped !== size) { - this.setState({ fontSize: clamped.toString() }); - SettingsStore.setValue( - "baseFontSize", - null, - SettingLevel.DEVICE, - clamped - FontWatcher.SIZE_DIFF, - ); + this.onFontSizeChanged(clamped); } } }} diff --git a/test/components/views/settings/FontScalingPanel-test.tsx b/test/components/views/settings/FontScalingPanel-test.tsx index d52dacee54c..0e0e051d4e2 100644 --- a/test/components/views/settings/FontScalingPanel-test.tsx +++ b/test/components/views/settings/FontScalingPanel-test.tsx @@ -15,10 +15,11 @@ limitations under the License. */ import React from "react"; -import { render } from "@testing-library/react"; +import { fireEvent, render, waitFor } from "@testing-library/react"; import * as TestUtils from "../../../test-utils"; import FontScalingPanel from "../../../../src/components/views/settings/FontScalingPanel"; +import SettingsStore from "../../../../src/settings/SettingsStore"; // Fake random strings to give a predictable snapshot jest.mock("matrix-js-sdk/src/randomstring", () => { @@ -33,4 +34,19 @@ describe("FontScalingPanel", () => { const { asFragment } = render(); expect(asFragment()).toMatchSnapshot(); }); + + it("should clamp custom font size when disabling it", async () => { + jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); + TestUtils.stubClient(); + const { container, getByText } = render(); + fireEvent.click(getByText("Use custom size")); + await waitFor(() => { + expect(container.querySelector("input[checked]")).toBeDefined(); + }); + fireEvent.change(container.querySelector("#font_size_field"), { target: { value: "20" } }); + fireEvent.click(getByText("Use custom size")); + await waitFor(() => { + expect(container.querySelector("#font_size_field")).toHaveValue(18); + }); + }); }); From 36997ec60daf770fd9449ed99860edb7e4094491 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 29 Mar 2023 14:31:51 +0100 Subject: [PATCH 08/11] Iterate --- test/components/views/settings/FontScalingPanel-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/views/settings/FontScalingPanel-test.tsx b/test/components/views/settings/FontScalingPanel-test.tsx index 0e0e051d4e2..9c1be7540f7 100644 --- a/test/components/views/settings/FontScalingPanel-test.tsx +++ b/test/components/views/settings/FontScalingPanel-test.tsx @@ -43,7 +43,7 @@ describe("FontScalingPanel", () => { await waitFor(() => { expect(container.querySelector("input[checked]")).toBeDefined(); }); - fireEvent.change(container.querySelector("#font_size_field"), { target: { value: "20" } }); + fireEvent.change(container.querySelector("#font_size_field")!, { target: { value: "20" } }); fireEvent.click(getByText("Use custom size")); await waitFor(() => { expect(container.querySelector("#font_size_field")).toHaveValue(18); From 411df0c44c1c4e5a26da4349536cdbb95e8933ec Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 30 Mar 2023 17:32:26 +0100 Subject: [PATCH 09/11] Fix step size --- src/components/views/elements/Slider.tsx | 2 +- .../views/settings/__snapshots__/FontScalingPanel-test.tsx.snap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/Slider.tsx b/src/components/views/elements/Slider.tsx index cb8037b796a..a6f11aa0751 100644 --- a/src/components/views/elements/Slider.tsx +++ b/src/components/views/elements/Slider.tsx @@ -75,7 +75,7 @@ export default class Slider extends React.Component { value={this.props.value} onChange={this.onChange} disabled={this.props.disabled} - step="any" + step={this.props.step} autoComplete="off" /> {selection} diff --git a/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap b/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap index ce9be9f7959..ac2c9e9945e 100644 --- a/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap +++ b/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap @@ -40,7 +40,7 @@ exports[`FontScalingPanel renders the font scaling UI 1`] = ` autocomplete="off" max="18" min="13" - step="any" + step="1" type="range" value="15" /> From c4e8486572053ca5ec25a807e238ee77e855935b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 4 Apr 2023 14:10:12 +0100 Subject: [PATCH 10/11] Add focus outline to slider --- res/css/views/elements/_Slider.pcss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/res/css/views/elements/_Slider.pcss b/res/css/views/elements/_Slider.pcss index fb3777556d9..b34cff346f8 100644 --- a/res/css/views/elements/_Slider.pcss +++ b/res/css/views/elements/_Slider.pcss @@ -19,6 +19,12 @@ limitations under the License. margin: 0; flex-grow: 1; + &:focus-within { + /* focus outline for accessibility */ + outline: 5px auto Highlight; + outline: 5px auto -webkit-focus-ring-color; + } + input[type="range"] { height: 2.4em; appearance: none; From bd42460e1337f0f04960e2649d6ba53a1ded3047 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 5 Apr 2023 12:27:13 +0100 Subject: [PATCH 11/11] Derp --- res/css/views/elements/_Slider.pcss | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/res/css/views/elements/_Slider.pcss b/res/css/views/elements/_Slider.pcss index b34cff346f8..e234cad8378 100644 --- a/res/css/views/elements/_Slider.pcss +++ b/res/css/views/elements/_Slider.pcss @@ -19,12 +19,6 @@ limitations under the License. margin: 0; flex-grow: 1; - &:focus-within { - /* focus outline for accessibility */ - outline: 5px auto Highlight; - outline: 5px auto -webkit-focus-ring-color; - } - input[type="range"] { height: 2.4em; appearance: none; @@ -40,7 +34,7 @@ limitations under the License. --active-color: $slider-background-color; } - &:focus { + &:focus:not(.focus-visible) { outline: none; }