Release: v0.2.0 - #152
Merged
Merged
Conversation
Version-bump-only scope: 2.x is a Rust-core engine rewrite behind an unchanged Node API. Real work is the hologit-drop blob-write migration + two documented byte re-baselines (integer underscores, markdown bodies). The three cache workarounds are ported, not deleted — gitsheets#184 (the per-sheet refresh API that would let them go) is open and not in 2.x. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Upgrade gitsheets ^1.4.1 → ^2.2.0. The 2.x line rewrites the core in Rust (via @gitsheets/core-napi) but keeps the Node public API surface (openRepo/openStore/Sheet/Transaction) unchanged. Command run: npm install gitsheets@^2.2.0 -w apps/api hologit is dropped as a transitive dependency in 2.x; it no longer appears in the lockfile. Code migration (avatar blob-write path) is in the next commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gitsheets 2.x drops hologit as a dependency. Replace the
BlobObject.write(hologitRepo, buf) pattern with the 2.x surface:
const blob = await repo.writeBlob(buffer: Buffer) // → BlobHandle
await sheet.setAttachments(record, { 'name.jpg': blob })
Sites migrated:
- apps/api/src/routes/people.ts: avatar upload (original + 128 thumb)
- apps/api/scripts/import-laddr/importer.ts: legacy avatar + blog-media
The `as unknown as string` casts are removed — 2.x writeBlob takes
a Buffer natively, matching the actual runtime types throughout.
The three workarounds for gitsheets#184 (swapPublic in store.ts,
git cat-file in attachments.ts, data-repo-lock.ts) are unaffected
and confirmed to still compile. They are kept as-is pending upstream
resolution.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gitsheets 2.x (Rust core) refuses to marshal a null- or undefined- valued field to TOML — `serializeRecords`/`upsert` throw "cannot marshal JS value of type Null/Undefined to a TOML value". gitsheets 1.4.1 (`@iarna/toml`) silently dropped such keys, so they were never written to disk (verified against the on-disk `published` snapshot — no record carries a null-valued key). Our Zod schemas mark optional fields `.nullable().optional()` and the write services normalize cleared fields to `?? null`, so under 2.x every write of a record with a cleared optional field threw a 500. This surfaced as 16 test failures across write-api, people-lifecycle, and import-laddr (all "cannot marshal ... Undefined/Null"). Fix: wrap the per-sheet Standard Schema validator in openPublicStore so the validated record has null/undefined-valued keys stripped (recursively) before it reaches the core marshaller. gitsheets runs the validator host-side and marshals its output, so this is the single authoritative write boundary. The result is byte-identical to 1.4.1's on-disk form: an absent optional field is an absent TOML key. This is not one of the two documented 2.x re-baselines (integer underscores, markdown bodies) — it's an undocumented marshal-contract change in the Rust core that the upgrade plan did not anticipate. Adds a focused store test asserting null-valued keys are dropped and present fields survive (also pins the integer-underscore re-baseline). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion Record the undocumented null/undefined marshal-contract change found during the bump (Rust core throws where 1.4.1 dropped the key) and the stripNullish fix at the write boundary. Check off the validation checklist; note no test needed a re-baseline update. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The `not.toContain('null')` check tripped on the fixture's own
"Nullish Person" / "nullish-person" strings. The three field-absence
regexes above already verify null-valued keys aren't written; replace
the broad substring check with `not.toMatch(/=\s*null\b/)` — no field is
assigned a bare null value.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chore(deps): upgrade gitsheets to 2.x (Rust core, #150)
Author
Changelog- chore(deps): upgrade gitsheets to 2.x (Rust core, #150) [#151] @themightychris
- feat(api): send notifier email through Postmark [#158] @themightychris
- fix(api): rebuild every in-memory index on hot reload [#159] @themightychris
- fix(api): build SAML entityID and SSO endpoints from our own host, not Slack's [#161] @themightychris
- fix(web): reorder header, pad mobile sheet, repoint dead GitBook links [#154] @heyoub
- fix(web): repair invalid ARIA and make search/tag pickers real comboboxes [#155] @heyoub
- fix(web): mechanical a11y fixes — breadcrumbs, headings, names, announcements [#157] @heyoub |
Postmark is what the legacy site already sends through, with the
codeforphilly.org sender domain verified there; Resend was an
unreviewed choice. Generated by:
npm install -w apps/api postmark
npm uninstall -w apps/api resend
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Postmark is the provider the legacy site already sends through, and the codeforphilly.org sender domain is verified there; Resend was a provider choice nobody reviewed. The notifier contract itself (LoggingNotifier fallback when unconfigured, log-not-throw delivery, CFP_NOTIFICATION_FROM / CFP_SITE_HOST) is unchanged. Env surface becomes POSTMARK_SERVER_TOKEN (secret, optional) plus POSTMARK_MESSAGE_STREAM (ConfigMap, default `outbound`). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Replace the Resend-backed EmailNotifier with a provider-neutral seam.
EmailNotifier now depends on a one-method `EmailTransport`
(`transport.ts`), and `PostmarkTransport` is the only file that knows
a vendor: it maps our from/to/subject/text/html onto Postmark's
PascalCase Message and tags the configured MessageStream. The notifier
contract is unchanged — LoggingNotifier fallback when unconfigured,
log-and-`delivered:false` on any failure, same templates, same
CFP_NOTIFICATION_FROM / CFP_SITE_HOST inputs.
Postmark's SDK throws on every non-2xx rather than resolving with an
`{ error }` envelope, so the notifier's four copy-pasted send blocks
collapse into one `#deliver` with a single catch; the logged `err`
carries Postmark's code/statusCode so operators can still tell a
rejected sender from a network blip.
Env: `RESEND_API_KEY` -> `POSTMARK_SERVER_TOKEN` (optional), plus
`POSTMARK_MESSAGE_STREAM` (default `outbound`). The cutover-mailout
script drops its hand-rolled fetch call and reuses PostmarkTransport.
Tests: the notifier suite stubs the transport seam directly; a new
postmark-transport suite covers the field mapping with a stub and the
real ServerClient against an MSW intercept of POST /email, so an SDK
wire-format change surfaces in CI instead of production.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
feat(api): send notifier email through Postmark
The hot-reload section described the rebuild as "mutate the live Maps in place" without saying which Maps. The implementation had quietly skipped three secondary indices (legacy-id, buzz-by-slug, slug-history), which is exactly the gap an unqualified sentence leaves open. State the invariant explicitly: every collection on the live state, primary and secondary, is replaced from the fresh one, so no lookup path can serve pre-reload contents after a reload. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
swapInPlace named each Map on InMemoryState by hand and skipped three secondary indices: projectIdByLegacyId, buzzIdBySlug, and slugHistory. After POST /api/_internal/reload-data those three still described the pre-reload state. Because the laddr importer mints fresh UUIDv7 ids on every run, a re-import merged into `published` followed by a hot reload left projectIdByLegacyId pointing at project ids that no longer existed, so legacy /projects?ID=<n> redirects fell through to the SPA until the pod restarted; /project-buzz/<slug> and slug-history 301s went stale the same way. Enumerate the fresh state's own properties instead of maintaining a list, and throw if a property is ever not a Map so a future field is handled deliberately rather than skipped again. Guards against recurrence: - reload-swap.test.ts builds two states with one record of every entity type (different ids, same legacy ids), swaps, and asserts every own property of the fresh state was replaced while Map identities are kept. Fails on the old code for all three missing indices. - internal-reload.test.ts gains a re-import scenario through the real webhook: the project and buzz are replaced with fresh ids and slugs plus a slug-history record, and the legacy-id, buzz-slug, and old-slug redirects must all land on the new slug afterwards. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
fix(api): rebuild every in-memory index on hot reload
The live IdP metadata advertises entityID and SingleSignOnService Locations on the Slack team host because the route derived both from SLACK_TEAM_HOST. Spec now separates the three values by source: the entity ID (metadata entityID + assertion Issuer) is a stable logical identifier from a new optional SAML_ENTITY_ID env var, defaulting to https://codeforphilly.org/api/saml/slack/metadata and deliberately independent of CFP_SITE_HOST so the pre-/post-cutover host flip doesn't invalidate the issuer Slack stored at setup; endpoint Locations follow CFP_SITE_HOST; SLACK_TEAM_HOST keeps only its Slack-side roles. Env tables in architecture.md, deploy.md, secrets.md and .env.example gain the new var. Adds plans/saml-self-host.md (in-progress) to carry the code change, closing the follow-up left open by plans/saml-idp.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
getSamlContext derived the IdP entity ID and both SingleSignOnService Locations from SLACK_TEAM_HOST, so the live metadata advertised https://codeforphilly.slack.com/api/saml/slack/... — Slack's host, not ours. Per specs/api/saml.md#idp-identity-and-hosts: - entityId (metadata entityID + assertion Issuer, one value flowing through SlackSamlEntities.entityId → issuerEntityId) now comes from the new SAML_ENTITY_ID env var, defaulting to https://codeforphilly.org/api/saml/slack/metadata. It is a stable logical identifier and intentionally does not follow CFP_SITE_HOST, so the next.codeforphilly.org → codeforphilly.org flip at cutover leaves the issuer Slack stored at setup untouched. - ssoLoginPostUrl / ssoLoginRedirectUrl are built on CFP_SITE_HOST so the metadata points Slack at the host actually serving the API. - SLACK_TEAM_HOST keeps only its Slack-side roles (ACS URL, NameID NameQualifier, launch redirect). Tests assert the default entityID, the CFP_SITE_HOST-driven Locations, Issuer == entityID on both Response and Assertion, that CFP_SITE_HOST=next.example.org moves the Locations without moving the entityID, and that an explicit SAML_ENTITY_ID flows to both. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
fix(api): build SAML entityID and SSO endpoints from our own host, not Slack's
Issue #153 walks the live site on desktop and mobile and collects five findings. Four are shippable together because they all land in the app shell or in one screen's outbound links; the fifth (replace the Home hero CTA with a mailing-list invite) has no mechanism to build against, so the plan records the block up front rather than inventing one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Specs lead, so both files change before the code does. app-shell: the header's single "primary nav" table conflated content navigation with utilities, which is why the Volunteer CTA ended up buried between Members and About. Splitting it into a content cluster and a right-pinned utility cluster makes the CTA's position a stated rule rather than an accident, and gives the GitHub link and the auth control a declared home. Also states the sheet's accessible name and the icon-only-controls labelling rule, both of which the header violated. volunteer: the whole codeforphilly.gitbook.io space now returns 404 "Content owner not found", so the spec was prescribing two dead targets. The Meetup group and the CodeForPhilly/partnerships first-steps doc are the live equivalents; both verified reachable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Site check (#153) found the Volunteer button buried mid-nav between Members and About, where it read as one more section link rather than the call to action. It now closes the utility cluster, after the auth control, per the reordered app-shell spec. About joins the content links; a GitHub icon link is added to both breakpoints. The mobile sheet had no horizontal padding at all: SheetContent's base classes carry none and the only override here was a pt-8 hack, so nav items and the search box sat flush against the panel edge. Replaced with the structure shadcn intends — SheetHeader + SheetTitle, which bring their own p-4 — plus explicit px-4 on the nav and search. SheetTitle also gives the underlying Radix dialog the accessible name it never had. Three ARIA defects fixed while the file was open: aria-label on a roleless skeleton div (prohibited; now aria-hidden), a hand-written aria-expanded duplicating what Dialog.Trigger already supplies, and an aria-label overriding the About trigger's own visible text. The account-menu label stays — below sm the person's name is display:none, so it is the only accessible name there. Per-child ml-1 margins are gone; the parent gap-2 is now the single source of spacing at the same effective density. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The whole codeforphilly.gitbook.io space now returns 404 "Content owner not found", so both of the Volunteer screen's outbound CTAs were dead. PR #128 fixed the Home screen's copy of the same URL; these are the last two in the SPA. "When we meet" goes to the Meetup group, which is where hack nights are actually announced and which the footer already links to; "Read the guide" goes to the partnerships repo's first-steps doc, the surviving source of the GitBook page it replaces. The footer's "view this site on GitHub" link still named the repo codeforphilly-rewrite. That only resolves through GitHub's rename redirect, which is not something to depend on indefinitely. New Volunteer test asserts both hrefs and, following the Home dead-link idiom, that no gitbook.io URL survives anywhere in the rendered screen — so a copy-paste of the old constant cannot come back unnoticed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sheet is a full-height flex column with no scroll container, so a nav list taller than the viewport was simply unreachable below the fold. That was already latent; adding the GitHub row makes it one row likelier on short phones. min-h-0 lets the flex child shrink at all, and overflow-y-auto gives it somewhere to put the excess. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ticks the criteria verified during implementation and, deliberately, leaves two unticked rather than rewriting them to match what was achievable: the browser pass belongs to whoever does UI QA, and the all-workspaces test gate cannot close on this Windows dev box. Notes record why. apps/api fails ten tests here on a tree whose API code is byte-identical to develop, because the fixtures assume POSIX — store.test.ts injects a write failure via /dev/null/impossible-path, which Windows will happily create, so the expected rejection never comes. Reproduces with the files run alone, so it is not runner contention. Filed as a follow-up rather than fixed: cross-platform fixtures are their own scope, and silently ticking a gate that did not run is worse than leaving the box open. Also flags HomeStub.tsx, which carries the same stale repo URL the footer had but is imported by nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins the numbers behind the unticked all-workspaces gate so a future reader can tell a known Windows baseline from a real regression, and notes the develop re-run that confirms it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Desktop header verified in headed Chrome at 1400px; the sheet verified via its portal at desktop width because the automation harness could not shrink a maximized window below md — same limitation web-shell.md's plan recorded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each of these either lies to assistive tech or withholds state that sighted users get for free. Grouped into one commit because they are all one-line attribute repairs with no shared behavior change. Prohibited attributes on roleless elements (aria-label needs a role to attach to, so today it is simply dropped): PersonAvatar's initials span gains role="img"; StageProgressBar's wrapper loses its aria-label in favour of real role="progressbar" semantics on the bar that actually encodes the percentage. Announcements that never fire: the LoginPlaceholder and AccountClaim spinners put aria-live + aria-label on an empty roleless div, which announces nothing — now role="status" with sr-only text and the spinner hidden. TopProgressBar was permanently exposed (it only fades via opacity), so every page read out a finished "Page loading" bar; it is hidden from AT while idle. Names that fight their labels: NetworkErrorBanner's button reads "Retry" but was named "Dismiss error" (SC 2.5.3), and ConnectGitHubBanner duplicated its own visible "Dismiss" text. Both aria-labels go. Pagination's page buttons were named only "3"; the filter chips on ProjectsIndex and HelpWantedIndex never said they remove the filter. State conveyed by styling alone: TagChip and the Home activity filters get aria-pressed, matching StageFilterRow. Also: StageBadge's tooltip triggers were non-focusable, so the stage description was hover-only; the sessions table's <th>s had no scope; the skip link — the one control that exists purely for keyboard users — ended its class list with focus:outline-none; and ManageMembersModal's inline role field was labelled only by its placeholder. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
aria-invalid was set on several controls but the error <p> beneath was never
referenced, so a screen-reader user heard "invalid" with no way to reach the
reason — the one piece of information the error exists to convey. Every
audited error message now carries an ${id}-error id and its control an
aria-describedby, applied the same way everywhere: conditional, present only
while the error is.
Fields covered: AddMemberModal, ProjectEdit (5), ProjectBuzzNew (4),
PostHelpWantedModal, TagEditModal (2), ProfileEdit (2) and MarkdownEditor.
Where a field showed an error without aria-invalid, that is set too so the
pair stays consistent.
Two related labelling fixes on the same screens. ProjectEdit's debounced slug
check ("Checking…" / "✓ Available" / "✗ Taken") was never announced, so a
non-sighted author could submit a slug already known to be taken; it becomes a
role="status" the input describes. ProfileEdit's "Avatar" Label pointed at
nothing and its file input had no id — the wrapping <label> is now a div so it
cannot compete for the accessible name.
MarkdownEditor also drops aria-live from its preview pane: the preview is the
whole document re-rendered on every debounce, so the live region read the
entire text back on each pause in typing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixing the Label-in-Name violation on NetworkErrorBanner turned up a deeper disagreement: the button says "Retry", app-shell.md prescribes "[Retry]", and the handler only calls clearError(). Spec and label agree; the code does not. That is a behavior decision, not an ARIA repair, so it does not belong in this plan's scope — but it should not evaporate either, and the aria-label removed here was the only remaining trace of what the button really does. Record it as a follow-up needing its own spec decision so nobody later "fixes" it by quietly renaming whichever side is easiest to reach. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With showLabel the nested StageBadge is already a focusable trigger for the same tooltip, so the wrapper's tabIndex added a second, redundant stop right next to it. The wrapper now joins the tab order only in the bar-only variant, where it is the sole way to reach the stage description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The header search is now an ARIA combobox (PR #155), so Enter no longer unconditionally submits: with a highlighted result it navigates there, and the arrow keys move the highlight. Record that so the spec and the widget agree. The 5xx banner's Retry button had only ever dismissed; the spec, the label and the code disagreed. Retry now re-fetches active queries, and when the failing call has nothing to re-issue the button reads Dismiss, so the visible label is always accurate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Review follow-ups on the combobox rewrite: - Drop the Home/End branches. In an editable combobox those keys move the text caret (APG); hijacking them for the listbox surprised anyone editing the query. - activate() now blurs the input, so focus does not stay parked in an emptied combobox after choosing a result. The Enter fallback reuses seeAllUrl + activate() instead of repeating the same four steps. - Options track the pointer with a guarded onMouseMove instead of onMouseEnter: results arriving under a stationary pointer would otherwise steal the highlight from a keyboard user. - The inline (mobile-sheet) instance renders its results in-flow with a smaller cap rather than absolutely positioned, so on short phones the list can no longer hang below the sheet's fixed-height viewport. AppHeader's test queries the sheet search as a combobox now, and gains a case for clicking a result closing the sheet (via #154's location-key mechanism). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Two interaction gaps in the combobox rewrite:
- After a mouse selection the input kept focus (the listbox swallowed
mousedown), so onFocus could never fire again and clicking the input
a second time did nothing. The input now opens on click as well.
- Tab-out never closed the list; only a document-level mousedown did.
Replace that listener with an onBlur on the container that closes when
relatedTarget is outside it. Options and the listbox get tabIndex={-1}
so a click inside keeps focus within the container, which also lets
the mousedown swallow go (it blocked scrollbar dragging in Firefox).
selectOption refocuses the input so the next tag can be typed.
Also: options highlight on guarded onMouseMove rather than onMouseEnter
(async option arrival under a still pointer no longer steals the
keyboard highlight), and the repeated setOpen(false)/setActiveIndex(-1)
pair is a close() helper. Two tests cover the mouse-select/reopen and
focus-out paths.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
- TagChip emitted aria-pressed on every onClick chip, so the filter
removal chips announced as "toggle button, not pressed". Only emit it
when a caller actually passes `active`.
- StageBadge and StageProgressBar had gained tabIndex={0} to make the
tooltip keyboard-reachable, which added a roleless focus stop per
project card and opened two nested tooltips at once on the detail
page. Expose the description non-visually instead: an sr-only span in
the badge, and aria-describedby on the progressbar. No focus stop.
- PersonAvatar's inner span is role="img" with the person's name, so the
wrapping Link's aria-label read the name twice. Let the link take its
name from content.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
The 5xx banner's only button said "Retry" but just dismissed. PR #155 removed the aria-label that admitted as much, leaving a button whose name and behaviour still disagreed. showError now takes an optional retry callback. The query client passes one that re-fetches every active query, so Retry re-issues whatever is on screen and then clears the banner. The header typeahead reports errors with no callback (it re-runs on the next keystroke), and in that case the button reads "Dismiss" — the visible label always says what the button does, per the amended specs/behaviors/app-shell.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Several screen tests (ProjectEdit, ExpressInterestModal, …) pass in isolation but exceed vitest's 5 s default when the whole web suite runs under CI-like load. The individual 20 s overrides in the new combobox tests were papering over the same thing; a suite-level 15 s is the honest number. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Check the validation boxes actually verified (new combobox tests, the audited aria-label/aria-invalid sets, and the full gate: type-check, lint, api 434/434, web 114/114, shared 75/75), fill in the closeout Notes (mousedown-swallow vs focus-out decision, hardcoded error ids kept, the post-#154 review pass, and that the mobile-sheet layout was reasoned rather than screenshotted), and convert the prose Follow-ups into taxonomy shapes: #164 (useId-derived form error ids) and #165 (shared combobox hook) filed and linked; the Retry contradiction is resolved in this PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
fix(web): repair invalid ARIA and make search/tag pickers real comboboxes
The accessibility audit's third bucket — findings with one obviously-correct fix each and no design decision attached. Recording it as a plan before touching code so the DAG carries the scope, the PR stack it sits on (#154 + #155), and its relationship to issue #156, which holds the design-decision findings that are deliberately NOT implemented here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
specs/behaviors/app-shell.md carries a table of exact breadcrumb trails, and Breadcrumbs.tsx already implements them correctly — nav[aria-label], an ordered list, aria-current on the last crumb. Nothing ever imported it, so every trail in that table was spec-only. This is code brought into conformance with a spec that has not moved. Placed as a sibling above each screen's content container rather than inside it: the component supplies its own `container mx-auto px-4`, which only lands correctly as a direct child of <main> — nesting it would double the gutter. No specs/screens/*.md mentions breadcrumbs, so app-shell.md is the sole authority and there is nothing to reconcile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A screen reader's heading outline is a navigation aid, and three index screens broke it by jumping h1 -> h3: the card components render h3, which is correct where those cards sit under a section h2 (TagDetail, Home, Volunteer) but leaves a gap on the index screens. Rather than change a shared card and break it in its other homes, each index gains an sr-only h2 over its results region — the grid genuinely is a section. ProjectsIndex was checked and does not have the defect; ProjectCard already renders h2. The detail-screen aside headings go h3 -> h2 directly. They sit under the screen h1 with nothing between, are used nowhere else, and keep their classes so nothing moves: heading level and visual size are independent. PersonCard wrapped the entire card in one <a>, so its accessible name was the avatar title, the name, the project count and every tag chip concatenated into one string. Restructured to the ProjectCard idiom with a stretched pseudo-element so the whole card stays clickable. The header's navs rendered bare links; a nav without a list does not tell you how many destinations it has. Both are now ul/li matching AppFooter, with the mobile sheet's three groups as three lists so the separators and the About heading are not list children. That "About" label was a styled <p>; it is now an h3, one level under the SheetTitle that Radix renders as an h2. HelpWantedIndex wrapped FacetSidebar — which renders its own labelled aside — in a second bare <aside>, nesting two complementary landmarks with the outer one unnamed. The outer element is now a div. Result-count badges move out of the h1 on all three index screens: a heading whose accessible name changes on every keystroke is not a stable landmark. The two GitHub links in this file also pick up their new-tab cues here rather than splitting one file's edits across two commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The six formatting controls were an unlabelled div of buttons named "B", "I", "Link" — meaningless out of visual context — and six consecutive tab stops between the label and the textarea. They are now a labelled toolbar with one tab stop and arrow-key movement (ARIA APG). Each accessible name is a superset of its visible label so speech input still works. Four places changed state with nothing announced: - ProjectDetail's "Copy link" and "Share to Slack" gave no feedback at all, to anyone — the clipboard write was the entire interaction. They now raise a sonner toast, which the modals this screen already renders use for the same purpose, and surface a failed clipboard write instead of swallowing it. - Sponsor's "Copy email" signals success by renaming itself, and a control's own name changing is not announced. An sr-only live region mirrors it. - ProfileEdit's "Uploading…" appeared and vanished silently; it is now a status region that persists across both states so it can announce. - ConnectGitHubBanner was role="region", which is a landmark: it is only reachable by going looking for it. The banner renders after auth resolves, i.e. after first paint, so it needs role="status" to be heard at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three unrelated-looking defects with the same root cause: information that is obvious on screen but absent from the accessibility tree. Repeated button names. Tabbing a member list, a session table, the claim queue or a project's open roles produced "Remove, Remove, Remove" with no way to tell which row you were on — the row context lived only in visual adjacency. Each button now carries an aria-label naming its subject, with the visible text kept as a substring so speech input still reaches it (SC 2.5.3). The claim queue's labels stay fixed while a request is in flight and its buttons read "Working…"; the busy state is transient and the name should not move under a user mid-interaction. Dates. A relative string like "3 months ago" inside a title-only span is imprecise for everyone and the title is unreachable by touch and by most screen readers. These become <time dateTime> carrying the ISO instant, the idiom BlogIndex already uses, with title kept as a sighted-mouse bonus. ProjectCard's wrapper title duplicated what PersonAvatar already emits, so it is deleted rather than converted. New tabs. Every target="_blank" link now says so — an sr-only span where there is visible text, appended to the aria-label where there is not. Losing your place because a link silently opened elsewhere is a bigger problem for a screen-reader or magnifier user than for anyone else. Also: the "More ▾" menu trigger reads as an actual action list, and the stage-explainer button declares aria-haspopup="dialog" so it is not mistaken for navigation. HomeStub.tsx is skipped — nothing imports it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regression cover for the parts of this branch that are easy to undo by accident. The breadcrumb tests assert the exact trails app-shell.md prescribes, including that the final crumb is text with aria-current rather than a link — the detail that makes a trail a trail. The Revoke test asserts that two rows produce two distinct accessible names, which is the property that actually broke, rather than asserting one label's spelling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both gate runs green; breadcrumbs, copy toast, badge placement, and the PersonCard click affordance verified in headed Chrome against a seeded dev data repo. Toolbar keyboard nav rides on the jsdom coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wrapping the sheet's NavLinks in <li> left the anchors inline, so each row's tap target shrank to the width of its text. `block` on navLinkClass and on the two plain <a>s makes the rows full-width again. Swapping HelpWantedIndex's outer <aside> for a <div> removed the nested landmark but orphaned the Commitment heading and fieldset outside any landmark, since FacetSidebar renders its own aside[aria-label="Filters"] as a sibling. FacetSidebar now accepts children inside that aside, and Commitment rides there — still exactly one complementary landmark, and it holds every filter control. PeopleIndex and ProjectsIndex mount FacetSidebar directly and are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Turning the banner into role="status" traded a landmark for a live region that mounts late — and a container that appears already populated is not announced reliably, while wrapping the two buttons in a status role is invalid content for it. Revert to region + aria-label and add a sibling sr-only role="status" span carrying the headline, the same idiom Sponsor and ProfileEdit use in this PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Breadcrumbs keyed each <li> by label, which collides when a
user-authored title matches an ancestor crumb; key by index instead.
TagDetail's namespace crumb showed the raw slug ("tech") while the page
it links to is headed "Tech"; it now uses TagsNamespace's NS_LABELS so
the crumb and the destination h1 agree.
ProjectEdit rendered a blank crumb linking to /projects/ when the edit
query settled without a record; hold the loading state in that case so
the trail is only ever built from a loaded project.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
navigator.clipboard is undefined outside secure contexts and reading .writeText off it throws synchronously, before a .catch() could run, so the failure toast never fired there. One copyWithToast helper guards it and replaces the two duplicated promise chains on ProjectDetail. ProfileEdit's always-mounted status span reserved mt-1 even while empty; the margin now applies only when it has text. Three relative timestamps were still title-only: HelpWantedCard's "posted", PersonDetail's "joined" and its recent-update dates. They get the same <time dateTime title> treatment as the rest of the sweep. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
handleToolbarKeyDown read `activeButton` from its closure, which lags a focus change by a render cycle. Take the index from the button the key landed on instead; `activeButton` stays as render state for tabIndex. The name-superset test compared two literals from its own table; it now checks the rendered button's aria-label against its visible text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Fill in the Notes and Follow-ups that were left as placeholders, correct two sentences that no longer matched the code (crumbs are a fragment sibling above the content container, not its first child; the PersonCard hover lift stays on the article, there is no group-hover), describe the post-rebase state of the banner, landmark and toolbar items, and record the full validation gate across every workspace. Follow-ups filed as #166–#170 for the refactors the review surfaced but deliberately kept out of this PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
fix(web): mechanical a11y fixes — breadcrumbs, headings, names, announcements
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First release aimed at production: outbound email is live via Postmark, Slack SSO advertises the right issuer and endpoints, the header and mobile menu were reworked, search and tag pickers are real accessible comboboxes, and the data store runs on gitsheets 2.x.
Improvements
Technical