Skip to content

Commit 9f94032

Browse files
Valentin Grünerclaude
andcommitted
fix(profile): make the group cards headings a reader can navigate by
`CardTitle` renders a div, so the group names looked like headings and were not ones. The section above them is an `h2`, which makes each card an `h3` — and without that a screen-reader user has no way to move between groups. Found by two tests this adds: that the cards are ordered worst standing first, whatever order the workspace lists its groups in, and that past three the card collapses and says how many are hidden. Both are behaviour a story cannot show. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MAQxdMw579siMh1wXKEEh4
1 parent f0c2329 commit 9f94032

3 files changed

Lines changed: 55 additions & 16 deletions

File tree

webapp/src/components/profile/PracticeGroupStandingCard.test.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,53 @@ describe("PracticeGroupStandingCard", () => {
4747
expect(onOpenDetails).toHaveBeenCalledWith(group);
4848
});
4949

50+
it("puts the groups that need attention first", () => {
51+
// The order is the registry's, so a reader scanning top-down meets the worst standing first —
52+
// and it must not follow the order the workspace happens to list its groups in.
53+
const groups = ["going-well", "needs-attention", "mixed"].map((slug, index) => ({
54+
...group,
55+
id: index + 1,
56+
slug,
57+
name: slug,
58+
}));
59+
render(
60+
<PracticeGroupStandingCard
61+
groups={groups}
62+
standings={{
63+
"going-well": { ...standing, groupSlug: "going-well", standing: "STRENGTH" },
64+
"needs-attention": {
65+
...standing,
66+
groupSlug: "needs-attention",
67+
standing: "DEVELOPING",
68+
},
69+
mixed: { ...standing, groupSlug: "mixed", standing: "MIXED" },
70+
}}
71+
isLoading={false}
72+
/>,
73+
);
74+
75+
const headings = screen.getAllByRole("heading", { level: 3 }).map((node) => node.textContent);
76+
expect(headings).toStrictEqual(["needs-attention", "mixed", "going-well"]);
77+
});
78+
79+
it("collapses past three groups and says how many are hidden", () => {
80+
const groups = Array.from({ length: 5 }, (_, index) => ({
81+
...group,
82+
id: index + 1,
83+
slug: `group-${index}`,
84+
name: `Group ${index}`,
85+
}));
86+
render(<PracticeGroupStandingCard groups={groups} standings={{}} isLoading={false} />);
87+
88+
expect(screen.getAllByRole("heading", { level: 3 })).toHaveLength(3);
89+
const toggle = screen.getByRole("button", { name: "Show all 5 practice groups" });
90+
expect(toggle.getAttribute("aria-expanded")).toBe("false");
91+
92+
fireEvent.click(toggle);
93+
expect(screen.getAllByRole("heading", { level: 3 })).toHaveLength(5);
94+
screen.getByRole("button", { name: "Show fewer groups" });
95+
});
96+
5097
it("shows an empty workspace state", () => {
5198
render(<PracticeGroupStandingCard groups={[]} standings={{}} isLoading={false} />);
5299
screen.getByText("No practice groups are configured yet.");

webapp/src/components/profile/PracticeGroupStandingCard.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { PRACTICE_GROUP_STANDING_DEFS } from "@/components/practice-vocabulary/p
77
import { statusToneClass, statusValues } from "@/components/practice-vocabulary/status-def";
88
import { StatusBadge } from "@/components/practice-vocabulary/StatusBadge";
99
import { Button } from "@/components/ui/button";
10-
import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
10+
import { Card, CardContent, CardHeader } from "@/components/ui/card";
1111
import { Skeleton } from "@/components/ui/skeleton";
1212
import { artifactKindCountLabel, artifactKindIcon } from "@/lib/artifact-kinds";
1313
import { cn } from "@/lib/utils";
@@ -117,9 +117,9 @@ export function PracticeGroupStandingCard({
117117
>
118118
<Icon className="size-5" aria-hidden />
119119
</span>
120-
<CardTitle className="min-w-0 flex-1 text-lg leading-snug">
121-
{group.name}
122-
</CardTitle>
120+
{/* A real heading, not just card-title styling: the section above is an h2, so
121+
these are the level a reader navigates the groups by. */}
122+
<h3 className="min-w-0 flex-1 text-lg font-medium leading-snug">{group.name}</h3>
123123
{onOpenDetails && (
124124
<ChevronRightIcon className="size-4 text-muted-foreground" aria-hidden />
125125
)}

webapp/src/components/profile/practice-trend-presentation.ts

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,13 @@ function reviewedWork(count: number): string {
66
return `${count} ${count === 1 ? "piece" : "pieces"} of reviewed work`;
77
}
88

9-
/**
10-
* Which trend a chip is showing. The server computes the two differently and the sentence has to
11-
* follow: a practice trend really does hold one bundle of opportunities against the one before it,
12-
* while a group trend pools the practices' finished comparisons — there is no group-level "latest
13-
* against previous" to describe.
14-
*/
9+
/** A practice trend compares two bundles; a group trend pools finished comparisons. */
1510
export type TrendScope = "practice" | "group";
1611

1712
/**
18-
* Says what the returned support actually establishes — never a comparison the server did not make.
19-
*
20-
* Three cases the earlier one-liner got wrong. `INSUFFICIENT_EVIDENCE` means no posterior was formed
21-
* at all, so describing a comparison there contradicted the chip's own "Not enough to compare yet".
22-
* A group trend never compares bundles. And the calendar span covers the visible evidence trail, not
23-
* the two bundles, so it is its own sentence rather than a clause hung on the comparison.
13+
* Says what the support establishes, never a comparison the server did not make: none is formed for
14+
* `INSUFFICIENT_EVIDENCE`, and a group trend compares no bundles at all. The calendar span covers
15+
* the visible evidence trail rather than the two bundles, so it stands as its own sentence.
2416
*/
2517
export function formatTrendProvenance(
2618
support: TrendSupport,

0 commit comments

Comments
 (0)