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}

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..116d260 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,31 @@ 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', + ); + }); }); 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( + + + , + ); + } + + 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/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 `