Skip to content

Commit 4e4fcbc

Browse files
committed
fix(ui): give the shared components the behaviour their roles promise
`SegmentedControl` announced `role="radiogroup"` while behaving like a row of buttons: every option was its own tab stop and the arrow keys did nothing, so a screen-reader user was told to expect one control and handed six. It now carries a roving `tabIndex` and Arrow/Home/End handling that moves focus and selects as it travels, skipping disabled options and wrapping at the ends. Six screens use it (users, settings store, tasks, flags, add-people, branding preview). `initTheme` called `media.addEventListener` unguarded. On Safari < 14 that is a TypeError thrown inside the effect that calls it, which takes the whole theme init down and leaves the app on whatever the class list happened to say. It now falls back to the deprecated `addListener`, and to a no-op unsubscribe when neither exists. The auth screens used the default 36px `Input` and 40px `size="lg"` Button against a deck that specifies ~46px fields and ~48px buttons on a 10px radius (docs/superpowers/specs/hifi-gap/auth-screens-02-07.md). `AuthCardShell` now sizes its card's controls once, so a seventh auth screen cannot be born a size behind — and the fields clear the 44px phone tap target, which they did not before. The `max-lg:min-h-11` overrides those `size="lg"` buttons carried are now redundant and gone. `split-light` put both columns on `bg-background`, leaving the card's 1px border as the only thing marking the split on register and accept-invite. The aside moves to `bg-secondary/40` — the same page surface the centred variant sits on, and still a token, so the dark theme applies. `useRelativeTime`'s `until` re-implemented the null/invalid parsing `ageOf` already encodes. It cannot simply call `ageOf`, which clamps at zero and would read every future deadline as expired; the shared `parseTimestamp` is extracted instead and `timeUntil` is its unclamped sibling. The file table's select-all box grew its phone hit area with a symmetric `before:-inset-3.5`, but sits at the top edge of a table whose own container is `overflow-x-auto` (which computes the block axis to `auto` too) inside an `overflow-hidden` card — the upward 14px was clipped, and clipped overflow is not hit-testable. A 56px header row on phones centres the box with 20px of cell either side, so the whole 44px target lands inside both clipping boxes and clear of row one's own bleed. Settings Browse sized its card with `100vh - --app-chrome-h - 15rem`, where the `15rem` was a hand-measured stand-in for the heading, tabs and search bar above it and drifted whenever any of them changed. `PageShell` grows a `fill` prop that hands the leftover viewport height to the content column, so the card asks for `lg:flex-1` and the arithmetic stays in the layout engine. Tests: `@testing-library/user-event` is added so the keyboard work is driven through real Tab/Arrow/Home/End rather than asserted from markup. The layout items are class-level assertions — jsdom has no layout engine — so the emitted utilities were checked against a real `npm run build` instead. Closes #297
1 parent 4a3e588 commit 4e4fcbc

21 files changed

Lines changed: 531 additions & 45 deletions

File tree

‎modules/file_storage/file_storage/pages/components/FileTable.tsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ const HEAD =
3939
'bg-secondary/40 text-[11px] font-semibold uppercase tracking-[0.08em] ' +
4040
'text-muted-foreground sm:px-6';
4141

