diff --git a/apps/web/src/components/ActivityCard.tsx b/apps/web/src/components/ActivityCard.tsx index 409c45f..3d65587 100644 --- a/apps/web/src/components/ActivityCard.tsx +++ b/apps/web/src/components/ActivityCard.tsx @@ -33,9 +33,13 @@ function UpdateCard({ update }: { update: ProjectUpdateResponse }) { Update #{update.number} - + + {update.author && ( @@ -69,9 +73,9 @@ function BuzzCard({ buzz }: { buzz: ProjectBuzzResponse }) { {buzz.project.title} · Buzz · - + + @@ -89,6 +93,7 @@ function BuzzCard({ buzz }: { buzz: ProjectBuzzResponse }) {

{buzz.headline} + (opens in new tab)

{hostname}

diff --git a/apps/web/src/components/AppFooter.tsx b/apps/web/src/components/AppFooter.tsx index 620b03f..b381912 100644 --- a/apps/web/src/components/AppFooter.tsx +++ b/apps/web/src/components/AppFooter.tsx @@ -182,7 +182,7 @@ export function AppFooter() { Open source — view this site on GitHub + (opens in new tab) diff --git a/apps/web/src/components/AppHeader.tsx b/apps/web/src/components/AppHeader.tsx index c0d6516..8f8eac7 100644 --- a/apps/web/src/components/AppHeader.tsx +++ b/apps/web/src/components/AppHeader.tsx @@ -8,11 +8,19 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from '@/components/ui/dropdown-menu'; -import { Sheet, SheetContent, SheetTrigger } from '@/components/ui/sheet'; +import { + Sheet, + SheetContent, + SheetHeader, + SheetTitle, + SheetTrigger, +} from '@/components/ui/sheet'; import { Separator } from '@/components/ui/separator'; import { SearchBox } from '@/components/SearchBox'; import { useAuth } from '@/hooks/useAuth'; +const GITHUB_URL = 'https://github.com/CodeForPhilly'; + function ChevronDownIcon() { return ( + ); +} + function AuthControls({ mobile = false }: { mobile?: boolean }) { const { person, loading, signOut } = useAuth(); @@ -60,7 +83,7 @@ function AuthControls({ mobile = false }: { mobile?: boolean }) { return ( diff --git a/apps/web/src/components/AppShell.tsx b/apps/web/src/components/AppShell.tsx index a7c6bd6..cb73a1c 100644 --- a/apps/web/src/components/AppShell.tsx +++ b/apps/web/src/components/AppShell.tsx @@ -11,7 +11,7 @@ export function AppShell() { {/* Skip to main content — must be the first focusable element */} Skip to main content diff --git a/apps/web/src/components/ConnectGitHubBanner.tsx b/apps/web/src/components/ConnectGitHubBanner.tsx index 1935971..efd1c2a 100644 --- a/apps/web/src/components/ConnectGitHubBanner.tsx +++ b/apps/web/src/components/ConnectGitHubBanner.tsx @@ -30,8 +30,11 @@ export function ConnectGitHubBanner() { if (dismissed) return null; return ( + // role="status", not "region": the banner appears only once auth has + // resolved, so it arrives after first paint and a landmark would never + // announce it. The aria-label stays as its accessible name.
@@ -53,7 +56,6 @@ export function ConnectGitHubBanner() { variant="ghost" size="sm" onClick={() => setDismissed(true)} - aria-label="Dismiss" > Dismiss diff --git a/apps/web/src/components/MarkdownEditor.tsx b/apps/web/src/components/MarkdownEditor.tsx index b1f8d2d..25eb0b0 100644 --- a/apps/web/src/components/MarkdownEditor.tsx +++ b/apps/web/src/components/MarkdownEditor.tsx @@ -1,4 +1,4 @@ -import { useEffect, useId, useRef, useState } from 'react'; +import { useEffect, useId, useRef, useState, type KeyboardEvent } from 'react'; import { Label } from '@/components/ui/label'; import { Textarea } from '@/components/ui/textarea'; import { Button } from '@/components/ui/button'; @@ -20,17 +20,23 @@ interface MarkdownEditorProps { interface ToolbarButton { label: string; + /** + * Accessible name. Each one *contains* the visible label so that a + * speech-input user saying what they see still hits the control + * (WCAG 2.5.3 Label in Name). + */ + name: string; insert: string; wrap?: { before: string; after: string }; } const TOOLBAR: ToolbarButton[] = [ - { label: 'B', insert: 'bold text', wrap: { before: '**', after: '**' } }, - { label: 'I', insert: 'italic text', wrap: { before: '_', after: '_' } }, - { label: 'Link', insert: 'link text', wrap: { before: '[', after: '](https://)' } }, - { label: 'List', insert: '- item' }, - { label: 'Code', insert: 'code', wrap: { before: '`', after: '`' } }, - { label: 'Quote', insert: '> quote' }, + { label: 'B', name: 'Bold', insert: 'bold text', wrap: { before: '**', after: '**' } }, + { label: 'I', name: 'Italic', insert: 'italic text', wrap: { before: '_', after: '_' } }, + { label: 'Link', name: 'Insert link', insert: 'link text', wrap: { before: '[', after: '](https://)' } }, + { label: 'List', name: 'Bulleted list', insert: '- item' }, + { label: 'Code', name: 'Code', insert: 'code', wrap: { before: '`', after: '`' } }, + { label: 'Quote', name: 'Quote', insert: '> quote' }, ]; /** @@ -53,7 +59,13 @@ export function MarkdownEditor({ required, }: MarkdownEditorProps) { const id = useId(); + const errorId = `${id}-error`; const textareaRef = useRef(null); + const toolbarRefs = useRef>([]); + // Roving tabindex: a toolbar is one tab stop, and the arrow keys move + // within it (ARIA APG Toolbar pattern). Without this the six formatting + // buttons sat between the label and the textarea as six separate tab stops. + const [activeButton, setActiveButton] = useState(0); const [previewHtml, setPreviewHtml] = useState(''); const [previewLoading, setPreviewLoading] = useState(false); const [previewError, setPreviewError] = useState(null); @@ -131,6 +143,28 @@ export function MarkdownEditor({ }); }; + const focusToolbarButton = (index: number) => { + setActiveButton(index); + toolbarRefs.current[index]?.focus(); + }; + + const handleToolbarKeyDown = (e: KeyboardEvent) => { + const last = TOOLBAR.length - 1; + if (e.key === 'ArrowRight') { + e.preventDefault(); + focusToolbarButton(activeButton === last ? 0 : activeButton + 1); + } else if (e.key === 'ArrowLeft') { + e.preventDefault(); + focusToolbarButton(activeButton === 0 ? last : activeButton - 1); + } else if (e.key === 'Home') { + e.preventDefault(); + focusToolbarButton(0); + } else if (e.key === 'End') { + e.preventDefault(); + focusToolbarButton(last); + } + }; + const count = value.length; const overSoftLimit = maxLength !== undefined && count > maxLength; @@ -146,14 +180,25 @@ export function MarkdownEditor({

{description}

)}
-
- {TOOLBAR.map((btn) => ( +
+ {TOOLBAR.map((btn, i) => (
diff --git a/apps/web/src/components/Pagination.tsx b/apps/web/src/components/Pagination.tsx index df54d9c..c666e7a 100644 --- a/apps/web/src/components/Pagination.tsx +++ b/apps/web/src/components/Pagination.tsx @@ -62,6 +62,7 @@ export function Pagination({ page, totalPages, onPageChange, siblingCount = 1, c variant={p === page ? 'default' : 'outline'} size="sm" onClick={() => onPageChange(p)} + aria-label={`Page ${p}`} aria-current={p === page ? 'page' : undefined} > {p} diff --git a/apps/web/src/components/PersonAvatar.tsx b/apps/web/src/components/PersonAvatar.tsx index fa74652..0d6cdab 100644 --- a/apps/web/src/components/PersonAvatar.tsx +++ b/apps/web/src/components/PersonAvatar.tsx @@ -24,6 +24,7 @@ export function PersonAvatar({ person, size = 32, asLink = true, className, titl /> ) : ( + // The whole card used to be one , so its accessible name was the + // avatar's title plus the name plus the project count plus every tag + // chip, read as one run-on string. Follow the ProjectCard idiom instead: + // an
whose heading wraps the only link, named by the person. + // A stretched pseudo-element keeps the entire card clickable — safe here + // because the avatar and chips are deliberately non-interactive. +
-

{person.fullName}

+

+ + {person.fullName} + +

{person.memberOfCount > 0 && (

Member of {person.memberOfCount} project{person.memberOfCount === 1 ? '' : 's'} @@ -29,6 +39,6 @@ export function PersonCard({ person }: PersonCardProps) {

)}
- + ); } diff --git a/apps/web/src/components/ProjectCard.tsx b/apps/web/src/components/ProjectCard.tsx index 3fd9b55..8c19e1a 100644 --- a/apps/web/src/components/ProjectCard.tsx +++ b/apps/web/src/components/ProjectCard.tsx @@ -34,7 +34,9 @@ export function ProjectCard({ project }: ProjectCardProps) { {project.members.slice(0, 8).map((m) => { const isMaintainer = m.slug === project.maintainer?.slug; return ( -
+ // No title here: PersonAvatar already emits the member's name, + // so this produced two identical tooltips stacked. +
); @@ -61,6 +63,7 @@ export function ProjectCard({ project }: ProjectCardProps) { )} @@ -68,6 +71,7 @@ export function ProjectCard({ project }: ProjectCardProps) { )} diff --git a/apps/web/src/components/SearchBox.tsx b/apps/web/src/components/SearchBox.tsx index 42eca78..553d076 100644 --- a/apps/web/src/components/SearchBox.tsx +++ b/apps/web/src/components/SearchBox.tsx @@ -1,7 +1,8 @@ -import { useCallback, useRef, useState } from 'react'; +import { useCallback, useId, useMemo, useRef, useState } from 'react'; import { useNavigate } from 'react-router'; import { Input } from '@/components/ui/input'; import { useSearch, type SearchResult } from '@/hooks/useSearch'; +import { cn } from '@/lib/utils'; interface SearchBoxProps { /** If true, renders compactly for embedding in the mobile sheet */ @@ -22,47 +23,138 @@ function groupResults(results: SearchResult[]): Array<{ type: SearchResult['type .map((t) => ({ type: t, items: groups[t] })); } +/** + * Site search — an ARIA APG combobox with a listbox popup. + * + * Focus never leaves the `role="combobox"` input; the active option is pointed + * at with `aria-activedescendant` instead of being focused. The popup swallows + * `mousedown`, so a pointer click on an option cannot blur the input — which is + * why there is no close-on-blur timeout here (the old 150ms one raced the click + * and made results unreachable). + * + * Options stay `` — `option` is an allowed role for `a[href]`, and the + * href keeps middle-click / "open in new tab" working. Plain clicks and Enter + * are intercepted and routed through `useNavigate()` so activation stays inside + * the SPA instead of triggering a full-page reload. + */ export function SearchBox({ inline = false }: SearchBoxProps) { const navigate = useNavigate(); const { query, results, loading, setQuery, clear } = useSearch(); const [open, setOpen] = useState(false); + const [activeIndex, setActiveIndex] = useState(-1); const inputRef = useRef(null); - const handleFocus = useCallback(() => { - setOpen(true); + const baseId = useId(); + const listboxId = `${baseId}-listbox`; + const optionId = (i: number) => `${baseId}-option-${i}`; + const groupHeaderId = (type: string) => `${baseId}-group-${type}`; + + const trimmed = query.trim(); + const showDropdown = open && trimmed.length > 0; + + const grouped = useMemo(() => groupResults(results), [results]); + const flat = useMemo(() => grouped.flatMap((g) => g.items), [grouped]); + const seeAllUrl = trimmed ? `/projects?q=${encodeURIComponent(trimmed)}` : null; + const optionUrls = useMemo( + () => [...flat.map((r) => r.url), ...(seeAllUrl ? [seeAllUrl] : [])], + [flat, seeAllUrl], + ); + + // Clamp instead of resetting from an effect: results land asynchronously and + // can shrink out from under the cursor mid-keystroke. + const activeIdx = activeIndex >= 0 && activeIndex < optionUrls.length ? activeIndex : -1; + const activeDescendant = showDropdown && activeIdx >= 0 ? optionId(activeIdx) : undefined; + + const close = useCallback(() => { + setOpen(false); + setActiveIndex(-1); }, []); - const handleBlur = useCallback(() => { - setTimeout(() => setOpen(false), 150); + const activate = useCallback( + (url: string) => { + void navigate(url); + clear(); + close(); + }, + [navigate, clear, close], + ); + + const handleFocus = useCallback(() => { + setOpen(true); }, []); const handleChange = useCallback( (e: React.ChangeEvent) => { setQuery(e.target.value); setOpen(true); + setActiveIndex(-1); }, [setQuery], ); const handleKeyDown = useCallback( (e: React.KeyboardEvent) => { - if (e.key === 'Enter' && query.trim()) { - void navigate(`/projects?q=${encodeURIComponent(query.trim())}`); - clear(); - setOpen(false); - inputRef.current?.blur(); + const len = optionUrls.length; + + if (e.key === 'ArrowDown') { + e.preventDefault(); + setOpen(true); + if (len > 0) setActiveIndex(activeIdx === -1 ? 0 : (activeIdx + 1) % len); + return; + } + if (e.key === 'ArrowUp') { + e.preventDefault(); + setOpen(true); + if (len > 0) setActiveIndex(activeIdx <= 0 ? len - 1 : activeIdx - 1); + return; + } + if (e.key === 'Home' && showDropdown && len > 0) { + e.preventDefault(); + setActiveIndex(0); + return; + } + if (e.key === 'End' && showDropdown && len > 0) { + e.preventDefault(); + setActiveIndex(len - 1); + return; + } + if (e.key === 'Enter') { + const target = showDropdown && activeIdx >= 0 ? optionUrls[activeIdx] : undefined; + if (target) { + e.preventDefault(); + activate(target); + } else if (trimmed) { + void navigate(`/projects?q=${encodeURIComponent(trimmed)}`); + clear(); + close(); + inputRef.current?.blur(); + } + return; } if (e.key === 'Escape') { clear(); - setOpen(false); + close(); inputRef.current?.blur(); } }, - [navigate, query, clear], + [optionUrls, activeIdx, showDropdown, trimmed, activate, navigate, clear, close], + ); + + /** Let the browser handle modified clicks (new tab / new window) natively. */ + const handleOptionClick = useCallback( + (e: React.MouseEvent, url: string) => { + if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0) return; + e.preventDefault(); + activate(url); + }, + [activate], ); - const showDropdown = open && query.trim().length > 0; - const grouped = groupResults(results); + const optionClass = (i: number) => + cn( + 'block px-3 py-2 text-sm hover:bg-accent hover:text-accent-foreground', + i === activeIdx && 'bg-accent text-accent-foreground', + ); return (
{showDropdown && ( )}
diff --git a/apps/web/src/components/StageBadge.tsx b/apps/web/src/components/StageBadge.tsx index bb103db..7d8a958 100644 --- a/apps/web/src/components/StageBadge.tsx +++ b/apps/web/src/components/StageBadge.tsx @@ -92,7 +92,10 @@ export function StageBadge({ stage, className }: StageBadgeProps) { return ( + {/* tabIndex makes the trigger focusable so the tooltip — which carries + the stage description — is reachable without a pointer. */} -
+ {/* When the badge is shown it is its own focusable tooltip trigger, so + the wrapper stays out of the tab order to avoid two adjacent stops + opening the same tooltip. */} +
diff --git a/apps/web/src/components/TagChip.tsx b/apps/web/src/components/TagChip.tsx index fcfa602..3e22f61 100644 --- a/apps/web/src/components/TagChip.tsx +++ b/apps/web/src/components/TagChip.tsx @@ -38,7 +38,7 @@ export function TagChip({ tag, count, showNamespace = false, active = false, asL if (onClick) { return ( - ); diff --git a/apps/web/src/components/TagPicker.tsx b/apps/web/src/components/TagPicker.tsx index 12edec8..494b300 100644 --- a/apps/web/src/components/TagPicker.tsx +++ b/apps/web/src/components/TagPicker.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useEffect, useId, useMemo, useRef, useState } from 'react'; import { useQuery } from '@tanstack/react-query'; import { Label } from '@/components/ui/label'; import { Input } from '@/components/ui/input'; @@ -18,7 +18,16 @@ interface TagPickerProps { description?: string; } -/** Tag picker — autocompletes against the existing tag space for `namespace`. */ +const CREATABLE_SLUG = /^[a-z0-9][a-z0-9-]{0,49}$/; + +/** + * Tag picker — autocompletes against the existing tag space for `namespace`. + * + * An ARIA APG combobox: focus stays on the `role="combobox"` input and the + * active `role="option"` is pointed at with `aria-activedescendant`, so the + * list is operable from the keyboard (arrows wrap, Enter selects, Escape + * closes) as `specs/behaviors/app-shell.md` requires of every dropdown. + */ export function TagPicker({ namespace, label, @@ -29,8 +38,14 @@ export function TagPicker({ }: TagPickerProps) { const [query, setQuery] = useState(''); const [open, setOpen] = useState(false); + const [activeIndex, setActiveIndex] = useState(-1); const containerRef = useRef(null); + const baseId = useId(); + const inputId = `${baseId}-input`; + const listboxId = `${baseId}-listbox`; + const optionId = (i: number) => `${baseId}-option-${i}`; + const tagsQ = useQuery({ queryKey: ['tag-picker', namespace], queryFn: () => api.tags.list({ namespace, perPage: 100 }), @@ -50,14 +65,23 @@ export function TagPicker({ .slice(0, 12); }, [allTags, query, value]); - const exactMatch = filtered.find( - (t) => t.slug.toLowerCase() === query.trim().toLowerCase(), + const trimmedQuery = query.trim().toLowerCase(); + const exactMatch = filtered.find((t) => t.slug.toLowerCase() === trimmedQuery); + const canCreate = Boolean( + allowCreate && trimmedQuery && !exactMatch && CREATABLE_SLUG.test(trimmedQuery), ); + const optionCount = filtered.length + (canCreate ? 1 : 0); + const showList = open && optionCount > 0; + // Clamp instead of resetting from an effect — the tag list loads async and + // filtering can shrink the option set out from under the cursor. + const activeIdx = activeIndex >= 0 && activeIndex < optionCount ? activeIndex : -1; + useEffect(() => { const handler = (e: MouseEvent) => { if (containerRef.current && !containerRef.current.contains(e.target as Node)) { setOpen(false); + setActiveIndex(-1); } }; document.addEventListener('mousedown', handler); @@ -68,6 +92,7 @@ export function TagPicker({ if (!value.includes(slug)) onChange([...value, slug]); setQuery(''); setOpen(false); + setActiveIndex(-1); }; const removeTag = (slug: string) => { @@ -79,26 +104,74 @@ export function TagPicker({ return found?.title ?? slug; }; + /** Activate the option at `i`: an existing tag, or the trailing create entry. */ + const selectOption = (i: number) => { + const tag = filtered[i]; + if (tag) { + addTag(tag.slug); + } else if (canCreate && i === filtered.length) { + addTag(trimmedQuery); + } + }; + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'ArrowDown') { + e.preventDefault(); + setOpen(true); + if (optionCount > 0) { + setActiveIndex(activeIdx === -1 ? 0 : (activeIdx + 1) % optionCount); + } + return; + } + if (e.key === 'ArrowUp') { + e.preventDefault(); + setOpen(true); + if (optionCount > 0) { + setActiveIndex(activeIdx <= 0 ? optionCount - 1 : activeIdx - 1); + } + return; + } + if (e.key === 'Escape') { + setOpen(false); + setActiveIndex(-1); + return; + } if (e.key === 'Enter') { e.preventDefault(); - const q = query.trim().toLowerCase(); - if (!q) return; + if (showList && activeIdx >= 0) { + selectOption(activeIdx); + return; + } + // No active option — fall back to the historical + // exact-match → first-match → create chain. + if (!trimmedQuery) return; if (exactMatch) { addTag(exactMatch.slug); } else if (filtered[0]) { addTag(filtered[0].slug); - } else if (allowCreate && /^[a-z0-9][a-z0-9-]{0,49}$/.test(q)) { - addTag(q); + } else if (allowCreate && CREATABLE_SLUG.test(trimmedQuery)) { + addTag(trimmedQuery); } - } else if (e.key === 'Backspace' && !query && value.length > 0) { + return; + } + if (e.key === 'Backspace' && !query && value.length > 0) { onChange(value.slice(0, -1)); } }; + const optionClass = (i: number) => + cn( + 'cursor-pointer px-3 py-1.5 text-sm hover:bg-accent', + i === activeIdx && 'bg-accent', + ); + return (
- {label && } + {label && ( + + )} {description && (

{description}

)} @@ -122,10 +195,20 @@ export function TagPicker({
= 0 ? optionId(activeIdx) : undefined + } onChange={(e) => { setQuery(e.target.value); setOpen(true); + setActiveIndex(-1); }} onFocus={() => setOpen(true)} onKeyDown={handleKeyDown} @@ -135,41 +218,43 @@ export function TagPicker({ : `Add ${namespace} tag — type to search…` } /> - {open && (filtered.length > 0 || (allowCreate && query.trim())) && ( + {showList && (
    e.preventDefault()} className="absolute z-20 mt-1 w-full max-h-56 overflow-auto rounded-md border border-border bg-popover shadow-md" > - {filtered.map((t: TagResponse) => ( -
  • - + {filtered.map((t: TagResponse, i) => ( +
  • selectOption(i)} + onMouseEnter={() => setActiveIndex(i)} + className={optionClass(i)} + > + {t.title}{' '} + + ({t.slug} · {t.projectCount} projects) +
    • ))} - {allowCreate && - query.trim() && - !exactMatch && - /^[a-z0-9][a-z0-9-]{0,49}$/.test(query.trim().toLowerCase()) && ( -
  • - -
    • - )} + {canCreate && ( +
  • selectOption(filtered.length)} + onMouseEnter={() => setActiveIndex(filtered.length)} + className={cn(optionClass(filtered.length), 'text-primary')} + > + Create new tag “{trimmedQuery}” +
    • + )}
)}
diff --git a/apps/web/src/components/TopProgressBar.tsx b/apps/web/src/components/TopProgressBar.tsx index cf3007e..f51f52f 100644 --- a/apps/web/src/components/TopProgressBar.tsx +++ b/apps/web/src/components/TopProgressBar.tsx @@ -15,6 +15,10 @@ export function TopProgressBar() { aria-valuenow={isNavigating ? 50 : 100} aria-valuemin={0} aria-valuemax={100} + // The bar is only faded out when idle, not unmounted (the fade needs the + // node to stay put). Hide it from AT meanwhile, or every page announces a + // finished "Page loading" bar. + aria-hidden={!isNavigating} className="fixed top-0 left-0 right-0 h-0.5 z-50 bg-primary transition-all duration-300" style={{ opacity: isNavigating ? 1 : 0, diff --git a/apps/web/src/components/modals/AddMemberModal.tsx b/apps/web/src/components/modals/AddMemberModal.tsx index adb99e7..ee9ff3c 100644 --- a/apps/web/src/components/modals/AddMemberModal.tsx +++ b/apps/web/src/components/modals/AddMemberModal.tsx @@ -84,9 +84,14 @@ export function AddMemberModal({ open, onOpenChange, projectSlug }: AddMemberMod placeholder="e.g. chris" required aria-invalid={fieldErrors['personSlug'] ? 'true' : 'false'} + aria-describedby={ + fieldErrors['personSlug'] ? 'member-slug-error' : undefined + } /> {fieldErrors['personSlug'] && ( -

{fieldErrors['personSlug']}

+

+ {fieldErrors['personSlug']} +

)}
diff --git a/apps/web/src/components/modals/ManageMembersModal.tsx b/apps/web/src/components/modals/ManageMembersModal.tsx index ca2f95b..2303b5b 100644 --- a/apps/web/src/components/modals/ManageMembersModal.tsx +++ b/apps/web/src/components/modals/ManageMembersModal.tsx @@ -113,6 +113,7 @@ export function ManageMembersModal({ open, onOpenChange, project }: ManageMember setEditingRole((r) => ({ ...r, [rowKey]: e.target.value })) } placeholder="Role" + aria-label="Role" className="h-7 mt-1 text-xs" /> ) : ( @@ -157,10 +158,14 @@ export function ManageMembersModal({ open, onOpenChange, project }: ManageMember ) : ( <> + {/* Every row's buttons read identically out of + context, so each name carries its member. The + visible text stays a substring (SC 2.5.3). */}
setTitle(e.target.value)} maxLength={80} required + aria-invalid={fieldErrors['title'] ? 'true' : 'false'} + aria-describedby={fieldErrors['title'] ? 'title-error' : undefined} /> {fieldErrors['title'] && ( -

{fieldErrors['title']}

+

+ {fieldErrors['title']} +

)}
) : ( @@ -97,9 +101,15 @@ export function TagEditModal({ open, onOpenChange, tag, mode }: TagEditModalProp onChange={(e) => setMergeInto(e.target.value)} placeholder="e.g. tech.flutter" required + aria-invalid={fieldErrors['mergeInto'] ? 'true' : 'false'} + aria-describedby={ + fieldErrors['mergeInto'] ? 'mergeInto-error' : undefined + } /> {fieldErrors['mergeInto'] && ( -

{fieldErrors['mergeInto']}

+

+ {fieldErrors['mergeInto']} +

)}
)} diff --git a/apps/web/src/index.css b/apps/web/src/index.css index 088889e..d08befc 100644 --- a/apps/web/src/index.css +++ b/apps/web/src/index.css @@ -135,7 +135,7 @@ nav[aria-label="Breadcrumb"], [data-testid="offline-banner"], [data-testid="network-error-banner"], - [id="search-results-dropdown"] { + [data-search-dropdown] { display: none !important; } } diff --git a/apps/web/src/pages/AccountClaim.tsx b/apps/web/src/pages/AccountClaim.tsx index be71480..a939b04 100644 --- a/apps/web/src/pages/AccountClaim.tsx +++ b/apps/web/src/pages/AccountClaim.tsx @@ -105,8 +105,12 @@ export function AccountClaim() { if (loading || authLoading) { return ( -
-
+
+ ); } @@ -172,11 +176,11 @@ export function AccountClaim() { -
- Last updated {formatRelativeTime(c.lastActiveAt)} +
+ Last updated{' '} +
{c.matchedEmail ? (
diff --git a/apps/web/src/pages/LoginPlaceholder.tsx b/apps/web/src/pages/LoginPlaceholder.tsx index 5b82177..1900e2c 100644 --- a/apps/web/src/pages/LoginPlaceholder.tsx +++ b/apps/web/src/pages/LoginPlaceholder.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState, type FormEvent } from 'react'; +import { useEffect, useId, useState, type FormEvent } from 'react'; import { Link, useNavigate, useSearchParams } from 'react-router'; import { Card, @@ -40,6 +40,7 @@ const ERROR_MESSAGES: Record = { className="underline hover:no-underline" > verify a primary email on GitHub + (opens in new tab) {' '} and ensure email visibility is enabled for our app. @@ -63,6 +64,7 @@ function GitHubIcon() { function WhyGitHub() { const [open, setOpen] = useState(false); + const panelId = useId(); return (
@@ -71,11 +73,15 @@ function WhyGitHub() { onClick={() => setOpen((v) => !v)} className="text-sm text-muted-foreground hover:text-foreground underline-offset-2 hover:underline" aria-expanded={open} + aria-controls={open ? panelId : undefined} > Why GitHub? {open && ( -
+
We chose GitHub as the sole identity provider for three reasons: (1) the civic-tech community already lives there, (2) it filters spam and scam accounts more effectively than email-only sign-ups, and (3) most @@ -87,6 +93,7 @@ function WhyGitHub() { className="underline hover:no-underline" > create a GitHub account + (opens in new tab) {' '} in under a minute.
@@ -114,8 +121,12 @@ export function LoginPlaceholder() { if (loading) { return ( -
-
+
+ ); } diff --git a/apps/web/src/pages/StaffAccountClaimQueue.tsx b/apps/web/src/pages/StaffAccountClaimQueue.tsx index e4410df..18ae752 100644 --- a/apps/web/src/pages/StaffAccountClaimQueue.tsx +++ b/apps/web/src/pages/StaffAccountClaimQueue.tsx @@ -119,12 +119,13 @@ export function StaffAccountClaimQueue() { )} {' · '} - {formatRelativeTime(item.submittedAt)} - + @@ -148,7 +149,10 @@ export function StaffAccountClaimQueue() { />
+ {/* The queue renders one card per request, so a bare + "Approve"/"Deny" repeats verbatim down the page. */} ) : ( @@ -255,10 +260,10 @@ export function Account() { - - - - + + + + @@ -266,8 +271,10 @@ export function Account() { -
DeviceIPIssuedStatusDeviceIPIssuedStatus
{parseUA(s.userAgent)} {s.ipAddress} - {formatRelativeTime(s.issuedAt)} + + {s.current ? ( @@ -279,6 +286,7 @@ export function Account() { type="button" size="sm" variant="outline" + aria-label={`Revoke session on ${parseUA(s.userAgent)}`} onClick={() => revokeSession(s.jti)} > Revoke @@ -373,5 +381,6 @@ export function Account() { + ); } diff --git a/apps/web/src/screens/BlogDetail.tsx b/apps/web/src/screens/BlogDetail.tsx index 8fcbb1c..c052af9 100644 --- a/apps/web/src/screens/BlogDetail.tsx +++ b/apps/web/src/screens/BlogDetail.tsx @@ -57,7 +57,9 @@ export function BlogDetail() { {showEdited && post.editedAt && ( <> · - Edited + )} diff --git a/apps/web/src/screens/HelpWantedIndex.tsx b/apps/web/src/screens/HelpWantedIndex.tsx index 0fd54ea..6755035 100644 --- a/apps/web/src/screens/HelpWantedIndex.tsx +++ b/apps/web/src/screens/HelpWantedIndex.tsx @@ -90,12 +90,14 @@ export function HelpWantedIndex() {
-

- Help Wanted + {/* The count is a sibling of the h1, not part of it: an accessible + name that mutates on every filter change is a moving target. */} +
+

Help Wanted

{totalItems} -

+

Concrete, time-boxed ways to contribute to Code for Philly projects.

@@ -107,7 +109,10 @@ export function HelpWantedIndex() {
-
+ {/* HelpWantedCard renders an h3 (it also sits under section h2s on + Home, TagDetail and Volunteer), so the results list needs its own + h2 or the page skips h1 → h3. Visually redundant, hence sr-only. */} +

Results

{hasActiveFilters && ( <> @@ -173,6 +182,7 @@ export function HelpWantedIndex() { p.delete('commitmentMax'); }) } + aria-label={`Remove filter: ≤ ${commitmentMax} hrs/week`} className="inline-flex items-center gap-1 rounded-full border border-border px-2.5 py-0.5 text-xs hover:bg-accent" > ≤ {commitmentMax} hrs/week × diff --git a/apps/web/src/screens/Home.tsx b/apps/web/src/screens/Home.tsx index 6b6b9eb..1b6e56e 100644 --- a/apps/web/src/screens/Home.tsx +++ b/apps/web/src/screens/Home.tsx @@ -156,6 +156,7 @@ export function Home() { key={f} type="button" onClick={() => setActivityFilter(f)} + aria-pressed={activityFilter === f} className={cn( 'text-xs font-medium px-3 py-1 rounded-full border transition-colors capitalize', activityFilter === f diff --git a/apps/web/src/screens/PeopleIndex.tsx b/apps/web/src/screens/PeopleIndex.tsx index 9930b1b..4de703c 100644 --- a/apps/web/src/screens/PeopleIndex.tsx +++ b/apps/web/src/screens/PeopleIndex.tsx @@ -103,12 +103,14 @@ export function PeopleIndex() { return (
-

- Members + {/* The count is a sibling of the h1, not part of it: an accessible + name that mutates on every filter change is a moving target. */} +
+

Members

{totalItems} -

+
+ {/* PersonCard renders an h3 (it also sits under section h2s on + TagDetail), so the results grid needs its own h2 or the page + skips h1 → h3. Visually redundant, hence sr-only. */} +

Results

{hasActiveFilters && ( diff --git a/apps/web/src/screens/PersonDetail.tsx b/apps/web/src/screens/PersonDetail.tsx index ebd8a68..6564737 100644 --- a/apps/web/src/screens/PersonDetail.tsx +++ b/apps/web/src/screens/PersonDetail.tsx @@ -12,6 +12,7 @@ import { DialogHeader, DialogTitle, } from '@/components/ui/dialog'; +import { Breadcrumbs } from '@/components/Breadcrumbs'; import { MarkdownView } from '@/components/MarkdownView'; import { StageBadge } from '@/components/StageBadge'; import { TagChip } from '@/components/TagChip'; @@ -103,6 +104,9 @@ export function PersonDetail() { }); return ( + <> + {/* specs/behaviors/app-shell.md → Breadcrumbs: Members › */} +
@@ -215,9 +219,9 @@ export function PersonDetail() {
+ ); } diff --git a/apps/web/src/screens/ProfileEdit.tsx b/apps/web/src/screens/ProfileEdit.tsx index 3e92c61..c1c007b 100644 --- a/apps/web/src/screens/ProfileEdit.tsx +++ b/apps/web/src/screens/ProfileEdit.tsx @@ -182,7 +182,9 @@ export function ProfileEdit() {
- +
{person.avatarUrl ? ( )} -
@@ -219,9 +228,13 @@ export function ProfileEdit() { value={form.fullName} onChange={(e) => setForm((f) => ({ ...f, fullName: e.target.value }))} required + aria-invalid={fieldErrors['fullName'] ? 'true' : 'false'} + aria-describedby={fieldErrors['fullName'] ? 'fullName-error' : undefined} /> {fieldErrors['fullName'] && ( -

{fieldErrors['fullName']}

+

+ {fieldErrors['fullName']} +

)}
@@ -260,10 +273,14 @@ export function ProfileEdit() { value={form.slug} onChange={(e) => setForm((f) => ({ ...f, slug: slugify(e.target.value) }))} pattern="^[a-z0-9][a-z0-9-_]{1,79}$" + aria-invalid={fieldErrors['slug'] ? 'true' : 'false'} + aria-describedby={fieldErrors['slug'] ? 'slug-error' : undefined} />

URL: /members/{form.slug}

{fieldErrors['slug'] && ( -

{fieldErrors['slug']}

+

+ {fieldErrors['slug']} +

)}
)} diff --git a/apps/web/src/screens/ProjectBuzzNew.tsx b/apps/web/src/screens/ProjectBuzzNew.tsx index cb4c3b5..6ed3c7f 100644 --- a/apps/web/src/screens/ProjectBuzzNew.tsx +++ b/apps/web/src/screens/ProjectBuzzNew.tsx @@ -117,9 +117,12 @@ export function ProjectBuzzNew() { required placeholder="The Inquirer praises Project X" aria-invalid={fieldErrors['headline'] ? 'true' : 'false'} + aria-describedby={fieldErrors['headline'] ? 'headline-error' : undefined} /> {fieldErrors['headline'] && ( -

{fieldErrors['headline']}

+

+ {fieldErrors['headline']} +

)}
@@ -135,9 +138,12 @@ export function ProjectBuzzNew() { required placeholder="https://www.inquirer.com/…" aria-invalid={fieldErrors['url'] ? 'true' : 'false'} + aria-describedby={fieldErrors['url'] ? 'url-error' : undefined} /> {fieldErrors['url'] && ( -

{fieldErrors['url']}

+

+ {fieldErrors['url']} +

)}

Must be HTTPS. Each URL can only be logged once per project. @@ -156,9 +162,14 @@ export function ProjectBuzzNew() { required max={todayIso()} aria-invalid={fieldErrors['publishedAt'] ? 'true' : 'false'} + aria-describedby={ + fieldErrors['publishedAt'] ? 'publishedAt-error' : undefined + } /> {fieldErrors['publishedAt'] && ( -

{fieldErrors['publishedAt']}

+

+ {fieldErrors['publishedAt']} +

)}
@@ -171,12 +182,16 @@ export function ProjectBuzzNew() { maxLength={2000} rows={4} placeholder="Optional excerpt or quote. Markdown supported." + aria-invalid={fieldErrors['summary'] ? 'true' : 'false'} + aria-describedby={fieldErrors['summary'] ? 'summary-error' : undefined} />

{form.summary.length} / 2000

{fieldErrors['summary'] && ( -

{fieldErrors['summary']}

+

+ {fieldErrors['summary']} +

)}
diff --git a/apps/web/src/screens/ProjectDetail.tsx b/apps/web/src/screens/ProjectDetail.tsx index 35d592e..cb269d9 100644 --- a/apps/web/src/screens/ProjectDetail.tsx +++ b/apps/web/src/screens/ProjectDetail.tsx @@ -1,6 +1,7 @@ import { useEffect, useMemo, useState } from 'react'; import { Link, useParams, useSearchParams } from 'react-router'; import { useQuery } from '@tanstack/react-query'; +import { toast } from 'sonner'; import { Button } from '@/components/ui/button'; import { DropdownMenu, @@ -17,6 +18,7 @@ import { DialogDescription, DialogFooter, } from '@/components/ui/dialog'; +import { Breadcrumbs } from '@/components/Breadcrumbs'; import { MarkdownView } from '@/components/MarkdownView'; import { StageProgressBar, StageBadge } from '@/components/StageBadge'; import { StageInfoDialog } from '@/components/StageInfoDialog'; @@ -187,6 +189,9 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { const allTags = [...project.tags.tech, ...project.tags.topic, ...project.tags.event]; return ( + <> + {/* specs/behaviors/app-shell.md → Breadcrumbs: Projects › */} + <Breadcrumbs items={[{ label: 'Projects', href: '/projects' }, { label: project.title }]} /> <div className="container mx-auto px-4 py-8"> {/* Soft-delete banner — staff only (project-detail.md) */} {showDeletedBanner && ( @@ -223,7 +228,11 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { perms.canDelete) && ( <DropdownMenu> <DropdownMenuTrigger asChild> - <Button variant="outline">More ▾</Button> + {/* "More ▾" says nothing about what it opens; the label + keeps the visible word so speech input still works. */} + <Button variant="outline" aria-label="More actions"> + More ▾ + </Button> </DropdownMenuTrigger> <DropdownMenuContent align="end"> {perms.canManageMembers && ( @@ -310,10 +319,13 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { {role.tags.topic.map((t) => <TagChip key={`topic.${t.slug}`} tag={t} />)} </div> <div className="flex items-center justify-end gap-2"> + {/* One row per open role, so these names repeat + verbatim unless they carry the role title. */} {role.permissions.canFill && ( <Button size="sm" variant="outline" + aria-label={`Mark filled: ${role.title}`} onClick={() => setFillRole(role)} > Mark filled @@ -323,6 +335,7 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { <Button size="sm" variant="ghost" + aria-label={`Close ${role.title}`} onClick={() => { if (!window.confirm(`Close "${role.title}" without filling?`)) return; api.helpWantedRole @@ -432,14 +445,15 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { {/* Project info */} <section> - <h3 className="text-sm font-semibold mb-3 text-muted-foreground uppercase tracking-wide"> + <h2 className="text-sm font-semibold mb-3 text-muted-foreground uppercase tracking-wide"> Project Info - </h3> + </h2> <div className="flex flex-col gap-2"> {project.links.usersUrl && ( <Button asChild> <a href={project.links.usersUrl} target="_blank" rel="noopener noreferrer"> Users' Site + <span className="sr-only"> (opens in new tab)</span> </a> </Button> )} @@ -447,6 +461,7 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { <Button asChild variant="outline"> <a href={project.links.developersUrl} target="_blank" rel="noopener noreferrer"> Developers' Site + <span className="sr-only"> (opens in new tab)</span> </a> </Button> )} @@ -464,9 +479,9 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { {project.memberships.length > 0 && ( <section> <div className="flex items-center justify-between mb-3"> - <h3 className="text-sm font-semibold text-muted-foreground uppercase tracking-wide"> + <h2 className="text-sm font-semibold text-muted-foreground uppercase tracking-wide"> Members ({project.counts.members}) - </h3> + </h2> {perms.canManageMembers && ( <Button size="sm" @@ -495,9 +510,9 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { {/* Tags */} {allTags.length > 0 && ( <section> - <h3 className="text-sm font-semibold mb-3 text-muted-foreground uppercase tracking-wide"> + <h2 className="text-sm font-semibold mb-3 text-muted-foreground uppercase tracking-wide"> Tags - </h3> + </h2> <div className="space-y-2"> {project.tags.tech.length > 0 && ( <div> @@ -535,14 +550,21 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { {/* Share */} <section> - <h3 className="text-sm font-semibold mb-3 text-muted-foreground uppercase tracking-wide"> + <h2 className="text-sm font-semibold mb-3 text-muted-foreground uppercase tracking-wide"> Share - </h3> + </h2> <div className="flex flex-col gap-2"> + {/* Both buttons used to copy silently — nothing changed on + screen, so nobody (sighted or not) could tell it worked. + sonner is what this screen's own modals already use for + action confirmations. */} <Button variant="outline" onClick={() => { - void navigator.clipboard.writeText(`https://codeforphilly.org/projects/${slug}`); + void navigator.clipboard + .writeText(`https://codeforphilly.org/projects/${slug}`) + .then(() => toast.success('Link copied')) + .catch(() => toast.error("Couldn't copy the link")); }} > Copy link @@ -553,9 +575,12 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { // Copy a pre-formatted Slack message. Spec calls this // out as either system-share or copy; copy works in every // browser context without a Web Share API gate. - void navigator.clipboard.writeText( - `Check out ${project.title} on Code for Philly: https://codeforphilly.org/projects/${slug}`, - ); + void navigator.clipboard + .writeText( + `Check out ${project.title} on Code for Philly: https://codeforphilly.org/projects/${slug}`, + ) + .then(() => toast.success('Slack message copied')) + .catch(() => toast.error("Couldn't copy the message")); }} > Share to Slack @@ -567,15 +592,15 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { <section className="text-sm text-muted-foreground space-y-1"> <p> <span className="font-medium text-foreground">Created:</span>{' '} - <span title={formatAbsoluteDate(project.createdAt)}> + <time dateTime={project.createdAt} title={formatAbsoluteDate(project.createdAt)}> {formatRelativeTime(project.createdAt)} - </span> + </time> </p> <p> <span className="font-medium text-foreground">Last updated:</span>{' '} - <span title={formatAbsoluteDate(project.updatedAt)}> + <time dateTime={project.updatedAt} title={formatAbsoluteDate(project.updatedAt)}> {formatRelativeTime(project.updatedAt)} - </span> + </time> </p> <p className="flex items-center gap-2"> <span className="font-medium text-foreground">Stage:</span> @@ -584,6 +609,7 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { <p> <button type="button" + aria-haspopup="dialog" onClick={() => setStageInfoOpen(true)} className="text-primary underline hover:no-underline" > @@ -602,6 +628,7 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { className="hover:text-foreground" > Edit on GitHub → + <span className="sr-only"> (opens in new tab)</span> </a> </section> )} @@ -678,5 +705,6 @@ export function ProjectDetail({ anchor }: ProjectDetailProps = {}) { </DialogContent> </Dialog> </div> + </> ); } diff --git a/apps/web/src/screens/ProjectEdit.tsx b/apps/web/src/screens/ProjectEdit.tsx index 0dc8fc6..e1fe1c0 100644 --- a/apps/web/src/screens/ProjectEdit.tsx +++ b/apps/web/src/screens/ProjectEdit.tsx @@ -13,6 +13,7 @@ import { SelectValue, } from '@/components/ui/select'; import { Checkbox } from '@/components/ui/checkbox'; +import { Breadcrumbs } from '@/components/Breadcrumbs'; import { MarkdownEditor } from '@/components/MarkdownEditor'; import { TagPicker } from '@/components/TagPicker'; import { STAGES, type Stage } from '@/components/StageBadge'; @@ -268,6 +269,20 @@ export function ProjectEdit({ mode }: ProjectEditProps) { : ''; return ( + <> + {/* specs/behaviors/app-shell.md → Breadcrumbs: + create → Projects › New project; edit → Projects › <title> › Edit */} + <Breadcrumbs + items={ + mode === 'create' + ? [{ label: 'Projects', href: '/projects' }, { label: 'New project' }] + : [ + { label: 'Projects', href: '/projects' }, + { label: project?.title ?? '', href: `/projects/${project?.slug ?? ''}` }, + { label: 'Edit' }, + ] + } + /> <div className="container mx-auto px-4 py-8 max-w-3xl"> <header className="flex items-center justify-between mb-6"> <h1 className="text-2xl font-bold"> @@ -295,9 +310,12 @@ export function ProjectEdit({ mode }: ProjectEditProps) { maxLength={200} required aria-invalid={fieldErrors['title'] ? 'true' : 'false'} + aria-describedby={fieldErrors['title'] ? 'title-error' : undefined} /> {fieldErrors['title'] && ( - <p className="text-xs text-destructive">{fieldErrors['title']}</p> + <p id="title-error" className="text-xs text-destructive"> + {fieldErrors['title']} + </p> )} </div> @@ -317,8 +335,16 @@ export function ProjectEdit({ mode }: ProjectEditProps) { pattern="^[a-z0-9][a-z0-9-_]{1,79}$" required className="flex-1" + aria-invalid={fieldErrors['slug'] ? 'true' : 'false'} + aria-describedby={ + fieldErrors['slug'] ? 'slug-status slug-error' : 'slug-status' + } /> + {/* role="status" so the debounced availability check is announced + — it is otherwise a purely visual ✓/✗ next to the field. */} <span + id="slug-status" + role="status" className={ slugAvailability === 'available' ? 'text-xs text-green-600' @@ -334,7 +360,9 @@ export function ProjectEdit({ mode }: ProjectEditProps) { URL: /projects/<strong>{form.slug || 'your-slug'}</strong> </p> {fieldErrors['slug'] && ( - <p className="text-xs text-destructive">{fieldErrors['slug']}</p> + <p id="slug-error" className="text-xs text-destructive"> + {fieldErrors['slug']} + </p> )} </div> )} @@ -393,9 +421,13 @@ export function ProjectEdit({ mode }: ProjectEditProps) { value={form.usersUrl} onChange={(e) => setForm((f) => ({ ...f, usersUrl: e.target.value }))} placeholder="https://" + aria-invalid={fieldErrors['usersUrl'] ? 'true' : 'false'} + aria-describedby={fieldErrors['usersUrl'] ? 'usersUrl-error' : undefined} /> {fieldErrors['usersUrl'] && ( - <p className="text-xs text-destructive">{fieldErrors['usersUrl']}</p> + <p id="usersUrl-error" className="text-xs text-destructive"> + {fieldErrors['usersUrl']} + </p> )} </div> <div className="space-y-1.5"> @@ -406,9 +438,15 @@ export function ProjectEdit({ mode }: ProjectEditProps) { value={form.developersUrl} onChange={(e) => setForm((f) => ({ ...f, developersUrl: e.target.value }))} placeholder="https://" + aria-invalid={fieldErrors['developersUrl'] ? 'true' : 'false'} + aria-describedby={ + fieldErrors['developersUrl'] ? 'developersUrl-error' : undefined + } /> {fieldErrors['developersUrl'] && ( - <p className="text-xs text-destructive">{fieldErrors['developersUrl']}</p> + <p id="developersUrl-error" className="text-xs text-destructive"> + {fieldErrors['developersUrl']} + </p> )} </div> </div> @@ -422,10 +460,16 @@ export function ProjectEdit({ mode }: ProjectEditProps) { value={form.chatChannel} onChange={(e) => setForm((f) => ({ ...f, chatChannel: e.target.value }))} placeholder="my-channel" + aria-invalid={fieldErrors['chatChannel'] ? 'true' : 'false'} + aria-describedby={ + fieldErrors['chatChannel'] ? 'chatChannel-error' : undefined + } /> </div> {fieldErrors['chatChannel'] && ( - <p className="text-xs text-destructive">{fieldErrors['chatChannel']}</p> + <p id="chatChannel-error" className="text-xs text-destructive"> + {fieldErrors['chatChannel']} + </p> )} </div> @@ -474,5 +518,6 @@ export function ProjectEdit({ mode }: ProjectEditProps) { )} </form> </div> + </> ); } diff --git a/apps/web/src/screens/ProjectsIndex.tsx b/apps/web/src/screens/ProjectsIndex.tsx index 805f407..8e6022e 100644 --- a/apps/web/src/screens/ProjectsIndex.tsx +++ b/apps/web/src/screens/ProjectsIndex.tsx @@ -133,13 +133,13 @@ export function ProjectsIndex() { <div className="container mx-auto px-4 py-8"> {/* Header */} <div className="flex items-start justify-between gap-4 mb-2"> - <div> - <h1 className="text-3xl font-bold flex items-center gap-3"> - Civic Projects Directory - <span className="inline-flex items-center rounded-full bg-muted text-muted-foreground px-2.5 py-0.5 text-sm"> - {totalItems} - </span> - </h1> + {/* The count is a sibling of the h1, not part of it: an accessible + name that mutates on every filter change is a moving target. */} + <div className="flex items-center gap-3"> + <h1 className="text-3xl font-bold">Civic Projects Directory</h1> + <span className="inline-flex items-center rounded-full bg-muted text-muted-foreground px-2.5 py-0.5 text-sm"> + {totalItems} + </span> </div> {person && ( <Button asChild> @@ -200,16 +200,20 @@ export function ProjectsIndex() { /> ); })} - {stages.map((s) => ( - <button - key={s} - type="button" - onClick={() => handleToggleStage(s)} - className="inline-flex items-center gap-1 rounded-full border border-border px-2.5 py-0.5 text-xs hover:bg-accent" - > - Stage: {STAGES[s as Stage]?.label ?? s} × - </button> - ))} + {stages.map((s) => { + const stageLabel = STAGES[s as Stage]?.label ?? s; + return ( + <button + key={s} + type="button" + onClick={() => handleToggleStage(s)} + aria-label={`Remove filter: Stage: ${stageLabel}`} + className="inline-flex items-center gap-1 rounded-full border border-border px-2.5 py-0.5 text-xs hover:bg-accent" + > + Stage: {stageLabel} × + </button> + ); + })} <button type="button" onClick={handleClearAll} diff --git a/apps/web/src/screens/Sponsor.tsx b/apps/web/src/screens/Sponsor.tsx index 49cd58c..c6f7557 100644 --- a/apps/web/src/screens/Sponsor.tsx +++ b/apps/web/src/screens/Sponsor.tsx @@ -106,6 +106,11 @@ export function Sponsor() { <Button variant="outline" size="sm" onClick={handleCopy}> {copied ? 'Copied ✓' : 'Copy email'} </Button> + {/* The label swap is the only success signal, and a control's own + name changing is not announced. Mirror it in a live region. */} + <span role="status" className="sr-only"> + {copied ? `${email} copied to clipboard` : ''} + </span> </div> </div> </section> diff --git a/apps/web/src/screens/TagDetail.tsx b/apps/web/src/screens/TagDetail.tsx index a3902c1..8ce9706 100644 --- a/apps/web/src/screens/TagDetail.tsx +++ b/apps/web/src/screens/TagDetail.tsx @@ -3,6 +3,7 @@ import { Link, useNavigate, useParams } from 'react-router'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { toast } from 'sonner'; import { Button } from '@/components/ui/button'; +import { Breadcrumbs } from '@/components/Breadcrumbs'; import { ProjectCard } from '@/components/ProjectCard'; import { PersonCard } from '@/components/PersonCard'; import { HelpWantedCard } from '@/components/HelpWantedCard'; @@ -102,6 +103,15 @@ export function TagDetail() { }; return ( + <> + {/* specs/behaviors/app-shell.md → Breadcrumbs: Tags › <namespace> › <title> */} + <Breadcrumbs + items={[ + { label: 'Tags', href: '/tags' }, + { label: tag.namespace, href: `/tags/${tag.namespace}` }, + { label: tag.title }, + ]} + /> <div className="container mx-auto px-4 py-8 space-y-10"> <header className="flex items-start justify-between gap-3"> <div> @@ -224,5 +234,6 @@ export function TagDetail() { </section> )} </div> + </> ); } diff --git a/apps/web/src/screens/Volunteer.tsx b/apps/web/src/screens/Volunteer.tsx index 8b58cf9..7f81437 100644 --- a/apps/web/src/screens/Volunteer.tsx +++ b/apps/web/src/screens/Volunteer.tsx @@ -5,10 +5,11 @@ import { HelpWantedCard } from '@/components/HelpWantedCard'; import { useAuth } from '@/hooks/useAuth'; import { api } from '@/lib/api'; -const HACK_NIGHT_URL = - 'https://codeforphilly.gitbook.io/projects/contributing-to-projects/hack-night-program-details'; +// The codeforphilly.gitbook.io space is gone (404 "Content owner not found"), +// so both of these point at the surviving live equivalents. +const MEETUP_URL = 'https://www.meetup.com/Code-for-Philly/'; const START_PROJECT_URL = - 'https://codeforphilly.gitbook.io/projects/creating-new-partnerships/first-steps'; + 'https://github.com/CodeForPhilly/partnerships/blob/master/creating-new-partnerships/first-steps.md'; export function Volunteer() { const { person } = useAuth(); @@ -69,8 +70,9 @@ export function Volunteer() { We meet weekly. Bring your laptop, or just yourself. </p> <Button asChild variant="outline" size="sm"> - <a href={HACK_NIGHT_URL} target="_blank" rel="noopener noreferrer"> + <a href={MEETUP_URL} target="_blank" rel="noopener noreferrer"> When we meet → + <span className="sr-only"> (opens in new tab)</span> </a> </Button> </div> @@ -143,6 +145,7 @@ export function Volunteer() { <Button asChild> <a href={START_PROJECT_URL} target="_blank" rel="noopener noreferrer"> Read the guide → + <span className="sr-only"> (opens in new tab)</span> </a> </Button> <Button asChild variant="outline"> diff --git a/apps/web/tests/Account.test.tsx b/apps/web/tests/Account.test.tsx index 774e8e5..be6d510 100644 --- a/apps/web/tests/Account.test.tsx +++ b/apps/web/tests/Account.test.tsx @@ -5,7 +5,7 @@ * by AppShell on every page), and is covered by its own test file. */ import { describe, expect, it, vi, afterEach } from 'vitest'; -import { screen, waitFor } from '@testing-library/react'; +import { screen, waitFor, within } from '@testing-library/react'; import { renderScreen, mockOk } from './test-utils.js'; import { Account } from '../src/screens/Account.js'; import { AuthProvider } from '../src/hooks/useAuth.js'; @@ -17,7 +17,24 @@ interface MeShape { lastLoginMethod: 'github' | 'legacy_password' | 'password_reset' | null; } -function mockApi(me: MeShape): void { +const SESSIONS = [ + { + jti: 'sess-1', + userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X) Chrome/120', + ipAddress: '203.0.113.1', + issuedAt: '2026-05-01T00:00:00Z', + current: false, + }, + { + jti: 'sess-2', + userAgent: 'Mozilla/5.0 (Windows NT 10.0) Firefox/121', + ipAddress: '203.0.113.2', + issuedAt: '2026-05-02T00:00:00Z', + current: false, + }, +]; + +function mockApi(me: MeShape, sessions: unknown[] = []): void { vi.spyOn(globalThis, 'fetch').mockImplementation(((input: string) => { if (input.startsWith('/api/auth/me')) { return Promise.resolve( @@ -29,7 +46,7 @@ function mockApi(me: MeShape): void { } if (input.startsWith('/api/auth/sessions')) { return Promise.resolve( - new Response(JSON.stringify(mockOk([])), { + new Response(JSON.stringify(mockOk(sessions)), { status: 200, headers: { 'content-type': 'application/json' }, }), @@ -105,3 +122,38 @@ describe('Account — Identity card', () => { ).toBe(0); }); }); + +describe('Account — accessibility structure', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('renders the "Settings" breadcrumb trail from app-shell.md', async () => { + mockApi(githubPerson); + render(); + const trail = await screen.findByRole('navigation', { name: 'Breadcrumb' }); + expect(within(trail).getByText('Settings')).toHaveAttribute('aria-current', 'page'); + }); + + it('names each Revoke button after its own session', async () => { + mockApi(githubPerson, SESSIONS); + render(); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Revoke session on Chrome on macOS' })) + .toBeInTheDocument(); + }); + expect( + screen.getByRole('button', { name: 'Revoke session on Firefox on Windows' }), + ).toBeInTheDocument(); + // The visible text is still "Revoke" on both (SC 2.5.3 keeps it a substring). + expect(screen.getAllByRole('button', { name: /^Revoke session on/ })).toHaveLength(2); + }); + + it('exposes session timestamps as machine-readable <time>', async () => { + mockApi(githubPerson, SESSIONS); + render(); + await waitFor(() => { + expect(document.querySelector('time[datetime="2026-05-01T00:00:00Z"]')).not.toBeNull(); + }); + }); +}); diff --git a/apps/web/tests/AppFooter.test.tsx b/apps/web/tests/AppFooter.test.tsx index 6b0eb49..8924baf 100644 --- a/apps/web/tests/AppFooter.test.tsx +++ b/apps/web/tests/AppFooter.test.tsx @@ -17,7 +17,7 @@ describe('AppFooter', () => { expect(link).toBeInTheDocument(); expect(link).toHaveAttribute( 'href', - 'https://github.com/CodeForPhilly/codeforphilly-rewrite', + 'https://github.com/CodeForPhilly/codeforphilly-ng', ); }); diff --git a/apps/web/tests/AppHeader.test.tsx b/apps/web/tests/AppHeader.test.tsx index 5b58ff4..4c9f877 100644 --- a/apps/web/tests/AppHeader.test.tsx +++ b/apps/web/tests/AppHeader.test.tsx @@ -1,5 +1,5 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; -import { screen, waitFor } from '@testing-library/react'; +import { screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderWithRouter } from './test-utils.js'; import { AppHeader } from '../src/components/AppHeader.js'; @@ -38,10 +38,27 @@ describe('AppHeader', () => { it('renders primary nav links', async () => { renderWithRouter(<Wrapped />); + const nav = screen.getByRole('navigation', { name: /primary navigation/i }); expect(screen.getByRole('link', { name: 'Projects' })).toBeInTheDocument(); expect(screen.getByRole('link', { name: 'Help Wanted' })).toBeInTheDocument(); expect(screen.getByRole('link', { name: 'Members' })).toBeInTheDocument(); - expect(screen.getByRole('link', { name: 'Volunteer' })).toBeInTheDocument(); + expect(within(nav).getByRole('button', { name: 'About' })).toBeInTheDocument(); + + // The Volunteer CTA lives in the utility cluster, not the content nav — + // it is the rightmost header element (specs/behaviors/app-shell.md). + const volunteer = screen.getByRole('link', { name: 'Volunteer' }); + expect(volunteer).toHaveAttribute('href', '/volunteer'); + expect(nav).not.toContainElement(volunteer); + }); + + it('renders the GitHub link in the utility cluster', async () => { + renderWithRouter(<Wrapped />); + const gh = screen.getByRole('link', { + name: 'Code for Philly on GitHub (opens in new tab)', + }); + expect(gh).toHaveAttribute('href', 'https://github.com/CodeForPhilly'); + expect(gh).toHaveAttribute('target', '_blank'); + expect(gh).toHaveAttribute('rel', 'noopener noreferrer'); }); it('shows Sign in button(s) when anonymous', async () => { @@ -59,7 +76,8 @@ describe('AppHeader', () => { const user = userEvent.setup(); renderWithRouter(<Wrapped />); - const aboutBtn = screen.getByRole('button', { name: /about menu/i }); + // The trigger's visible text is its accessible name — no aria-label. + const aboutBtn = screen.getByRole('button', { name: 'About' }); await user.click(aboutBtn); await waitFor(() => { @@ -75,6 +93,8 @@ describe('AppHeader', () => { const hamburger = screen.getByRole('button', { name: /open navigation menu/i }); expect(hamburger).toBeInTheDocument(); + // aria-expanded is supplied by Radix's Dialog.Trigger, not hand-written. + expect(hamburger).toHaveAttribute('aria-expanded', 'false'); // Open await user.click(hamburger); @@ -83,6 +103,7 @@ describe('AppHeader', () => { // Sheet content includes "Mobile navigation" aria-label expect(screen.getByRole('navigation', { name: /mobile navigation/i })).toBeInTheDocument(); }); + expect(hamburger).toHaveAttribute('aria-expanded', 'true'); // Close via Escape key await user.keyboard('{Escape}'); @@ -91,4 +112,50 @@ describe('AppHeader', () => { expect(screen.queryByRole('navigation', { name: /mobile navigation/i })).not.toBeInTheDocument(); }); }); + + it('gives the mobile sheet dialog an accessible name', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + + await user.click(screen.getByRole('button', { name: /open navigation menu/i })); + + const dialog = await screen.findByRole('dialog', { name: 'Menu' }); + expect(dialog).toBeInTheDocument(); + }); + + it('marks up both navs as lists', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + + const desktop = screen.getByRole('navigation', { name: /primary navigation/i }); + expect(within(desktop).getByRole('list')).toBeInTheDocument(); + // Projects, Help Wanted, Members, About + expect(within(desktop).getAllByRole('listitem')).toHaveLength(4); + + await user.click(screen.getByRole('button', { name: /open navigation menu/i })); + const mobile = await screen.findByRole('navigation', { name: /mobile navigation/i }); + // Three groups: primary, About, and the GitHub/Volunteer tail. + expect(within(mobile).getAllByRole('list')).toHaveLength(3); + expect( + within(mobile).getByRole('heading', { name: 'About', level: 3 }), + ).toBeInTheDocument(); + }); + + it('lists GitHub and Volunteer in the mobile sheet', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + + await user.click(screen.getByRole('button', { name: /open navigation menu/i })); + + const nav = await screen.findByRole('navigation', { name: /mobile navigation/i }); + // Regex, not an exact string: the sr-only cue is a separate text node and + // accname implementations differ on whether they insert a separator. + expect( + within(nav).getByRole('link', { name: /^GitHub\s*\(opens in new tab\)$/ }), + ).toHaveAttribute('href', 'https://github.com/CodeForPhilly'); + expect(within(nav).getByRole('link', { name: 'Volunteer' })).toHaveAttribute( + 'href', + '/volunteer', + ); + }); }); diff --git a/apps/web/tests/ConnectGitHubBanner.test.tsx b/apps/web/tests/ConnectGitHubBanner.test.tsx index 6a21568..3637bc2 100644 --- a/apps/web/tests/ConnectGitHubBanner.test.tsx +++ b/apps/web/tests/ConnectGitHubBanner.test.tsx @@ -70,11 +70,11 @@ describe('ConnectGitHubBanner', () => { render(); await waitFor(() => { expect( - screen.getByRole('region', { name: /connect github/i }), + screen.getByRole('status', { name: /connect github/i }), ).toBeInTheDocument(); }); // CTA form posts to the link endpoint. - const region = screen.getByRole('region', { name: /connect github/i }); + const region = screen.getByRole('status', { name: /connect github/i }); expect(region.querySelector('form[action="/api/auth/link-github"]')).not.toBeNull(); expect(screen.getByRole('button', { name: /dismiss/i })).toBeInTheDocument(); }); @@ -84,7 +84,7 @@ describe('ConnectGitHubBanner', () => { render(); await waitFor(() => { expect( - screen.getByRole('region', { name: /connect github/i }), + screen.getByRole('status', { name: /connect github/i }), ).toBeInTheDocument(); }); }); @@ -101,7 +101,7 @@ describe('ConnectGitHubBanner', () => { // microtask gap. await new Promise((r) => setTimeout(r, 0)); expect( - screen.queryByRole('region', { name: /connect github/i }), + screen.queryByRole('status', { name: /connect github/i }), ).not.toBeInTheDocument(); }); @@ -110,7 +110,7 @@ describe('ConnectGitHubBanner', () => { render(); await new Promise((r) => setTimeout(r, 0)); expect( - screen.queryByRole('region', { name: /connect github/i }), + screen.queryByRole('status', { name: /connect github/i }), ).not.toBeInTheDocument(); }); @@ -121,7 +121,7 @@ describe('ConnectGitHubBanner', () => { fireEvent.click(dismissBtn); await waitFor(() => { expect( - screen.queryByRole('region', { name: /connect github/i }), + screen.queryByRole('status', { name: /connect github/i }), ).not.toBeInTheDocument(); }); }); diff --git a/apps/web/tests/MarkdownEditor.test.tsx b/apps/web/tests/MarkdownEditor.test.tsx new file mode 100644 index 0000000..a621cf9 --- /dev/null +++ b/apps/web/tests/MarkdownEditor.test.tsx @@ -0,0 +1,72 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { renderWithRouter } from './test-utils.js'; +import { MarkdownEditor } from '../src/components/MarkdownEditor.js'; + +function Harness() { + return <MarkdownEditor label="Overview" value="" onChange={() => {}} />; +} + +describe('MarkdownEditor formatting toolbar', () => { + beforeEach(() => { + // The preview round-trip is skipped for empty content, but stub fetch + // anyway so a stray call can never reach the network. + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, { status: 404 })); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('exposes a labelled toolbar whose buttons have real names', () => { + renderWithRouter(<Harness />); + const toolbar = screen.getByRole('toolbar', { name: 'Formatting' }); + for (const name of ['Bold', 'Italic', 'Insert link', 'Bulleted list', 'Code', 'Quote']) { + expect(within(toolbar).getByRole('button', { name })).toBeInTheDocument(); + } + }); + + it('keeps every accessible name a superset of the visible label (SC 2.5.3)', () => { + renderWithRouter(<Harness />); + const toolbar = screen.getByRole('toolbar', { name: 'Formatting' }); + for (const [visible, name] of [ + ['B', 'Bold'], + ['I', 'Italic'], + ['Link', 'Insert link'], + ['List', 'Bulleted list'], + ] as const) { + const btn = within(toolbar).getByRole('button', { name }); + expect(btn.textContent).toBe(visible); + expect(name.toLowerCase()).toContain(visible.toLowerCase()); + } + }); + + it('is a single tab stop with a roving tabindex', async () => { + const user = userEvent.setup(); + renderWithRouter(<Harness />); + const toolbar = screen.getByRole('toolbar', { name: 'Formatting' }); + const buttons = within(toolbar).getAllByRole('button'); + + // Only the active button is reachable by Tab. + expect(buttons.filter((b) => b.getAttribute('tabindex') === '0')).toHaveLength(1); + expect(buttons[0]).toHaveAttribute('tabindex', '0'); + + buttons[0]!.focus(); + await user.keyboard('{ArrowRight}'); + expect(buttons[1]).toHaveFocus(); + expect(buttons[1]).toHaveAttribute('tabindex', '0'); + expect(buttons[0]).toHaveAttribute('tabindex', '-1'); + + await user.keyboard('{End}'); + expect(buttons[buttons.length - 1]).toHaveFocus(); + + // Wraps forward off the end, and Home returns to the first. + await user.keyboard('{ArrowRight}'); + expect(buttons[0]).toHaveFocus(); + await user.keyboard('{ArrowLeft}'); + expect(buttons[buttons.length - 1]).toHaveFocus(); + await user.keyboard('{Home}'); + expect(buttons[0]).toHaveFocus(); + }); +}); diff --git a/apps/web/tests/PersonDetail.test.tsx b/apps/web/tests/PersonDetail.test.tsx index e5d2642..0c35fd3 100644 --- a/apps/web/tests/PersonDetail.test.tsx +++ b/apps/web/tests/PersonDetail.test.tsx @@ -1,5 +1,5 @@ import { describe, expect, it, vi, afterEach } from 'vitest'; -import { screen, waitFor } from '@testing-library/react'; +import { screen, waitFor, within } from '@testing-library/react'; import { Routes, Route } from 'react-router'; import { renderScreen, mockOk } from './test-utils.js'; import { PersonDetail } from '../src/screens/PersonDetail.js'; @@ -42,6 +42,31 @@ function makeFetchMock(person: typeof BASE_PERSON) { }) as typeof fetch; } +describe('PersonDetail breadcrumbs', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('renders the "Members › <fullName>" trail from app-shell.md', async () => { + vi.spyOn(globalThis, 'fetch').mockImplementation(makeFetchMock(BASE_PERSON)); + renderScreen( + <AuthProvider> + <Routes> + <Route path="/members/:slug" element={<PersonDetail />} /> + </Routes> + </AuthProvider>, + { initialEntries: ['/members/jane-doe'] }, + ); + + const trail = await screen.findByRole('navigation', { name: 'Breadcrumb' }); + expect(within(trail).getByRole('link', { name: 'Members' })).toHaveAttribute( + 'href', + '/members', + ); + expect(within(trail).getByText('Jane Doe')).toHaveAttribute('aria-current', 'page'); + }); +}); + describe('PersonDetail Contact sidebar', () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/apps/web/tests/ProjectDetail.test.tsx b/apps/web/tests/ProjectDetail.test.tsx index ff4c486..4c323a6 100644 --- a/apps/web/tests/ProjectDetail.test.tsx +++ b/apps/web/tests/ProjectDetail.test.tsx @@ -1,5 +1,5 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; -import { screen, waitFor } from '@testing-library/react'; +import { screen, waitFor, within } from '@testing-library/react'; import { Routes, Route } from 'react-router'; import { renderScreen, mockOk, mockPaginated } from './test-utils.js'; import { ProjectDetail } from '../src/screens/ProjectDetail.js'; @@ -59,6 +59,31 @@ describe('ProjectDetail', () => { vi.restoreAllMocks(); }); + it('renders the "Projects › <title>" breadcrumb trail from app-shell.md', async () => { + renderScreen( + <AuthProvider> + <Routes> + <Route path="/projects/:slug" element={<ProjectDetail />} /> + </Routes> + </AuthProvider>, + { initialEntries: ['/projects/sample-project'] }, + ); + + const trail = await screen.findByRole('navigation', { name: 'Breadcrumb' }); + expect(within(trail).getByRole('link', { name: 'Projects' })).toHaveAttribute( + 'href', + '/projects', + ); + // The last crumb is the current page, so it is text, not a link. + expect(within(trail).getByText('Sample Project')).toHaveAttribute( + 'aria-current', + 'page', + ); + expect( + within(trail).queryByRole('link', { name: 'Sample Project' }), + ).not.toBeInTheDocument(); + }); + it('renders the title, overview, and Sign-in CTA for anonymous', async () => { renderScreen( <AuthProvider> diff --git a/apps/web/tests/SearchBox.test.tsx b/apps/web/tests/SearchBox.test.tsx new file mode 100644 index 0000000..45402c6 --- /dev/null +++ b/apps/web/tests/SearchBox.test.tsx @@ -0,0 +1,173 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useLocation } from 'react-router'; +import { renderWithRouter, mockPaginated } from './test-utils.js'; +import { SearchBox } from '../src/components/SearchBox.js'; +import { NetworkErrorProvider } from '../src/components/NetworkErrorBanner.js'; + +/** Surfaces the router location so we can assert on in-SPA navigation. */ +function LocationProbe() { + const loc = useLocation(); + return <div data-testid="location">{`${loc.pathname}${loc.search}`}</div>; +} + +function Wrapped() { + return ( + <NetworkErrorProvider> + <SearchBox /> + <LocationProbe /> + </NetworkErrorProvider> + ); +} + +/** Type a query and wait for the debounced results to land. */ +async function openWithResults(user: ReturnType<typeof userEvent.setup>) { + const input = screen.getByRole('combobox', { name: 'Search the site' }); + await user.type(input, 'react'); + await waitFor( + () => { + // 3 results + the trailing "See all results" option + expect(screen.getAllByRole('option')).toHaveLength(4); + }, + { timeout: 3000 }, + ); + return input; +} + +describe('SearchBox', () => { + beforeEach(() => { + vi.spyOn(globalThis, 'fetch').mockImplementation(((input: string) => { + if (input.startsWith('/api/projects')) { + return Promise.resolve( + new Response(JSON.stringify(mockPaginated([{ slug: 'p1', title: 'Project One' }])), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + } + if (input.startsWith('/api/people')) { + return Promise.resolve( + new Response(JSON.stringify(mockPaginated([{ slug: 'm1', fullName: 'Member One' }])), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + } + if (input.startsWith('/api/tags')) { + return Promise.resolve( + new Response( + JSON.stringify( + mockPaginated([ + { slug: 'react', namespace: 'tech', handle: 'tech.react', title: 'React' }, + ]), + ), + { status: 200, headers: { 'content-type': 'application/json' } }, + ), + ); + } + return Promise.resolve(new Response(null, { status: 404 })); + }) as typeof fetch); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('exposes the APG combobox attributes', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + + const input = screen.getByRole('combobox', { name: 'Search the site' }); + expect(input).toHaveAttribute('aria-autocomplete', 'list'); + expect(input).toHaveAttribute('aria-expanded', 'false'); + expect(input).not.toHaveAttribute('aria-activedescendant'); + + await openWithResults(user); + + expect(input).toHaveAttribute('aria-expanded', 'true'); + const listbox = screen.getByRole('listbox', { name: 'Search results' }); + expect(input).toHaveAttribute('aria-controls', listbox.id); + }, 20000); + + it('owns only groups and options inside the listbox', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + await openWithResults(user); + + // Group headers are exposed as labelled groups, not stray divs. + expect(screen.getByRole('group', { name: 'Projects' })).toBeInTheDocument(); + expect(screen.getByRole('group', { name: 'Members' })).toBeInTheDocument(); + expect(screen.getByRole('group', { name: 'Tags' })).toBeInTheDocument(); + + // The status region sits outside the listbox. + const listbox = screen.getByRole('listbox', { name: 'Search results' }); + for (const child of Array.from(listbox.children)) { + expect(['group', 'option']).toContain(child.getAttribute('role')); + } + }, 20000); + + it('moves aria-activedescendant with ArrowDown and navigates on Enter', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + const input = await openWithResults(user); + + await user.keyboard('{ArrowDown}'); + + const options = screen.getAllByRole('option'); + expect(input).toHaveAttribute('aria-activedescendant', options[0]!.id); + expect(options[0]).toHaveAttribute('aria-selected', 'true'); + expect(options[1]).toHaveAttribute('aria-selected', 'false'); + + await user.keyboard('{ArrowDown}'); + expect(input).toHaveAttribute('aria-activedescendant', options[1]!.id); + + await user.keyboard('{ArrowUp}'); + expect(input).toHaveAttribute('aria-activedescendant', options[0]!.id); + + await user.keyboard('{Enter}'); + + await waitFor(() => { + expect(screen.getByTestId('location')).toHaveTextContent('/projects/p1'); + }); + }, 20000); + + it('wraps from the last option back to the first', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + const input = await openWithResults(user); + + const options = screen.getAllByRole('option'); + await user.keyboard('{ArrowUp}'); + expect(input).toHaveAttribute('aria-activedescendant', options[3]!.id); + + await user.keyboard('{ArrowDown}'); + expect(input).toHaveAttribute('aria-activedescendant', options[0]!.id); + }, 20000); + + it('closes the popup on Escape', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + const input = await openWithResults(user); + + await user.keyboard('{Escape}'); + + await waitFor(() => { + expect(screen.queryByRole('listbox')).not.toBeInTheDocument(); + }); + expect(input).toHaveAttribute('aria-expanded', 'false'); + expect(input).not.toHaveAttribute('aria-activedescendant'); + }, 20000); + + it('falls back to the all-results route when no option is active', async () => { + const user = userEvent.setup(); + renderWithRouter(<Wrapped />); + await openWithResults(user); + + await user.keyboard('{Enter}'); + + await waitFor(() => { + expect(screen.getByTestId('location')).toHaveTextContent('/projects?q=react'); + }); + }, 20000); +}); diff --git a/apps/web/tests/TagPicker.test.tsx b/apps/web/tests/TagPicker.test.tsx new file mode 100644 index 0000000..9a80d15 --- /dev/null +++ b/apps/web/tests/TagPicker.test.tsx @@ -0,0 +1,169 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useState } from 'react'; +import { renderScreen, mockPaginated } from './test-utils.js'; +import { TagPicker } from '../src/components/TagPicker.js'; + +const TAGS = [ + { + id: 't1', + handle: 'topic.civic-tech', + namespace: 'topic', + slug: 'civic-tech', + title: 'Civic Tech', + projectCount: 3, + personCount: 2, + helpWantedCount: 0, + }, + { + id: 't2', + handle: 'topic.housing', + namespace: 'topic', + slug: 'housing', + title: 'Housing', + projectCount: 1, + personCount: 0, + helpWantedCount: 0, + }, +]; + +/** Drives TagPicker as a real consumer would — controlled `value`. */ +function Harness({ allowCreate = false }: { allowCreate?: boolean }) { + const [value, setValue] = useState<string[]>([]); + return ( + <TagPicker + namespace="topic" + label="Topics" + value={value} + onChange={setValue} + allowCreate={allowCreate} + /> + ); +} + +async function findCombobox() { + return waitFor(() => screen.getByRole('combobox', { name: 'Topics' })); +} + +describe('TagPicker', () => { + beforeEach(() => { + vi.spyOn(globalThis, 'fetch').mockImplementation(((input: string) => { + if (input.startsWith('/api/tags')) { + return Promise.resolve( + new Response(JSON.stringify(mockPaginated(TAGS)), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + } + return Promise.resolve(new Response(null, { status: 404 })); + }) as typeof fetch); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('associates its label with the combobox input', async () => { + renderScreen(<Harness />); + + // getByLabelText only resolves if <Label htmlFor> points at the input id. + const input = await waitFor(() => screen.getByLabelText('Topics')); + expect(input).toHaveAttribute('role', 'combobox'); + expect(input).toHaveAttribute('aria-autocomplete', 'list'); + expect(input).toHaveAttribute('aria-expanded', 'false'); + }, 20000); + + it('selects the active option with ArrowDown + Enter', async () => { + const user = userEvent.setup(); + renderScreen(<Harness />); + + const input = await findCombobox(); + await user.click(input); + + await waitFor(() => { + expect(screen.getAllByRole('option')).toHaveLength(2); + }); + expect(input).toHaveAttribute('aria-expanded', 'true'); + + const options = screen.getAllByRole('option'); + expect(input).not.toHaveAttribute('aria-activedescendant'); + + await user.keyboard('{ArrowDown}'); + expect(input).toHaveAttribute('aria-activedescendant', options[0]!.id); + expect(options[0]).toHaveAttribute('aria-selected', 'true'); + + await user.keyboard('{ArrowDown}'); + expect(input).toHaveAttribute('aria-activedescendant', options[1]!.id); + + await user.keyboard('{Enter}'); + + // The selected tag becomes a removable chip using the house idiom. + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Remove housing' })).toBeInTheDocument(); + }); + expect(screen.queryByRole('listbox')).not.toBeInTheDocument(); + }, 20000); + + it('closes the listbox on Escape', async () => { + const user = userEvent.setup(); + renderScreen(<Harness />); + + const input = await findCombobox(); + await user.click(input); + + await waitFor(() => { + expect(screen.getByRole('listbox')).toBeInTheDocument(); + }); + + await user.keyboard('{Escape}'); + + await waitFor(() => { + expect(screen.queryByRole('listbox')).not.toBeInTheDocument(); + }); + expect(input).toHaveAttribute('aria-expanded', 'false'); + }, 20000); + + it('still offers the create branch as a keyboard-reachable option', async () => { + const user = userEvent.setup(); + renderScreen(<Harness allowCreate />); + + const input = await findCombobox(); + await user.type(input, 'brand-new'); + + await waitFor(() => { + expect(screen.getByRole('option', { name: /Create new tag/ })).toBeInTheDocument(); + }); + + await user.keyboard('{ArrowDown}{Enter}'); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Remove brand-new' })).toBeInTheDocument(); + }); + }, 20000); + + it('removes the last tag on Backspace in an empty input', async () => { + const user = userEvent.setup(); + renderScreen(<Harness />); + + const input = await findCombobox(); + await user.click(input); + await waitFor(() => { + expect(screen.getAllByRole('option')).toHaveLength(2); + }); + await user.keyboard('{ArrowDown}{Enter}'); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Remove civic-tech' })).toBeInTheDocument(); + }); + + await user.click(input); + await user.keyboard('{Backspace}'); + + await waitFor(() => { + expect( + screen.queryByRole('button', { name: 'Remove civic-tech' }), + ).not.toBeInTheDocument(); + }); + }, 20000); +}); diff --git a/apps/web/tests/Volunteer.test.tsx b/apps/web/tests/Volunteer.test.tsx new file mode 100644 index 0000000..3a45b0c --- /dev/null +++ b/apps/web/tests/Volunteer.test.tsx @@ -0,0 +1,86 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import { renderScreen, mockPaginated } from './test-utils.js'; +import { Volunteer } from '../src/screens/Volunteer.js'; +import { AuthProvider } from '../src/hooks/useAuth.js'; + +const MEETUP_URL = 'https://www.meetup.com/Code-for-Philly/'; +const START_PROJECT_URL = + 'https://github.com/CodeForPhilly/partnerships/blob/master/creating-new-partnerships/first-steps.md'; + +describe('Volunteer', () => { + beforeEach(() => { + vi.spyOn(globalThis, 'fetch').mockImplementation(((input: string) => { + if (input.startsWith('/api/auth/me')) { + return Promise.resolve(new Response(null, { status: 404 })); + } + if (input.startsWith('/api/projects')) { + return Promise.resolve( + new Response(JSON.stringify(mockPaginated([], { totalItems: 268 })), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + } + return Promise.resolve( + new Response(JSON.stringify(mockPaginated([])), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + }) as typeof fetch); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + function renderVolunteer() { + return renderScreen( + <AuthProvider> + <Volunteer /> + </AuthProvider>, + ); + } + + it('renders the hero headline', () => { + renderVolunteer(); + expect( + screen.getByRole('heading', { + name: /volunteer with code for philly/i, + level: 1, + }), + ).toBeInTheDocument(); + }); + + it('points "When we meet →" at the live Meetup group, not the dead GitBook page', () => { + renderVolunteer(); + const link = screen.getByRole('link', { name: /when we meet/i }); + expect(link).toHaveAttribute('href', MEETUP_URL); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', 'noopener noreferrer'); + }); + + it('points "Read the guide →" at the partnerships repo, not the dead GitBook page', () => { + renderVolunteer(); + const link = screen.getByRole('link', { name: /read the guide/i }); + expect(link).toHaveAttribute('href', START_PROJECT_URL); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', 'noopener noreferrer'); + }); + + it('has no codeforphilly.gitbook.io links anywhere on the screen', async () => { + const { container } = renderVolunteer(); + + // Wait for the live project count so the fully-settled DOM is asserted on. + await waitFor(() => { + expect(screen.getByText(/browse 268 active projects/i)).toBeInTheDocument(); + }); + + const hrefs = Array.from(container.querySelectorAll('a')).map( + (a) => a.getAttribute('href') ?? '', + ); + expect(hrefs.filter((h) => h.includes('gitbook.io'))).toHaveLength(0); + expect(container.innerHTML).not.toContain('codeforphilly.gitbook.io'); + }); +}); diff --git a/plans/a11y-mechanical.md b/plans/a11y-mechanical.md new file mode 100644 index 0000000..ce7892d --- /dev/null +++ b/plans/a11y-mechanical.md @@ -0,0 +1,190 @@ +--- +status: done +depends: [] +specs: + - specs/behaviors/app-shell.md +issues: [] +pr: 157 +--- + +# Plan: mechanical accessibility fixes across the SPA + +## Scope + +The third and largest bucket of the `apps/web` accessibility audit: findings +with **one obviously-correct fix each** and no design decision attached. +Repeated button names in lists, heading-level skips, missing toolbar +semantics, unannounced status changes, dates trapped in `title` attributes, +missing new-tab cues, landmark nesting, and the `Breadcrumbs` component that +`specs/behaviors/app-shell.md` prescribes but nothing renders. + +This branch **stacks on PR #154** (`fix/site-check-153` — header/nav rewrite) +and **PR #155** (`fix/aria-correctness` — ARIA validity), because it edits +many of the same files. It is cut from #154 with #155 merged in. + +It **complements issue #156**, which holds the audit's *design-decision* +findings (colour contrast, `document.title`, motion/pause controls, +`CardTitle` semantics, the `NetworkErrorBanner` "Retry" contradiction). +Nothing from #156 is implemented here — those need decisions, not fixes. + +Only one item is spec-facing, and it is conformance **to** an existing spec: +`specs/behaviors/app-shell.md` → Breadcrumbs already prescribes an exact +table of trails. **No spec change is needed anywhere in this plan.** + +## Implements + +- [app-shell.md](../specs/behaviors/app-shell.md) — **Breadcrumbs**: the + prescribed trail table is brought to code on all six screens that declare + one. The existing `Breadcrumbs.tsx` component was already correct and + complete; it was simply never imported. + +## Approach + +### 1. Breadcrumbs wiring (the spec-conformance item) + +`apps/web/src/components/Breadcrumbs.tsx` renders `nav[aria-label="Breadcrumb"] +> ol > li` with `aria-current="page"` on the last crumb — correct as written, +imported by nothing. Wired into the six screens the spec's table names, each +placed as the first child of the screen's content container (the spec's "row +below the header"): + +| Route | Trail | +|---|---| +| `/projects/:slug` | Projects › `<title>` | +| `/projects/:slug/edit` | Projects › `<title>` › Edit | +| `/projects/create` | Projects › New project | +| `/members/:slug` | Members › `<fullName>` | +| `/tags/:namespace/:slug` | Tags › `<namespace>` › `<title>` | +| `/account` | Settings | + +No `specs/screens/*.md` mentions breadcrumbs at all, so there is no +contradiction to resolve — app-shell.md is the sole authority. + +### 2. Repeated identical button names in lists (SC 4.1.2 / 2.4.6) + +A screen-reader user tabbing a list of "Remove / Remove / Remove" has no way +to tell the rows apart. Each gets an `aria-label` that **contains its visible +text** (SC 2.5.3) plus the row's subject: + +| File | Buttons | Label shape | +|---|---|---| +| `modals/ManageMembersModal.tsx` | Edit role, Make maintainer, Remove | `Remove ${fullName}` | +| `screens/Account.tsx` | Revoke (per session) | `Revoke session on ${device}` | +| `pages/StaffAccountClaimQueue.tsx` | Approve, Deny | `Approve claim from ${login}` | +| `screens/ProjectDetail.tsx` | Mark filled, Close | `Close ${role.title}` | + +### 3. Heading levels + +Three index screens jump `h1` → `h3` because their card components render +`h3`. **The cards are not changed** — `PersonCard` and `HelpWantedCard` are +each used in a second context (`TagDetail`, `Home`, `Volunteer`) where they +sit correctly under a section `h2`. Instead each index screen gains an +`sr-only <h2>` section heading above its results region, which is both the +smaller change and the more honest markup: the grid *is* a section. + +`ProjectDetail` and `PersonDetail` aside headings go `h3` → `h2` directly — +they sit under the screen `h1` with no intervening heading, are used nowhere +else, and keep their existing classes so nothing moves visually. (Heading +level and visual size are independent.) + +`ProjectsIndex` was checked for the same pattern and does **not** have it — +`ProjectCard` already renders `h2`. No sr-only heading added there. + +### 4. `MarkdownEditor` toolbar + +The six formatting buttons were a bare `<div>` of buttons named "B", "I", +"Link"… Now `role="toolbar"` + `aria-label="Formatting"`, each button +`aria-label`'d with a full name that contains its visible label as a +substring (B ⊂ Bold, I ⊂ Italic, Link ⊂ Insert link, List ⊂ Bulleted list), +and a roving tabindex: only the active button is tabbable, +ArrowLeft/ArrowRight move focus (wrapping), Home/End jump to the ends. + +### 5. Status announcements + +Three places changed state visually with nothing announced: + +- `ProjectDetail` "Copy link" / "Share to Slack" gave **no feedback at all** — + now `toast.success(...)` via sonner, which this screen's own modals already + use for exactly this kind of confirmation. +- `Sponsor`'s "Copy email" swaps its label to "Copied ✓" — visible text kept, + with an `sr-only role="status"` mirror added. +- `ProfileEdit`'s "Uploading…" span becomes `role="status"`. +- `ConnectGitHubBanner` was `role="region"`, which is never announced; the + banner appears *after* auth resolves, so it becomes `role="status"`. + +### 6. `<time dateTime>` for machine-readable dates + +`title` is not exposed to most screen readers and never on touch. Every date +rendered as relative text inside a `title`-only `<span>` becomes +`<time dateTime={iso} title={absolute}>` — the `BlogIndex.tsx` idiom. +Covers `ActivityCard` (×2), `ProjectDetail` (×2), `BlogDetail`, `Account`, +`StaffAccountClaimQueue`, `AccountClaim`. `ProjectCard`'s wrapper +`title={m.fullName}` is deleted outright — `PersonAvatar` already emits it. + +### 7. Structure and one-liners + +- `PersonCard` was one giant `<Link>`, so its accessible name concatenated + avatar + name + project count + every tag chip. Restructured to the + `ProjectCard` idiom: `<article>` with the `h3` wrapping the link. The hover + lift moves to the article via `group-hover`, so the affordance is unchanged. +- `AppHeader`'s two navs render bare links; wrapped in `<ul>/<li>` matching + `AppFooter`. Flex/gap classes move to the `ul`; `li` contributes nothing. + Sheet separators sit between the two lists rather than inside one. +- The mobile sheet's "About" group label was a styled `<p>` → `<h3>` (one + level below the Radix `SheetTitle`, which renders `h2`), same classes. +- `HelpWantedIndex`'s bare outer `<aside>` wrapped `FacetSidebar`, which + renders its own labelled `<aside>` — two nested `complementary` landmarks. + Outer becomes a `<div>`. (`PeopleIndex`/`ProjectsIndex` render + `FacetSidebar` directly and never had this.) +- Result-count badges move **out** of the `h1` into a flex sibling on all + three index screens, so the heading's accessible name stops mutating as + filters change. +- `target="_blank"` links get a new-tab cue: `<span className="sr-only"> (opens + in new tab)</span>` where visible text exists, appended to the `aria-label` + where it does not. +- `ProjectDetail`'s "More ▾" menu trigger gets `aria-label="More actions"`; + the "What does this stage mean?" dialog trigger gets `aria-haspopup="dialog"`. + +## Validation + +- [x] Breadcrumbs render on all six screens the spec's table names, with the + exact trails prescribed, and each non-final crumb links to its parent. +- [x] No two buttons in the audited lists share an accessible name; every + added `aria-label` contains the button's visible text (SC 2.5.3). +- [x] `PeopleIndex` / `HelpWantedIndex` no longer skip `h1` → `h3`; + `ProjectDetail` / `PersonDetail` aside headings are `h2`. +- [x] The `MarkdownEditor` toolbar exposes `role="toolbar"`, named buttons, + and a working roving tabindex (Arrow/Home/End). +- [x] Copy actions on `ProjectDetail` and `Sponsor` announce; `ProfileEdit` + upload and `ConnectGitHubBanner` are live regions. +- [x] Dates expose `datetime`; no date is `title`-only. +- [x] Exactly one `complementary` landmark per index screen. +- [x] `npm run -w packages/shared build && npm run type-check && npm run lint + && npm run -w apps/web test && npm run -w packages/shared test` clean + (web 116/116, shared 75/75; run twice — implementer and coordinator). +- [x] Browser test (headed Chrome against the live dev stack — api booted on + a `setup-dev-data` repo with two seeded records): breadcrumb trails + verified on `/projects/qa-sandbox` ("Projects › QA Sandbox Project") + and `/members/ada-tester` ("Members › Ada Tester"); "Copy link" fires + the "Link copied" toast; the count badge sits outside the `h1` with + the visual unchanged; `PersonCard` whole-card click still navigates. + Toolbar keyboard nav verified in jsdom only (`MarkdownEditor.test.tsx`). + +## Risks / unknowns + +- **Low.** Almost every change is attribute-level or a wrapper element. +- The two structural edits (`PersonCard`, `AppHeader` nav lists) touch files + PR #154 rewrote. Both keep every existing behavior — the sheet's `onClick` + close handlers, the separators, the NavLink active styling — and are + covered by the existing `AppHeader.test.tsx` suite plus updated name + matchers. +- The `apps/api` suite is deliberately not run: it has a known pre-existing + Windows fixture failure and no `apps/api` file changes here. + +## Notes + +_(filled in at closeout)_ + +## Follow-ups + +_(filled in at closeout)_ diff --git a/plans/aria-correctness.md b/plans/aria-correctness.md new file mode 100644 index 0000000..4cf4ce1 --- /dev/null +++ b/plans/aria-correctness.md @@ -0,0 +1,163 @@ +--- +status: done +depends: [] +specs: + - specs/behaviors/app-shell.md +issues: [] +pr: 155 +--- + +# Plan: repair invalid and missing ARIA across the SPA + +## Scope + +An accessibility audit of `apps/web` turned up a set of **verified ARIA +correctness defects** — markup that is invalid per the ARIA spec (prohibited +attributes, illegal role ownership, dangling references) or that withholds +state from assistive technology that sighted users get visually. This plan +fixes that class of defect only. + +The two headline items are the site search box and the tag picker: both claim +`role="listbox"` today while owning non-option children, and neither is +operable from the keyboard. `specs/behaviors/app-shell.md` → Accessibility +already requires "All dropdowns are keyboard-navigable", so this is code +brought into conformance with an existing spec — **no spec change is needed**. + +Out of scope by deliberate choice: heading hierarchy, colour contrast, +`document.title`, motion/pause controls, breadcrumbs, `target="_blank"` cues, +toolbar semantics, repeated button names, `CardTitle` semantics. See +[Follow-ups](#follow-ups). + +## Implements + +- [app-shell.md](../specs/behaviors/app-shell.md) — **Accessibility** section, + partially: "All dropdowns are keyboard-navigable" now holds for the header + search box and the tag picker. The remaining bullets (logo link, skip link, + mobile sheet focus trap) were already satisfied; the skip link's focus ring + is restored here. + +## Approach + +### 1. `SearchBox` — rebuild as an APG combobox + +`apps/web/src/components/SearchBox.tsx` was invalid on every axis: it put +`aria-expanded`/`aria-controls` on an implicit `searchbox`, gave the popup +`role="listbox"` while it owned a bare `<p>` and unroled `<div>`s, hardcoded +`aria-selected={false}` on every option, and was unreachable by keyboard — Tab +blurred the input and a 150 ms `setTimeout` unmounted the popup, so only Enter +and Escape ever worked. + +Rebuilt to the [ARIA APG combobox-with-listbox +pattern](https://www.w3.org/WAI/ARIA/apg/patterns/combobox/): + +- Input carries `role="combobox"`, `aria-expanded`, `aria-controls`, + `aria-autocomplete="list"`, and `aria-activedescendant`. +- Group headers become `role="group"` + `aria-labelledby` → a + `role="presentation"` header element, so the listbox owns only + groups and options. +- "Searching…" / "No results" moves **out** of the listbox into a sibling + `role="status"` region. +- "See all results" becomes the final option in the listbox. +- ArrowDown/ArrowUp (wrapping), Home/End, Enter, Escape; focus never leaves + the input, so the blur race is structurally gone for keyboard users. The + popup swallows `mousedown` so a pointer click cannot blur the input either — + the 150 ms timeout is deleted rather than tuned. +- Options stay `<a href>` (valid: `option` is an allowed role for `a[href]`) + with `tabIndex={-1}`, so middle-click / "open in new tab" still work, while + a plain click is intercepted and routed through `useNavigate()` instead of + doing a full-page reload. +- The popup's hardcoded `id="search-results-dropdown"` is replaced by + `useId()`-derived ids. That id was duplicated whenever both the desktop and + the mobile-sheet instance rendered; the print stylesheet's hook moves to + `[data-search-dropdown]` to keep `specs/behaviors/app-shell.md` → Print true. + +### 2. `TagPicker` — same combobox pattern + +`apps/web/src/components/TagPicker.tsx` had `role="listbox"` on a `<ul>` whose +`<li>`s carried no role and wrapped `<button>`s, no combobox ARIA on the +driving input, no Escape/arrow handling, and a `label` prop that rendered a +`<Label>` associated with nothing (affecting ProjectEdit, ProfileEdit and +PostHelpWantedModal). Reworked to the same pattern: `useId()` ties `<Label +htmlFor>` to the input; `<li role="option">` become the interactive targets; +ArrowDown/ArrowUp/Escape/Enter. Enter falls back to the existing +exact-match → first-match → create-tag chain when no option is active, and +Backspace-removes-last is preserved. + +### 3. Small repairs + +| File | Defect | Fix | +|---|---|---| +| `PersonAvatar.tsx` | `aria-label` on a roleless `<span>` (prohibited) | `role="img"` | +| `StageBadge.tsx` | `aria-label` on a roleless `<div>`; progress conveyed by width only; tooltip triggers not focusable | `role="progressbar"` + value attrs; `tabIndex={0}` on both triggers | +| `LoginPlaceholder.tsx`, `AccountClaim.tsx` | `aria-live` + `aria-label` on an empty roleless div announces nothing | `role="status"` + `sr-only` text, spinner `aria-hidden` | +| `NetworkErrorBanner.tsx` | button reads "Retry", `aria-label` says "Dismiss error" (SC 2.5.3) | drop the `aria-label` | +| `TopProgressBar.tsx` | progressbar permanently exposed at 100% | `aria-hidden` when idle | +| `TagChip.tsx`, `Home.tsx` | toggle state conveyed visually only | `aria-pressed` | +| `ConnectGitHubBanner.tsx` | `aria-label` duplicates visible text | removed | +| `MarkdownEditor.tsx` | `aria-live` re-announces the whole preview each debounce; error text unassociated | drop `aria-live`; `aria-describedby` | +| `ManageMembersModal.tsx` | placeholder-only labelling | `aria-label="Role"` | +| `ProfileEdit.tsx` | "Avatar" `<Label>` labels nothing | `htmlFor` → file input `id` | +| `ProjectsIndex.tsx`, `HelpWantedIndex.tsx` | filter chips don't say they remove | `aria-label="Remove filter: …"` | +| `Pagination.tsx` | page buttons named only "3" | `aria-label="Page 3"` | +| `Account.tsx` | sessions table headers have no scope | `scope="col"` | +| `AppShell.tsx` | skip link ends with `focus:outline-none` | class removed | +| `ProjectEdit.tsx` | async slug availability never announced | `role="status"` + `aria-describedby` | + +### 4. Form error wiring (systemic) + +`aria-invalid` was set in several forms but the error text was never +programmatically associated, so a screen-reader user hears "invalid" with no +reason. Every error `<p>` gains an `${id}-error` id and its control gains a +conditional `aria-describedby`, applied uniformly across `AddMemberModal`, +`ProjectEdit` (5 fields), `ProjectBuzzNew` (4), `PostHelpWantedModal`, +`TagEditModal` (2) and `ProfileEdit` (2). + +## Validation + +- [ ] `SearchBox` exposes `role="combobox"` with `aria-expanded`, + `aria-controls`, `aria-autocomplete="list"`; the listbox owns only + groups/options; status text sits outside it. +- [ ] `SearchBox` keyboard: type → ArrowDown moves `aria-activedescendant` → + Enter navigates to the active option; Escape closes. +- [ ] `TagPicker` label is associated with its input; ArrowDown + Enter + selects an option; Escape closes; Backspace-removes-last still works. +- [ ] New tests `apps/web/tests/SearchBox.test.tsx` and + `apps/web/tests/TagPicker.test.tsx` cover the above. +- [ ] No `aria-label` remains on a roleless generic element in the audited set. +- [ ] Every audited `aria-invalid` control references its error text via + `aria-describedby`. +- [ ] `npm run -w packages/shared build && npm run type-check && npm run lint + && npm test` clean. + +## Risks + +- **Medium, contained to two widgets.** The SearchBox and TagPicker rewrites + change interaction, not just attributes. Both are covered by new focused + tests; both keep their existing Tailwind classes so the visual design is + unchanged. +- The rest is attribute-level and mechanical. + +## Notes + +_(filled in at closeout)_ + +## Follow-ups + +- **Spec↔code contradiction, surfaced not patched — the 5xx banner's "Retry" + button does not retry.** `NetworkErrorBanner`'s button calls `clearError()` + and nothing else: it dismisses the banner. Its visible text says "Retry" and + `specs/behaviors/app-shell.md:162` prescribes `[Retry]`, so spec and label + agree with each other and both disagree with the code. This plan only + removed the `aria-label="Dismiss error"` that contradicted the visible name + (SC 2.5.3) — note that the removed label was the one place the code admitted + what the button actually does. Resolving the contradiction is a behavior + question, not an ARIA one: either the banner gains a real retry (re-issuing + the failed call, which the context does not currently retain) or the spec and + label change to "Dismiss". Needs its own spec decision and plan; do not + settle it by renaming one side. +- The audit surfaced further categories that are **not** ARIA-correctness + defects and were deliberately excluded here — heading hierarchy, colour + contrast, `document.title`, motion/pause controls, breadcrumbs, + `target="_blank"` cues, toolbar semantics, repeated button names, and + `CardTitle` semantics. The coordinator holds the full audit report; these + want their own triage and plan. diff --git a/plans/site-check-153.md b/plans/site-check-153.md new file mode 100644 index 0000000..021ed1a --- /dev/null +++ b/plans/site-check-153.md @@ -0,0 +1,192 @@ +--- +status: done +depends: [] +specs: + - specs/behaviors/app-shell.md + - specs/screens/volunteer.md +issues: [153] +pr: 154 +--- + +# Plan: site check — header order, mobile sheet padding, dead outbound links + +## Scope + +Issue [#153](https://github.com/CodeForPhilly/codeforphilly-ng/issues/153) +("Site check for desktop & mobile") collects a walkthrough of the live site on +both breakpoints. Four of its items are shippable now; one is blocked (see +Follow-ups). + +What ships: + +- **Desktop header reorder** (spec-governed). The Volunteer CTA leaves the + content nav and becomes the rightmost element of the header, after the auth + control; About joins the left cluster's text links; a GitHub icon link is + added to the right cluster. +- **Mobile sheet padding + accessible name.** The sheet's nav and search sat + flush against the panel edge. Fixed with the intended shadcn structure + (`SheetHeader` + `SheetTitle`) plus explicit horizontal padding — which also + gives the underlying Radix dialog the accessible name it was missing. +- **Header ARIA cleanups.** Three defects surfaced by an accessibility pass over + the header, done here because this plan rewrites the same file. +- **Dead outbound links** (spec-governed). The whole `codeforphilly.gitbook.io` + space returns 404 "Content owner not found"; `Volunteer.tsx`'s two remaining + GitBook links are repointed at live equivalents. Same class of defect as + [`home-start-project-cta`](home-start-project-cta.md) (PR #128), which fixed + the Home screen's copy of the same dead URL. +- **Footer repo URL.** The "view this site on GitHub" link still pointed at + `codeforphilly-rewrite`; the repo is `codeforphilly-ng` and the old URL only + resolves through GitHub's rename redirect. + +Explicitly out of scope: + +- **Replacing the Home hero's Volunteer CTA with a mailing-list invite** (also + recommended by #153) — blocked, see Follow-ups. `Home.tsx` is untouched. +- Any other visual restyle of the header. The Volunteer button keeps its + existing green treatment; only its position changes. + +## Implements + +- [app-shell.md](../specs/behaviors/app-shell.md) — "Center / right at ≥ md" + split into a left content cluster and a right utility cluster, with the new + item order; "Auth controls" repositioned second-from-right; GitHub link added + to the right cluster and to the mobile sheet; the sheet's accessible name + added under Accessibility. +- [volunteer.md](../specs/screens/volunteer.md) — "Show up to meetups" card + links to the Meetup group; "Start your own project" band links to the + `CodeForPhilly/partnerships` first-steps guide. Both replace dead GitBook + URLs. + +## Approach + +### 1. Spec changes first (specops — source of truth leads) + +`specs/behaviors/app-shell.md` and `specs/screens/volunteer.md` both prescribed +the current (wrong) state, so they lead. Header spacing is deliberately *not* +specced — [specs/README.md:49](../specs/README.md) puts spacing outside spec +scope — so the mobile-sheet padding fix carries no spec change. + +### 2. `apps/web/src/components/AppHeader.tsx` + +- Left `<nav>`: Projects, Help Wanted, Members, About ▾. `gap-2` replaces + `gap-1` + per-child `ml-1`, so the parent gap is the single source of spacing + at the same effective density (4px + 4px → 8px). +- Right cluster: GitHub icon link → SearchBox → AuthControls → Volunteer button. + New hand-rolled `GitHubIcon` SVG follows the file's existing icon convention + (`ChevronDownIcon` / `MenuIcon`); path data copied from `LoginPlaceholder.tsx`. +- Mobile sheet: `SheetHeader` + `SheetTitle` ("Menu") replace the `pt-8` hack; + nav and search get `px-4`. Mobile item order mirrors the new desktop order, + with a GitHub row added and Volunteer last. +- ARIA: `aria-hidden` replaces `aria-label` on the roleless loading-skeleton + div; the hand-written `aria-expanded` comes off the `SheetTrigger` (Radix + `Dialog.Trigger` supplies it); `aria-label="About menu"` comes off the About + trigger so its visible text is the accessible name. The account-menu + `aria-label` **stays** — below `sm` the person's name span is `display:none`, + so that label is the only accessible name there. + +### 3. `apps/web/src/screens/Volunteer.tsx` + +`HACK_NIGHT_URL` → `MEETUP_URL` = `https://www.meetup.com/Code-for-Philly/` +(the same target the footer's Meetup social icon already uses). +`START_PROJECT_URL` → the `CodeForPhilly/partnerships` first-steps markdown, +which is the surviving source of the retired GitBook page. Both stay external. + +### 4. `apps/web/src/components/AppFooter.tsx` + +One-line repo URL swap to `codeforphilly-ng`. + +### 5. Tests + +- `AppHeader.test.tsx` — new nav shape, About queried by its visible text, the + GitHub link's label + href, Radix still supplying `aria-expanded`, and the + sheet dialog's accessible name. +- `Volunteer.test.tsx` (new) — both CTA hrefs plus a dead-link regression + assertion that no `codeforphilly.gitbook.io` URL survives anywhere in the + rendered document, mirroring the idiom from `Home.test.tsx`. +- `AppFooter.test.tsx` — updated repo URL. + +## Validation + +- [x] Specs updated before code: app-shell header clusters + volunteer link targets. +- [x] Desktop header order is Projects · Help Wanted · Members · About ▾ … GitHub · Search · Sign in · Volunteer, with Volunteer rightmost and still green. +- [x] No `ml-1` spacing hacks remain among the header nav's children. +- [x] GitHub link is icon-only, labelled "Code for Philly on GitHub", and opens `https://github.com/CodeForPhilly` in a new tab with `rel="noopener noreferrer"`. +- [x] Mobile sheet has a "Menu" title, horizontal padding on nav + search, and no `pt-8`; the title does not collide with the close button. +- [x] The sheet dialog exposes an accessible name; Radix still supplies `aria-expanded` on the trigger. +- [x] Loading skeleton uses `aria-hidden`; About trigger's accessible name is its visible text; account-menu label retained. +- [x] Every mobile sheet item closes the sheet on click, including Contact. +- [x] No `codeforphilly.gitbook.io` URL remains in `apps/web/src`. +- [x] Footer "view this site on GitHub" points at `codeforphilly-ng`. +- [x] Both replacement URLs return 200 and carry the expected content. +- [x] `npm run -w packages/shared build`, `npm run type-check`, and `npm run lint` clean. +- [x] `npm test` clean for the workspaces this plan touches: web 96/96, shared 75/75. +- [ ] `npm test` clean for **all** workspaces — `apps/api` cannot pass on the Windows dev box used here (see Notes); needs a Linux run or CI to close. +- [x] Browser test (headed Chrome, Vite dev server): desktop header order, + spacing, GitHub icon, and Volunteer-rightmost verified at 1400px; the + sheet verified open — "Menu" title, padded nav/search, no title/close + collision — and Escape closes it. Caveat: the harness could not shrink + the (maximized) window below md, so the sheet was opened via its + CSS-hidden trigger at desktop width. The sheet is a fixed `w-72` + portal, so its rendering is identical at < md; the < md *header bar* + (logo + auth + hamburger row) still rides on the jsdom tests, same + limitation `web-shell.md:109` recorded. + +## Risks + +- **Low, but layout-shaped.** Moving Volunteer out of the flex-1 nav and into + the `ml-auto` cluster changes how much room the SearchBox has to expand at + narrow desktop widths. Watched by the browser-test criterion above rather + than by a unit test — jsdom has no layout. +- **`aria-expanded` regression risk.** Removing the hand-written attribute is + only safe because Radix supplies its own; asserted in the header test so a + future primitive swap can't silently drop it. + +## Notes + +(To be populated at closeout. Recorded during implementation:) + +- **`apps/api` tests do not pass on Windows, independent of this plan.** + `apps/api` finishes 3 failed | 30 passed (33 files), 10 failed | 413 passed + (423 tests) — the ten spread across `scrub-data.test.ts` (4), + `internal-reload.test.ts` (4), and `store.test.ts` (2), on a tree where + `git diff develop..HEAD -- apps/api packages/` is empty — this branch touches + no API code. Checking out `develop` and re-running `store.test.ts` there + reproduces its 2 failed | 11 passed exactly. The mechanism is + POSIX-isms in the test fixtures: `store.test.ts` injects a write failure by + pointing the private store at `/dev/null/impossible-path` and asserting the + transaction rejects, but on Windows that is an ordinary creatable directory, + so the write succeeds and the expected throw never happens. They reproduce + with the files run alone, so it is not test-runner contention. CI runs the + same gate on Linux, where the fixture behaves as intended. Worth a + cross-platform fixture cleanup if Windows dev boxes are to be supported; + filed under Follow-ups. + +## Follow-ups + +(To be populated at closeout. Known now:) + +- **Tracked as: blocked — hero "mailing list invite" CTA.** Issue #153 + recommends replacing the Home hero's Volunteer CTA with a mailing-list + invite. There is no anonymous mailing-list mechanism anywhere in the repo: + newsletter subscription exists only as an auth-gated checkbox on `/account` + (writing `PrivateProfile.newsletter`), and a public signup surface is + explicitly deferred — [app-shell.md:140](../specs/behaviors/app-shell.md) + lists "Newsletter signup (defer)" in the footer's Connect column, and + [deferred.md:100-104](../specs/deferred.md) defers the whole newsletter + sending pipeline with a promotion path (`/api/newsletter/send`, Resend-backed + worker, unsubscribe tokens). Building an anonymous-capture CTA ahead of that + spec would invent unspecified behavior. `Home.tsx` is deliberately untouched + here; the CTA swap should follow the newsletter spec work, not precede it. + +- **Tracked as: dead file, not fixed here — `apps/web/src/pages/HomeStub.tsx`.** + It carries the same stale `codeforphilly-rewrite` URL the footer had, but + nothing imports or routes it (`App.tsx` imports only `LoginPlaceholder` from + `src/pages/`; every live screen lives in `src/screens/`). Left alone because + the right fix is deleting the file, not patching a URL nobody renders — and + that deletion wants its own scope. Flagging so a future grep for the old repo + name doesn't read as an unfixed live link. + +- **Issue — make the `apps/api` test fixtures cross-platform.** The `/dev/null` + failure-injection idiom (and whatever the other seven failures share) makes + the API suite unrunnable on a Windows dev box, so the documented validation + gate can only be completed on Linux or in CI. See Notes for the mechanism. diff --git a/specs/behaviors/app-shell.md b/specs/behaviors/app-shell.md index 67659f3..2262f09 100644 --- a/specs/behaviors/app-shell.md +++ b/specs/behaviors/app-shell.md @@ -35,16 +35,31 @@ Sticky at the top of the viewport. Background opaque, slight shadow on scroll. ### Center / right at ≥ md -Primary nav, items in this order: +Two clusters. The **content cluster** sits next to the logo; the **utility +cluster** is pinned to the right edge and carries the outbound link, search, +auth, and the call to action. + +Content cluster, items in this order: | Item | Target | Style | | ---- | ------ | ----- | | Projects | `/projects` | text link | | Help Wanted | `/help-wanted` | text link | | Members | `/members` | text link | -| Volunteer | `/volunteer` | button (success, filled) — emphasized because it's the call to action | | About ▾ | dropdown | text link with caret | + +Utility cluster, items in this order (left to right): + +| Item | Target | Style | +| ---- | ------ | ----- | +| GitHub | `https://github.com/CodeForPhilly` | icon-only external link, accessible name "Code for Philly on GitHub", opens in a new tab | | Search 🔍 | inline expand | icon button | +| Sign in / account menu | see [Auth controls](#auth-controls) | button / avatar dropdown | +| Volunteer | `/volunteer` | button (success, filled) — emphasized because it's the call to action | + +**Volunteer is the rightmost element in the header.** It sits after the auth +control rather than among the content links so the call to action reads as the +header's terminal step, not as one more section. ### About dropdown @@ -57,7 +72,11 @@ Primary nav, items in this order: The `/pages/*` URLs serve **static content pages** authored as MDX/Markdown in the code repo (`apps/web/src/content/pages/`). They have no per-page screen spec — the content is the spec. Source copy ports from `codeforphilly.org/site-root/pages/` in the legacy repo. -### Auth controls (rightmost) +### Auth controls + +Second from the right in the utility cluster — between Search and the Volunteer +button. On mobile the auth control sits in the header bar itself, outside the +sheet. - **Anonymous:** "Sign in" (primary button) → `/login`. There is no separate "Sign up" button — sign-in and sign-up are the same flow once GitHub OAuth is specified (first sign-in creates the account). - **User:** Avatar + name dropdown: @@ -75,7 +94,9 @@ The `/pages/*` URLs serve **static content pages** authored as MDX/Markdown in t ### Mobile (< md) -Header collapses to: logo + hamburger menu + auth control. Hamburger opens a sheet (right-side overlay) with all nav items stacked vertically. Search is inside the sheet, not inline. +Header collapses to: logo + hamburger menu + auth control. Hamburger opens a sheet (right-side overlay) with all nav items stacked vertically — the content-cluster links, the About items under an "About" label, the GitHub link, and Volunteer last, mirroring the desktop order. Search is inside the sheet, not inline. The auth control stays in the header bar, outside the sheet. + +Every item in the sheet closes the sheet when activated. ## Search @@ -168,6 +189,8 @@ It does not block initial paint waiting on `me`. Auth controls render skeletons - Skip link at the very top: "Skip to main content" → focuses the `<main>` element - All dropdowns are keyboard-navigable - The mobile sheet traps focus while open and returns it to the trigger on close +- The mobile sheet is a dialog with the accessible name "Menu" +- Every icon-only control carries an accessible name; controls with visible text use that text as their accessible name rather than duplicating it in a label ## Print diff --git a/specs/screens/volunteer.md b/specs/screens/volunteer.md index 661925d..2bf03e1 100644 --- a/specs/screens/volunteer.md +++ b/specs/screens/volunteer.md @@ -27,7 +27,7 @@ Three cards in a row at ≥ md, stacked below: 1. **Join Slack** — "We coordinate everything in our Slack workspace." Button: "Open Slack →" → `/chat` 2. **Pick a project** — "Browse 268 active projects and find one that matches your interests." Button: "Browse projects →" → `/projects` -3. **Show up to meetups** — "We meet weekly. Bring your laptop, or just yourself." Button: "When we meet →" → external link (currently the GitBook hack-night-program-details URL) +3. **Show up to meetups** — "We meet weekly. Bring your laptop, or just yourself." Button: "When we meet →" → external link to the Meetup group (`https://www.meetup.com/Code-for-Philly/`), where upcoming hack nights are listed. Same target as the footer's Meetup social link. The "268" is read from a cheap call to `GET /api/projects?perPage=1` and rendered live; falls back to "hundreds of" if the call fails. @@ -48,7 +48,7 @@ Static content emphasizing the non-developer roles (designers, project managers, Footer-style band at the bottom: "Have an idea? Start your own project." -- Link to the external GitBook "creating-new-partnerships/first-steps" page (matches current codeforphilly.org) +- Link to the external "first steps" partnership guide (`https://github.com/CodeForPhilly/partnerships/blob/master/creating-new-partnerships/first-steps.md`) — the surviving canonical source of the retired GitBook page of the same name - Secondary link "or create one on the site →" to `/projects/create` (signed-in) or `/login?return=/projects/create` (anonymous) ## Actions