From d2066ba5f5fa42232a75a75f3e857cd5fd50e6ed Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 12 Apr 2023 11:57:31 +0100 Subject: [PATCH] Improve accessibility of font slider (#10473) * Clamp font size when disabling "Use custom size" * Switch Slider to use a semantic input range element * Iterate * delint * delint * snapshot * Iterate * Iterate * Fix step size * Add focus outline to slider * Derp --- res/css/views/elements/_Slider.pcss | 190 ++++++++++-------- src/components/views/elements/Slider.tsx | 140 ++++--------- .../views/settings/FontScalingPanel.tsx | 21 +- .../views/settings/FontScalingPanel-test.tsx | 18 +- .../FontScalingPanel-test.tsx.snap | 114 ++--------- 5 files changed, 193 insertions(+), 290 deletions(-) diff --git a/res/css/views/elements/_Slider.pcss b/res/css/views/elements/_Slider.pcss index d7d7dd7168e..e234cad8378 100644 --- a/res/css/views/elements/_Slider.pcss +++ b/res/css/views/elements/_Slider.pcss @@ -16,92 +16,110 @@ 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:not(.focus-visible) { + 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..a6f11aa0751 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 f8aa9186bb7..49fe5576718 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,9 @@ export default class FontScalingPanel extends React.Component { }; public render(): React.ReactNode { + const min = 13; + const max = 18; + return (
{_t("Font size")} @@ -117,9 +121,11 @@ export default class FontScalingPanel extends React.Component {
Aa
""} disabled={this.state.useCustomFontSize} /> @@ -129,7 +135,16 @@ 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, min, max); + if (clamped !== size) { + this.onFontSizeChanged(clamped); + } + } + }} useCheckbox={true} /> diff --git a/test/components/views/settings/FontScalingPanel-test.tsx b/test/components/views/settings/FontScalingPanel-test.tsx index d52dacee54c..9c1be7540f7 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); + }); + }); }); diff --git a/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap b/test/components/views/settings/__snapshots__/FontScalingPanel-test.tsx.snap index 8644bda9875..ac2c9e9945e 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 + +