42+
// The select-all box has the same bleed as the rows, but nothing above the
43+
// header row to bleed *into*: the table's own container is `overflow-x-auto`
44+
// (which computes the block axis to `auto` too) inside a card the page renders
45+
// `overflow-hidden`, and a clipped overflow region is not hit-testable. A 56px
46+
// header row on phones centres the 16px box with 20px of cell either side, so
47+
// the whole 44px target lands inside both clipping boxes — and clear of row
48+
// one's own bleed, which would otherwise sit on top of it.
49+
const HEAD_CHECKBOX_CELL = 'w-10 max-lg:h-14 sm:pr-0';
50+
4251
export function FileTable({
4352
files,
4453
selectedIds,
@@ -57,7 +66,7 @@ export function FileTable({
5766
<TableHeader>
5867
<TableRow>
5968
{canDelete && (
60-
<TableHead className={cn(HEAD, 'w-10 sm:pr-0')}>
69+
<TableHead className={cn(HEAD, HEAD_CHECKBOX_CELL)}>
6170
{/* Indeterminate while only some rows are ticked, so the header
6271
box never claims a selection the footer count contradicts. */}
6372
<Checkbox
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import '@testing-library/jest-dom/vitest';
2+
import { configureI18n } from '@simple-module-py/i18n';
3+
import { render, screen } from '@testing-library/react';
4+
import { describe, expect, test, vi } from 'vitest';
5+
6+
configureI18n({
7+
locale: 'en',
8+
messages: {
9+
'file_storage.table.select_all': 'Select every file on this page',
10+
'file_storage.table.select_row': 'Select {name}',
11+
'file_storage.table.filename': 'File',
12+
'file_storage.table.type': 'Type',
13+
'file_storage.table.size': 'Size',
14+
'file_storage.table.uploaded_by': 'Uploaded by',
15+
'file_storage.table.when': 'When',
16+
'file_storage.table.actions': 'Actions',
17+
'file_storage.actions.download': 'Download',
18+
},
19+
});
20+
21+
import { FileTable } from '../file_storage/pages/components/FileTable';
22+
import type { StoredFile } from '../file_storage/pages/types';
23+
24+
const FILES: StoredFile[] = [
25+
{
26+
id: 'f1',
27+
filename: 'quarterly.pdf',
28+
content_type: 'application/pdf',
29+
size_bytes: 2048,
30+
uploaded_by_label: 'admin',
31+
created_at: '2026-09-03T10:00:00Z',
32+
} as StoredFile,
33+
];
34+
35+
function renderTable(selectedIds: string[] = []) {
36+
return render(
37+
<FileTable
38+
files={FILES}
39+
selectedIds={selectedIds}
40+
canDelete
41+
onToggleRow={vi.fn()}
42+
onToggleAll={vi.fn()}
43+
empty={null}
44+
/>,
45+
);
46+
}
47+
48+
describe('FileTable selection checkboxes', () => {
49+
test('both checkboxes are named for a screen reader', () => {
50+
renderTable();
51+
52+
expect(
53+
screen.getByRole('checkbox', { name: 'Select every file on this page' }),
54+
).toBeInTheDocument();
55+
expect(screen.getByRole('checkbox', { name: /quarterly\.pdf/ })).toBeInTheDocument();
56+
});
57+
58+
test('both checkboxes carry the 44px phone bleed', () => {
59+
renderTable();
60+
61+
for (const name of ['Select every file on this page', /quarterly\.pdf/] as const) {
62+
expect(screen.getByRole('checkbox', { name })).toHaveClass('max-lg:before:-inset-3.5');
63+
}
64+
});
65+
66+
test('the header row is tall enough on phones to hold that bleed', () => {
67+
renderTable();
68+
69+
// The 14px above the select-all box has nowhere to go otherwise: the
70+
// table container is `overflow-x-auto` inside an `overflow-hidden` card,
71+
// and a clipped overflow region is not hit-testable. A 56px header row
72+
// centres the 16px box with 20px of cell either side, so the whole target
73+
// stays inside.
74+
const cell = screen
75+
.getByRole('checkbox', { name: 'Select every file on this page' })
76+
.closest('th');
77+
78+
expect(cell).toHaveClass('max-lg:h-14');
79+
});
80+
81+
test('the header reports the all-selected state', () => {
82+
renderTable(['f1']);
83+
84+
expect(screen.getByRole('checkbox', { name: 'Select every file on this page' })).toBeChecked();
85+
});
86+
});

‎modules/settings/settings/pages/Browse.tsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ function Browse({ settings, pagination, counts, filters }: Props) {
6565
<>
6666
<Head title={t(keys.settings.browse.title)} />
6767
<PageShell
68+
fill
6869
title={t(keys.settings.browse.title)}
6970
description={t(keys.settings.browse.description)}
7071
actions={
@@ -102,10 +103,12 @@ function Browse({ settings, pagination, counts, filters }: Props) {
102103
</div>
103104
</div>
104105

105-
{/* A min-height rather than a fixed one: the deck's card fills the
106-
viewport with the footer pinned to the bottom, and `min-h` gets
107-
that look without clipping a full page of rows on a short window. */}
108-
<Card className="flex flex-col overflow-hidden border-border p-0 lg:min-h-[calc(100vh-var(--app-chrome-h)-15rem)]">
106+
{/* The deck's card fills the viewport with the footer pinned to the
107+
bottom. `fill` on the shell hands the leftover height to this
108+
column, so the card grows into it — the `15rem` this replaced was a
109+
hand-measured stand-in for the heading, tabs and search bar above,
110+
and drifted whenever any of them changed. */}
111+
<Card className="flex flex-col overflow-hidden border-border p-0 lg:flex-1">
109112
{settings.length === 0 ? (
110113
<div className="flex flex-1 flex-col items-center justify-center gap-2 py-16 text-muted-foreground">
111114
<SettingsIcon className="size-8" aria-hidden="true" />

‎modules/users/users/auth_local/components/LoginForm.tsx‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export function LoginForm({
107107

108108
{error && <p className="text-sm text-destructive">{error}</p>}
109109

110-
<Button type="submit" size="lg" className="w-full max-lg:min-h-11" disabled={loading}>
110+
<Button type="submit" size="lg" className="w-full" disabled={loading}>
111111
{loading ? t(keys.users.login.submitting) : t(keys.users.login.submit)}
112112
</Button>
113113
</form>
@@ -122,14 +122,7 @@ export function LoginForm({
122122
<span className="h-px flex-1 bg-border" />
123123
</div>
124124
{oauthProviders.map((provider) => (
125-
<Button
126-
key={provider.name}
127-
type="button"
128-
variant="outline"
129-
size="lg"
130-
asChild
131-
className="max-lg:min-h-11"
132-
>
125+
<Button key={provider.name} type="button" variant="outline" size="lg" asChild>
133126
<a href={`/api/users/auth/${provider.name}/login`}>
134127
{t(keys.users.login.continue_with, { name: provider.display_name })}
135128
</a>

‎modules/users/users/pages/AcceptInvite.tsx‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,7 @@ function AcceptInvite() {
150150

151151
{error && <p className="text-sm text-destructive">{error}</p>}
152152

153-
<Button
154-
type="submit"
155-
size="lg"
156-
className="w-full max-lg:min-h-11"
157-
disabled={loading || !token}
158-
>
153+
<Button type="submit" size="lg" className="w-full" disabled={loading || !token}>
159154
{loading ? t(keys.users.accept_invite.submitting) : t(keys.users.accept_invite.submit)}
160155
</Button>
161156
</form>

‎modules/users/users/pages/ForgotPassword.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ function ForgotPassword() {
8484
autoComplete="email"
8585
/>
8686
</AuthField>
87-
<Button type="submit" size="lg" className="w-full max-lg:min-h-11" disabled={loading}>
87+
<Button type="submit" size="lg" className="w-full" disabled={loading}>
8888
{loading
8989
? t(keys.users.forgot_password.submitting)
9090
: t(keys.users.forgot_password.submit)}

‎modules/users/users/pages/Register.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ function Register() {
139139

140140
{error && <p className="text-sm text-destructive">{error}</p>}
141141

142-
<Button type="submit" size="lg" className="w-full max-lg:min-h-11" disabled={loading}>
142+
<Button type="submit" size="lg" className="w-full" disabled={loading}>
143143
{loading ? t(keys.users.register.submitting) : t(keys.users.register.submit)}
144144
</Button>
145145
</form>

‎modules/users/users/pages/ResetPassword.tsx‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,7 @@ function ResetPassword() {
153153

154154
{error && <p className="text-sm text-destructive">{error}</p>}
155155

156-
<Button
157-
type="submit"
158-
size="lg"
159-
className="w-full max-lg:min-h-11"
160-
disabled={loading || !token}
161-
>
156+
<Button type="submit" size="lg" className="w-full" disabled={loading || !token}>
162157
{loading ? t(keys.users.reset_password.submitting) : t(keys.users.reset_password.submit)}
163158
</Button>
164159
</form>

‎package-lock.json‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"@biomejs/biome": "^2.5.7",
2020
"@testing-library/jest-dom": "^7.0.0",
2121
"@testing-library/react": "^16.3.2",
22+
"@testing-library/user-event": "^14.6.7",
2223
"@types/node": "^26.1.2",
2324
"@vitejs/plugin-react": "^6.0.5",
2425
"jsdom": "^30.0.1",

0 commit comments

Comments
 (0)