fix: make ProxyMenu more accessible to screen readers (#11312)

* wip: commit progress on latency update

* chore: add stories and clean up tests

* refactor: clean up code

* fix: make sure headers aren't treated as interactive elements

* refactor: clean up tests

* fix: clean up stories

* docs: add clarifying comment

* fix: update stories again

* fix: clean up/extend prop definitions

* refactor: quick cleanup

* fix: apply Kira's feedback

* refactor: clean up abbr markup to account for pronunciation

* fix: more cleanup

* fix: refine screen reader output for VoiceOver

* refactor: clean up and redefine tests

* feature: add finishing touches
This commit is contained in:
Michael Smith
2024-01-07 18:37:01 -05:00
committed by GitHub
parent 8a9fe2bf00
commit 31f8fac1b9
6 changed files with 301 additions and 29 deletions
+1
View File
@@ -60,6 +60,7 @@
"idtoken",
"Iflag",
"incpatch",
"initialisms",
"ipnstate",
"isatty",
"Jobf",
+74
View File
@@ -0,0 +1,74 @@
import { type PropsWithChildren } from "react";
import type { Meta, StoryObj } from "@storybook/react";
import { Abbr } from "./Abbr";
// Just here to make the abbreviated part more obvious in the component library
const Underline = ({ children }: PropsWithChildren) => (
<span css={{ textDecoration: "underline dotted" }}>{children}</span>
);
const meta: Meta<typeof Abbr> = {
title: "components/Abbr",
component: Abbr,
decorators: [
(Story) => (
<>
<p>Try the following text out in a screen reader!</p>
<Story />
</>
),
],
};
export default meta;
type Story = StoryObj<typeof Abbr>;
export const InlinedShorthand: Story = {
args: {
pronunciation: "shorthand",
children: "ms",
title: "milliseconds",
},
decorators: [
(Story) => (
<p css={{ maxWidth: "40em" }}>
The physical pain of getting bonked on the head with a cartoon mallet
lasts precisely 593{" "}
<Underline>
<Story />
</Underline>
. The emotional turmoil and complete embarrassment lasts forever.
</p>
),
],
};
export const Acronym: Story = {
args: {
pronunciation: "acronym",
children: "NASA",
title: "National Aeronautics and Space Administration",
},
decorators: [
(Story) => (
<Underline>
<Story />
</Underline>
),
],
};
export const Initialism: Story = {
args: {
pronunciation: "initialism",
children: "CLI",
title: "Command-Line Interface",
},
decorators: [
(Story) => (
<Underline>
<Story />
</Underline>
),
],
};
+97
View File
@@ -0,0 +1,97 @@
import { render, screen } from "@testing-library/react";
import { Abbr, type Pronunciation } from "./Abbr";
type AbbreviationData = {
abbreviation: string;
title: string;
expectedLabel: string;
};
type AssertionInput = AbbreviationData & {
pronunciation: Pronunciation;
};
function assertAccessibleLabel({
abbreviation,
title,
expectedLabel,
pronunciation,
}: AssertionInput) {
const { unmount } = render(
<Abbr title={title} pronunciation={pronunciation}>
{abbreviation}
</Abbr>,
);
screen.getByLabelText(expectedLabel, { selector: "abbr" });
unmount();
}
describe(Abbr.name, () => {
it("Has an aria-label that equals the title if the abbreviation is shorthand", () => {
const sampleShorthands: AbbreviationData[] = [
{
abbreviation: "ms",
title: "milliseconds",
expectedLabel: "milliseconds",
},
{
abbreviation: "g",
title: "grams",
expectedLabel: "grams",
},
];
for (const shorthand of sampleShorthands) {
assertAccessibleLabel({ ...shorthand, pronunciation: "shorthand" });
}
});
it("Has an aria label with title and 'flattened' pronunciation if abbreviation is acronym", () => {
const sampleAcronyms: AbbreviationData[] = [
{
abbreviation: "NASA",
title: "National Aeronautics and Space Administration",
expectedLabel: "Nasa (National Aeronautics and Space Administration)",
},
{
abbreviation: "AWOL",
title: "Absent without Official Leave",
expectedLabel: "Awol (Absent without Official Leave)",
},
{
abbreviation: "YOLO",
title: "You Only Live Once",
expectedLabel: "Yolo (You Only Live Once)",
},
];
for (const acronym of sampleAcronyms) {
assertAccessibleLabel({ ...acronym, pronunciation: "acronym" });
}
});
it("Has an aria label with title and initialized pronunciation if abbreviation is initialism", () => {
const sampleInitialisms: AbbreviationData[] = [
{
abbreviation: "FBI",
title: "Federal Bureau of Investigation",
expectedLabel: "F.B.I. (Federal Bureau of Investigation)",
},
{
abbreviation: "YMCA",
title: "Young Men's Christian Association",
expectedLabel: "Y.M.C.A. (Young Men's Christian Association)",
},
{
abbreviation: "CLI",
title: "Command-Line Interface",
expectedLabel: "C.L.I. (Command-Line Interface)",
},
];
for (const initialism of sampleInitialisms) {
assertAccessibleLabel({ ...initialism, pronunciation: "initialism" });
}
});
});
+66
View File
@@ -0,0 +1,66 @@
import { type FC, type HTMLAttributes } from "react";
export type Pronunciation = "shorthand" | "acronym" | "initialism";
type AbbrProps = HTMLAttributes<HTMLElement> & {
children: string;
title: string;
pronunciation?: Pronunciation;
};
/**
* A more sophisticated version of the native <abbr> element.
*
* Features:
* - Better type-safety (requiring you to include certain properties)
* - All built-in HTML styling is stripped away by default
* - Better integration with screen readers (like exposing the title prop to
* them), with more options for influencing how they pronounce text
*/
export const Abbr: FC<AbbrProps> = ({
children,
title,
pronunciation = "shorthand",
...delegatedProps
}) => {
return (
<abbr
// Title attributes usually aren't natively available to screen readers;
// always have to supplement with aria-label
title={title}
aria-label={getAccessibleLabel(children, title, pronunciation)}
css={{
textDecoration: "inherit",
letterSpacing: children === children.toUpperCase() ? "0.02em" : "0",
}}
{...delegatedProps}
>
<span aria-hidden>{children}</span>
</abbr>
);
};
function getAccessibleLabel(
abbreviation: string,
title: string,
pronunciation: Pronunciation,
): string {
if (pronunciation === "initialism") {
return `${initializeText(abbreviation)} (${title})`;
}
if (pronunciation === "acronym") {
return `${flattenPronunciation(abbreviation)} (${title})`;
}
return title;
}
function initializeText(text: string): string {
return text.trim().toUpperCase().replaceAll(/\B/g, ".") + ".";
}
function flattenPronunciation(text: string): string {
const trimmed = text.trim();
return (trimmed[0] ?? "").toUpperCase() + trimmed.slice(1).toLowerCase();
}
@@ -18,6 +18,8 @@ import { ProxyStatusLatency } from "components/ProxyStatusLatency/ProxyStatusLat
import { CoderIcon } from "components/Icons/CoderIcon";
import { usePermissions } from "hooks/usePermissions";
import { UserDropdown } from "./UserDropdown/UserDropdown";
import { visuallyHidden } from "@mui/utils";
import { Abbr } from "components/Abbr/Abbr";
export const USERS_LINK = `/users?filter=${encodeURIComponent(
"status:active",
@@ -214,25 +216,22 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
const isLoadingLatencies = Object.keys(latencies).length === 0;
const isLoading = proxyContextValue.isLoading || isLoadingLatencies;
const permissions = usePermissions();
const proxyLatencyLoading = (proxy: TypesGen.Region): boolean => {
if (!refetchDate) {
// Only show loading if the user manually requested a refetch
return false;
}
const latency = latencies?.[proxy.id];
// Only show a loading spinner if:
// - A latency exists. This means the latency was fetched at some point, so the
// loader *should* be resolved.
// - A latency exists. This means the latency was fetched at some point, so
// the loader *should* be resolved.
// - The proxy is healthy. If it is not, the loader might never resolve.
// - The latency reported is older than the refetch date. This means the latency
// is stale and we should show a loading spinner until the new latency is
// fetched.
if (proxy.healthy && latency && latency.at < refetchDate) {
return true;
}
return false;
// - The latency reported is older than the refetch date. This means the
// latency is stale and we should show a loading spinner until the new
// latency is fetched.
const latency = latencies[proxy.id];
return proxy.healthy && latency !== undefined && latency.at < refetchDate;
};
if (isLoading) {
@@ -257,12 +256,18 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
"& .MuiSvgIcon-root": { fontSize: 14 },
}}
>
<span css={{ ...visuallyHidden }}>
Latency for {selectedProxy?.display_name ?? "your region"}
</span>
{selectedProxy ? (
<div css={{ display: "flex", gap: 8, alignItems: "center" }}>
<div css={{ width: 16, height: 16, lineHeight: 0 }}>
<img
src={selectedProxy.icon_url}
// Empty alt text used because we don't want to double up on
// screen reader announcements from visually-hidden span
alt=""
src={selectedProxy.icon_url}
css={{
objectFit: "contain",
width: "100%",
@@ -270,6 +275,7 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
}}
/>
</div>
<ProxyStatusLatency
latency={latencies?.[selectedProxy.id]?.latencyMS}
isLoading={proxyLatencyLoading(selectedProxy)}
@@ -279,12 +285,18 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
"Select Proxy"
)}
</Button>
<Menu
open={isOpen}
anchorEl={buttonRef.current}
onClick={closeMenu}
onClose={closeMenu}
css={{ "& .MuiMenu-paper": { paddingTop: 8, paddingBottom: 8 } }}
// autoFocus here does not affect modal focus; it affects whether the
// first item in the list will get auto-focus when the menu opens. Have
// to turn this off because otherwise, screen readers will skip over all
// the descriptive text and will only have access to the latency options
autoFocus={false}
>
<div
css={{
@@ -296,6 +308,8 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
}}
>
<h4
autoFocus
tabIndex={-1}
css={{
fontSize: "inherit",
fontWeight: 600,
@@ -306,6 +320,7 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
>
Select a region nearest to you
</h4>
<p
css={{
fontSize: 13,
@@ -315,12 +330,17 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
}}
>
Workspace proxies improve terminal and web app connections to
workspaces. This does not apply to CLI connections. A region must be
manually selected, otherwise the default primary region will be
used.
workspaces. This does not apply to{" "}
<Abbr title="Command-Line Interface" pronunciation="initialism">
CLI
</Abbr>{" "}
connections. A region must be manually selected, otherwise the
default primary region will be used.
</p>
</div>
<Divider css={{ borderColor: theme.palette.divider }} />
{proxyContextValue.proxies
?.sort((a, b) => {
const latencyA = latencies?.[a.id]?.latencyMS ?? Infinity;
@@ -329,6 +349,9 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
})
.map((proxy) => (
<MenuItem
key={proxy.id}
selected={proxy.id === selectedProxy?.id}
css={{ fontSize: 14 }}
onClick={() => {
if (!proxy.healthy) {
displayError("Please select a healthy workspace proxy.");
@@ -339,9 +362,6 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
proxyContextValue.setProxy(proxy);
closeMenu();
}}
key={proxy.id}
selected={proxy.id === selectedProxy?.id}
css={{ fontSize: 14 }}
>
<div
css={{
@@ -362,7 +382,9 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
}}
/>
</div>
{proxy.display_name}
<ProxyStatusLatency
latency={latencies?.[proxy.id]?.latencyMS}
isLoading={proxyLatencyLoading(proxy)}
@@ -370,7 +392,9 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
</div>
</MenuItem>
))}
<Divider css={{ borderColor: theme.palette.divider }} />
{Boolean(permissions.editWorkspaceProxies) && (
<MenuItem
css={{ fontSize: 14 }}
@@ -381,6 +405,7 @@ const ProxyMenu: FC<ProxyMenuProps> = ({ proxyContextValue }) => {
Proxy settings
</MenuItem>
)}
<MenuItem
css={{ fontSize: 14 }}
onClick={(e) => {
@@ -4,6 +4,8 @@ import Tooltip from "@mui/material/Tooltip";
import { type FC } from "react";
import { getLatencyColor } from "utils/latency";
import CircularProgress from "@mui/material/CircularProgress";
import { visuallyHidden } from "@mui/utils";
import { Abbr } from "components/Abbr/Abbr";
interface ProxyStatusLatencyProps {
latency?: number;
@@ -33,22 +35,29 @@ export const ProxyStatusLatency: FC<ProxyStatusLatencyProps> = ({
}
if (!latency) {
const notAvailableText = "Latency not available";
return (
<Tooltip title="Latency not available">
<HelpOutline
css={{
marginLeft: "auto",
fontSize: "14px !important",
color,
}}
/>
<Tooltip title={notAvailableText}>
<>
<span css={{ ...visuallyHidden }}>{notAvailableText}</span>
<HelpOutline
css={{
marginLeft: "auto",
fontSize: "14px !important",
color,
}}
/>
</>
</Tooltip>
);
}
return (
<div css={{ color, fontSize: 13, marginLeft: "auto" }}>
{latency.toFixed(0)}ms
</div>
<p css={{ color, fontSize: 13, margin: "0 0 0 auto" }}>
<span css={{ ...visuallyHidden }}>Latency: </span>
{latency.toFixed(0)}
<Abbr title="milliseconds">ms</Abbr>
</p>
);
};