mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): add alt text to Avatar so workspaces images pass WCAG (#26223)
## Summary Fixes the WCAG image-alt failures reported on https://dev.coder.com/workspaces. The audit flagged ~52 `<img>` elements without an `alt` attribute, all matching the inner `<img>` rendered by Radix `AvatarPrimitive.Image` inside our `Avatar` component (selectors like `.size-full.object-contain`, `.size-[--avatar-lg].rounded-[6px]`, `.size-[--avatar-sm]`). Two `ExternalImage` callsites on the same page were also missing `alt`. ## Changes - `Avatar`: add optional `alt?: string` and forward it to `AvatarPrimitive.Image`. Default is `""`, which marks the avatar as decorative and removes it from the accessibility tree. Every callsite on the workspaces page already renders the human-readable name (owner, template, organization, user) as adjacent text, so decorative-by-default is the WCAG-correct behavior. Callers that need a meaningful alt can override. - `AvatarData`: thread an optional `alt` through to the internal default `Avatar`. - `WorkspacesTable` `IconAppLink` `ExternalImage`: pass `alt=""`. The wrapping `BaseIconLink` already exposes the app name through an `sr-only` span on the link. - `BatchDeleteConfirmation` resource icons `ExternalImage`: pass `alt=""`. The resource-type label sits next to each icon. - `WorkspacesPageView.stories.tsx` `AllStates`: add a play function that scans the rendered canvas and asserts every `<img>` has an `alt` attribute, to prevent regressions. ## Validation - `pnpm check`, `pnpm lint`, `pnpm format` clean. - `pnpm test -- src/pages/WorkspacesPage/WorkspacesPage.test.tsx` passes (13/13). - Pre-commit (`make pre-commit`) passes locally. <details> <summary>Implementation plan</summary> ### Root cause The `Avatar` component (`site/src/components/Avatar/Avatar.tsx`) rendered `AvatarPrimitive.Image` without an `alt` attribute. Every consumer (`AvatarData`, `TopbarAvatar`, workspace table rows, filter menus, empty state, batch dialogs, "New workspace" dropdown) inherited the missing-alt bug, which is why a single page produced ~52 violations. ### Fix 1. Make `Avatar` accept an `alt` prop, default `""`, and forward it to the underlying `<img>`. Drop-in compatible with every existing call. 2. Mirror the prop on `AvatarData` so callers can label the implicit avatar without composing their own. 3. Explicitly mark the workspaces-page `ExternalImage` callsites as decorative because each is paired with adjacent text. 4. Lock the behavior with a Storybook play function so a future regression on the workspaces page fails CI. ### Why `alt=""` by default All workspaces-page avatars are rendered next to the corresponding name. Per WCAG, repeating that name in the image's alt text would only add noise for screen-reader users. Empty alt removes the image from the accessibility tree, which is the correct decorative pattern. </details> --- _PR opened by Coder Agents on behalf of @tracyjohnsonux._
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, waitFor, within } from "storybook/test";
|
||||
import { Avatar } from "./Avatar";
|
||||
|
||||
const meta: Meta<typeof Avatar> = {
|
||||
@@ -74,3 +75,19 @@ export const FallbackSmSize: Story = {
|
||||
fallback: "Adriana Rodrigues",
|
||||
},
|
||||
};
|
||||
|
||||
export const WithAlt: Story = {
|
||||
args: {
|
||||
variant: "icon",
|
||||
src: "/icon/code.svg",
|
||||
alt: "Visual Studio Code template",
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
await waitFor(async () => {
|
||||
const img = await within(canvasElement).findByAltText(
|
||||
"Visual Studio Code template",
|
||||
);
|
||||
expect(img.tagName).toBe("IMG");
|
||||
});
|
||||
},
|
||||
};
|
||||
|
||||
@@ -56,6 +56,12 @@ export type AvatarProps = AvatarPrimitive.AvatarProps &
|
||||
VariantProps<typeof avatarVariants> & {
|
||||
src?: string;
|
||||
fallback?: string;
|
||||
/**
|
||||
* Alt text for the inner `<img>`. Defaults to `""` (decorative,
|
||||
* hidden from assistive tech). Pass a descriptive value when no
|
||||
* adjacent text identifies the content.
|
||||
*/
|
||||
alt?: string;
|
||||
ref?: React.Ref<React.ComponentRef<typeof AvatarPrimitive.Root>>;
|
||||
};
|
||||
|
||||
@@ -65,6 +71,7 @@ export const Avatar: React.FC<AvatarProps> = ({
|
||||
variant,
|
||||
src,
|
||||
fallback,
|
||||
alt = "",
|
||||
children,
|
||||
...props
|
||||
}) => {
|
||||
@@ -77,6 +84,7 @@ export const Avatar: React.FC<AvatarProps> = ({
|
||||
>
|
||||
<AvatarPrimitive.Image
|
||||
src={src}
|
||||
alt={alt}
|
||||
className="aspect-square size-full object-contain"
|
||||
style={getExternalImageStylesFromUrl(theme.externalImages, src)}
|
||||
/>
|
||||
|
||||
@@ -17,6 +17,8 @@ interface AvatarDataProps {
|
||||
*/
|
||||
imgFallbackText?: string;
|
||||
|
||||
alt?: string;
|
||||
|
||||
/**
|
||||
* When true, the title and subtitle clip with an ellipsis if they overflow
|
||||
* the available width. Off by default because callers that pass non-text
|
||||
@@ -31,6 +33,7 @@ export const AvatarData: FC<AvatarDataProps> = ({
|
||||
src,
|
||||
imgFallbackText,
|
||||
avatar,
|
||||
alt = "",
|
||||
truncate = false,
|
||||
}) => {
|
||||
if (!avatar) {
|
||||
@@ -39,6 +42,7 @@ export const AvatarData: FC<AvatarDataProps> = ({
|
||||
size="lg"
|
||||
src={src}
|
||||
fallback={(typeof title === "string" ? title : imgFallbackText) || "-"}
|
||||
alt={alt}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3,13 +3,14 @@ import { getExternalImageStylesFromUrl } from "#/theme/externalImages";
|
||||
|
||||
export const ExternalImage: React.FC<React.ComponentPropsWithRef<"img">> = ({
|
||||
style,
|
||||
alt = "",
|
||||
...props
|
||||
}) => {
|
||||
const theme = useTheme();
|
||||
|
||||
return (
|
||||
// biome-ignore lint/a11y/useAltText: alt should be passed in as a prop
|
||||
<img
|
||||
alt={alt}
|
||||
style={{
|
||||
...getExternalImageStylesFromUrl(theme.externalImages, props.src),
|
||||
...style,
|
||||
|
||||
@@ -193,6 +193,14 @@ export const AllStates: Story = {
|
||||
workspaces: allWorkspaces,
|
||||
count: allWorkspaces.length,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
await within(canvasElement).findByText(allWorkspaces[0].name);
|
||||
const images = canvasElement.querySelectorAll("img");
|
||||
expect(images.length).toBeGreaterThan(0);
|
||||
for (const img of images) {
|
||||
expect(img).toHaveAttribute("alt");
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
export const Loading: Story = {
|
||||
|
||||
Reference in New Issue
Block a user