diff --git a/apps/web/src/components/AppFooter.tsx b/apps/web/src/components/AppFooter.tsx index 620b03f..11fbe62 100644 --- a/apps/web/src/components/AppFooter.tsx +++ b/apps/web/src/components/AppFooter.tsx @@ -200,7 +200,7 @@ export function AppFooter() { Copyright © Code for Philly {FOUNDED_YEAR}–{CURRENT_YEAR}

+ ); +} + export function AppHeader() { - const [mobileOpen, setMobileOpen] = useState(false); + const location = useLocation(); + // The sheet is open only for the location it was opened at, so any + // client-side navigation — a NavLink or Enter in the inline search — + // closes it without per-item onClick closers. Derived during render + // rather than synced in an effect (react-hooks/set-state-in-effect). + const [openedAtKey, setOpenedAtKey] = useState(null); + const mobileOpen = openedAtKey === location.key; + const setMobileOpen = (open: boolean) => + setOpenedAtKey(open ? location.key : null); return (
@@ -201,135 +233,125 @@ export function AppHeader() { /> - {/* Desktop nav */} + {/* Desktop content cluster. The parent gap is the only source of + spacing between children — no per-child margins. */} - {/* Desktop: search + auth */} -
+ {/* Desktop utility cluster: GitHub, search, auth, then the Volunteer + CTA pinned rightmost (specs/behaviors/app-shell.md). */} +
+ + {/* Mobile: auth + hamburger */}
+ {/* No aria-expanded here — Radix's Dialog.Trigger supplies it. */} - + + {/* SheetHeader/SheetTitle carry the panel's own padding and give + the underlying Radix dialog its accessible name; the + visually-hidden description satisfies aria-describedby. */} + + Menu + + Site navigation + + + {/* min-h-0 + overflow-y-auto so the list stays reachable on + short viewports instead of overflowing the panel. */} - +
+ +
diff --git a/apps/web/src/components/SearchBox.tsx b/apps/web/src/components/SearchBox.tsx index 42eca78..e1c67da 100644 --- a/apps/web/src/components/SearchBox.tsx +++ b/apps/web/src/components/SearchBox.tsx @@ -66,7 +66,7 @@ export function SearchBox({ inline = false }: SearchBoxProps) { return (
{loading && results.length === 0 && (

Searching…

diff --git a/apps/web/src/components/icons/GitHubIcon.tsx b/apps/web/src/components/icons/GitHubIcon.tsx new file mode 100644 index 0000000..6862c4e --- /dev/null +++ b/apps/web/src/components/icons/GitHubIcon.tsx @@ -0,0 +1,15 @@ +/** GitHub mark, sized to sit inline with text and icon buttons. */ +export function GitHubIcon() { + return ( + + ); +} diff --git a/apps/web/src/pages/HomeStub.tsx b/apps/web/src/pages/HomeStub.tsx deleted file mode 100644 index 63afc92..0000000 --- a/apps/web/src/pages/HomeStub.tsx +++ /dev/null @@ -1,29 +0,0 @@ -export function HomeStub() { - return ( -
-

- Code for Philly is being rebuilt -

-

- We are modernizing the platform. Check back soon for the full - experience. In the meantime, you can browse our projects below. -

-
- - Browse Projects - - - View Source - -
-
- ); -} diff --git a/apps/web/src/pages/LoginPlaceholder.tsx b/apps/web/src/pages/LoginPlaceholder.tsx index 5b82177..581185a 100644 --- a/apps/web/src/pages/LoginPlaceholder.tsx +++ b/apps/web/src/pages/LoginPlaceholder.tsx @@ -10,6 +10,7 @@ import { import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; +import { GitHubIcon } from '@/components/icons/GitHubIcon'; import { useAuth } from '@/hooks/useAuth'; import { api, ApiError } from '@/lib/api'; @@ -46,21 +47,6 @@ const ERROR_MESSAGES: Record = { ), }; -function GitHubIcon() { - return ( - - ); -} - function WhyGitHub() { const [open, setOpen] = useState(false); diff --git a/apps/web/src/screens/Volunteer.tsx b/apps/web/src/screens/Volunteer.tsx index 8b58cf9..458652f 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,7 +70,7 @@ export function Volunteer() { We meet weekly. Bring your laptop, or just yourself.

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..9f3c3c3 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,25 @@ describe('AppHeader', () => { it('renders primary nav links', async () => { renderWithRouter(); + 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(); + const gh = screen.getByRole('link', { name: 'Code for Philly on GitHub' }); + 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 +74,8 @@ describe('AppHeader', () => { const user = userEvent.setup(); renderWithRouter(); - 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 +91,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 +101,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 +110,64 @@ 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(); + + await user.click(screen.getByRole('button', { name: /open navigation menu/i })); + + const dialog = await screen.findByRole('dialog', { name: 'Menu' }); + expect(dialog).toBeInTheDocument(); + }); + + it('lists GitHub and Volunteer in the mobile sheet', async () => { + const user = userEvent.setup(); + renderWithRouter(); + + await user.click(screen.getByRole('button', { name: /open navigation menu/i })); + + const nav = await screen.findByRole('navigation', { name: /mobile navigation/i }); + expect(within(nav).getByRole('link', { name: 'GitHub' })).toHaveAttribute( + 'href', + 'https://github.com/CodeForPhilly', + ); + expect(within(nav).getByRole('link', { name: 'Volunteer' })).toHaveAttribute( + 'href', + '/volunteer', + ); + }); + + it('closes the mobile sheet when a sheet link navigates', async () => { + const user = userEvent.setup(); + renderWithRouter(); + + await user.click(screen.getByRole('button', { name: /open navigation menu/i })); + const nav = await screen.findByRole('navigation', { name: /mobile navigation/i }); + + await user.click(within(nav).getByRole('link', { name: 'Members' })); + + await waitFor(() => { + expect(screen.queryByRole('dialog', { name: 'Menu' })).not.toBeInTheDocument(); + }); + }); + + it('closes the mobile sheet when the inline search navigates', async () => { + const user = userEvent.setup(); + renderWithRouter(); + + await user.click(screen.getByRole('button', { name: /open navigation menu/i })); + const dialog = await screen.findByRole('dialog', { name: 'Menu' }); + + // Scope to the sheet: jsdom applies no breakpoints, so the desktop + // search box is in the DOM too. + await user.type( + within(dialog).getByRole('searchbox', { name: /search the site/i }), + 'civic{Enter}', + ); + + await waitFor(() => { + expect(screen.queryByRole('dialog', { name: 'Menu' })).not.toBeInTheDocument(); + }); + }); }); diff --git a/apps/web/tests/Volunteer.test.tsx b/apps/web/tests/Volunteer.test.tsx new file mode 100644 index 0000000..af17c99 --- /dev/null +++ b/apps/web/tests/Volunteer.test.tsx @@ -0,0 +1,90 @@ +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( + + + , + ); + } + + it('renders the hero headline', async () => { + renderVolunteer(); + expect( + screen.getByRole('heading', { + name: /volunteer with code for philly/i, + level: 1, + }), + ).toBeInTheDocument(); + // Let AuthProvider's /api/auth/me fetch settle before the test returns. + await screen.findByText(/browse 268 active projects/i); + }); + + it('points "When we meet →" at the live Meetup group, not the dead GitBook page', async () => { + 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'); + await screen.findByText(/browse 268 active projects/i); + }); + + it('points "Read the guide →" at the partnerships repo, not the dead GitBook page', async () => { + 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'); + await screen.findByText(/browse 268 active projects/i); + }); + + 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/docs/operations/cutover-announcement.md b/docs/operations/cutover-announcement.md index 6fd73cc..1adf51b 100644 --- a/docs/operations/cutover-announcement.md +++ b/docs/operations/cutover-announcement.md @@ -116,7 +116,7 @@ What you can do now: - Use the Slack workspace as normal; SSO is now backed by our own IdP Bugs / weirdness: file an issue on -https://github.com/CodeForPhilly/codeforphilly-rewrite/issues or just reply +https://github.com/CodeForPhilly/codeforphilly-ng/issues or just reply here. ``` diff --git a/plans/site-check-153.md b/plans/site-check-153.md new file mode 100644 index 0000000..599f5ed --- /dev/null +++ b/plans/site-check-153.md @@ -0,0 +1,228 @@ +--- +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 `