From 6fcfb709b49bec97137bd305a6562ad83520715e Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Mon, 1 Jun 2026 08:57:13 -0400 Subject: [PATCH] feat(web): persistent connect-GitHub banner under the navbar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hoists the "Connect your GitHub account" nag from the /account page to a top-level ConnectGitHubBanner rendered by AppShell directly under the navbar, so every page shows the prompt to legacy users until they link GitHub or dismiss for the session. Behavior unchanged from the per-page version: - Renders iff signed-in + hasGitHubLink === false + lastLoginMethod ∈ {legacy_password, password_reset} - Dismiss button hides it for the rest of the session - CTA submits a form to POST /api/auth/link-github Identity-card "Connect GitHub" form on /account stays — it's an in-context action for users who land on the settings page wanting to wire up the link, not a nag. Test split: new ConnectGitHubBanner.test.tsx covers the visibility matrix + dismiss; Account.test.tsx is trimmed to the identity-card branches. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/web/src/components/AppShell.tsx | 2 + .../src/components/ConnectGitHubBanner.tsx | 63 +++++++++ apps/web/src/screens/Account.tsx | 37 +---- apps/web/tests/Account.test.tsx | 66 ++------- apps/web/tests/ConnectGitHubBanner.test.tsx | 128 ++++++++++++++++++ 5 files changed, 202 insertions(+), 94 deletions(-) create mode 100644 apps/web/src/components/ConnectGitHubBanner.tsx create mode 100644 apps/web/tests/ConnectGitHubBanner.test.tsx diff --git a/apps/web/src/components/AppShell.tsx b/apps/web/src/components/AppShell.tsx index 88d0f6d..a7c6bd6 100644 --- a/apps/web/src/components/AppShell.tsx +++ b/apps/web/src/components/AppShell.tsx @@ -1,6 +1,7 @@ import { Outlet } from 'react-router'; import { AppHeader } from '@/components/AppHeader'; import { AppFooter } from '@/components/AppFooter'; +import { ConnectGitHubBanner } from '@/components/ConnectGitHubBanner'; import { OfflineBanner } from '@/components/OfflineBanner'; import { TopProgressBar } from '@/components/TopProgressBar'; @@ -18,6 +19,7 @@ export function AppShell() { +
diff --git a/apps/web/src/components/ConnectGitHubBanner.tsx b/apps/web/src/components/ConnectGitHubBanner.tsx new file mode 100644 index 0000000..1935971 --- /dev/null +++ b/apps/web/src/components/ConnectGitHubBanner.tsx @@ -0,0 +1,63 @@ +import { useState } from 'react'; +import { Button } from '@/components/ui/button'; +import { useAuth } from '@/hooks/useAuth'; + +/** + * Persistent nag banner shown directly under the navbar to legacy + * users (signed in via password) who haven't linked GitHub yet. + * + * Visibility rule per specs/screens/account.md + + * specs/behaviors/account-migration.md: + * + * person is signed in + * AND hasGitHubLink === false + * AND lastLoginMethod ∈ {legacy_password, password_reset} + * + * Dismissible per-session via in-memory state; reload brings it back + * (the nag is intentionally persistent across navigations to keep the + * "link your account" prompt visible until the user either links or + * dismisses). + */ +export function ConnectGitHubBanner() { + const { person, loading, hasGitHubLink, lastLoginMethod } = useAuth(); + const [dismissed, setDismissed] = useState(false); + + if (loading || !person) return null; + if (hasGitHubLink) return null; + if (lastLoginMethod !== 'legacy_password' && lastLoginMethod !== 'password_reset') { + return null; + } + if (dismissed) return null; + + return ( +
+
+

+ Connect your GitHub account + + {' '}— faster sign-in next time, and one less password to remember. + Code for Philly plans to retire password sign-in eventually. + +

+
+ +
+ +
+
+ ); +} diff --git a/apps/web/src/screens/Account.tsx b/apps/web/src/screens/Account.tsx index b808ffd..54f5f57 100644 --- a/apps/web/src/screens/Account.tsx +++ b/apps/web/src/screens/Account.tsx @@ -35,11 +35,10 @@ const LINK_GITHUB_ERROR_MESSAGES: Record = { }; export function Account() { - const { person, loading, signOut, reload, hasGitHubLink, lastLoginMethod } = useAuth(); + const { person, loading, signOut, reload, hasGitHubLink } = useAuth(); const navigate = useNavigate(); const queryClient = useQueryClient(); const [searchParams, setSearchParams] = useSearchParams(); - const [bannerDismissed, setBannerDismissed] = useState(false); // Toast on /account?linked=github or ?error= from the link-flow // callback, then strip the param so reloading doesn't re-toast. @@ -115,44 +114,10 @@ export function Account() { const sessions = sessionsQ.data?.data ?? []; - const showConnectGitHubBanner = - !bannerDismissed && - !hasGitHubLink && - (lastLoginMethod === 'legacy_password' || lastLoginMethod === 'password_reset'); - return (

Account Settings

- {showConnectGitHubBanner && ( -
-
-

Connect your GitHub account

-

- Faster sign-in next time, and one less password to remember. Code - for Philly plans to retire password sign-in eventually — link - GitHub now to stay ahead. -

-
-
- -
- -
- )} - {/* Identity */} diff --git a/apps/web/tests/Account.test.tsx b/apps/web/tests/Account.test.tsx index 9c98a6a..774e8e5 100644 --- a/apps/web/tests/Account.test.tsx +++ b/apps/web/tests/Account.test.tsx @@ -1,17 +1,11 @@ /** - * Account screen tests focused on the phase-D additions: - * - Connect-GitHub banner renders only for legacy-credential users - * who haven't linked yet. - * - Identity card swaps "Manage on GitHub" for "Connect GitHub" when - * hasGitHubLink is false. - * - Banner dismiss button hides it for the rest of the session. - * - * The existing Account page predates this test file — these tests focus - * narrowly on the banner + identity-card branches and leave the - * newsletter / sessions / claim-legacy regions alone. + * Account screen tests focused on the GitHub-link affordances inside + * the Identity card. The persistent "Connect GitHub" nag banner has + * been hoisted to a top-level ConnectGitHubBanner component (rendered + * by AppShell on every page), and is covered by its own test file. */ import { describe, expect, it, vi, afterEach } from 'vitest'; -import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { screen, waitFor } 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'; @@ -80,50 +74,6 @@ function render() { ); } -describe('Account — Connect-GitHub banner', () => { - afterEach(() => { - vi.restoreAllMocks(); - }); - - it('renders for a legacy-password user with no GitHub link', async () => { - mockApi(legacyPerson); - render(); - await waitFor(() => { - expect( - screen.getByRole('region', { name: /connect github/i }), - ).toBeInTheDocument(); - }); - // Banner has a Connect button + a Dismiss button - const region = screen.getByRole('region', { name: /connect github/i }); - expect(region.querySelector('form[action="/api/auth/link-github"]')).not.toBeNull(); - expect(screen.getByRole('button', { name: /dismiss/i })).toBeInTheDocument(); - }); - - it('does not render for a github-signed-in user', async () => { - mockApi(githubPerson); - render(); - // Wait for the Identity card to render — that proves the page is past loading. - await waitFor(() => { - expect(screen.getByText(/connected — primary identity/i)).toBeInTheDocument(); - }); - expect( - screen.queryByRole('region', { name: /connect github/i }), - ).not.toBeInTheDocument(); - }); - - it('dismiss button hides the banner for the rest of the session', async () => { - mockApi(legacyPerson); - render(); - const dismissBtn = await screen.findByRole('button', { name: /dismiss/i }); - fireEvent.click(dismissBtn); - await waitFor(() => { - expect( - screen.queryByRole('region', { name: /connect github/i }), - ).not.toBeInTheDocument(); - }); - }); -}); - describe('Account — Identity card', () => { afterEach(() => { vi.restoreAllMocks(); @@ -135,9 +85,9 @@ describe('Account — Identity card', () => { await waitFor(() => { expect(screen.getByText(/not connected/i)).toBeInTheDocument(); }); - // Two forms post to the link endpoint: banner + identity card + // Identity card has a form posting to the link endpoint. const forms = document.querySelectorAll('form[action="/api/auth/link-github"]'); - expect(forms.length).toBeGreaterThanOrEqual(2); + expect(forms.length).toBeGreaterThanOrEqual(1); }); it('shows the "Manage on GitHub" link when hasGitHubLink is true', async () => { @@ -149,7 +99,7 @@ describe('Account — Identity card', () => { expect( screen.getByRole('link', { name: /manage on github/i }), ).toHaveAttribute('href', 'https://github.com/settings'); - // No link-github form when already connected + // No link-github form when already connected. expect( document.querySelectorAll('form[action="/api/auth/link-github"]').length, ).toBe(0); diff --git a/apps/web/tests/ConnectGitHubBanner.test.tsx b/apps/web/tests/ConnectGitHubBanner.test.tsx new file mode 100644 index 0000000..6a21568 --- /dev/null +++ b/apps/web/tests/ConnectGitHubBanner.test.tsx @@ -0,0 +1,128 @@ +/** + * Tests for ConnectGitHubBanner — the persistent "Connect your GitHub + * account" nag rendered directly under the navbar on every page. + * + * Visibility rule: signed-in + hasGitHubLink === false + lastLoginMethod + * ∈ {legacy_password, password_reset} + not dismissed. + */ +import { describe, expect, it, vi, afterEach } from 'vitest'; +import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { renderScreen, mockOk } from './test-utils.js'; +import { ConnectGitHubBanner } from '../src/components/ConnectGitHubBanner.js'; +import { AuthProvider } from '../src/hooks/useAuth.js'; + +interface MeShape { + person: { id: string; slug: string; fullName: string; accountLevel: string; avatarUrl: string | null } | null; + accountLevel: string; + hasGitHubLink: boolean; + lastLoginMethod: 'github' | 'legacy_password' | 'password_reset' | null; +} + +function mockMe(me: MeShape): void { + vi.spyOn(globalThis, 'fetch').mockImplementation(((input: string) => { + if (input.startsWith('/api/auth/me')) { + return Promise.resolve( + new Response(JSON.stringify(mockOk(me)), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + } + return Promise.resolve(new Response(null, { status: 404 })); + }) as typeof fetch); +} + +const baseLegacyPerson: MeShape = { + person: { + id: '01951a3c-0000-7000-8000-0000ffffff01', + slug: 'legacy-user', + fullName: 'Legacy User', + accountLevel: 'user', + avatarUrl: null, + }, + accountLevel: 'user', + hasGitHubLink: false, + lastLoginMethod: 'legacy_password', +}; + +const anonMe: MeShape = { + person: null, + accountLevel: 'anonymous', + hasGitHubLink: false, + lastLoginMethod: null, +}; + +function render() { + return renderScreen( + + + , + ); +} + +describe('ConnectGitHubBanner', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('renders for legacy-password user with no GitHub link', async () => { + mockMe(baseLegacyPerson); + render(); + await waitFor(() => { + expect( + screen.getByRole('region', { name: /connect github/i }), + ).toBeInTheDocument(); + }); + // CTA form posts to the link endpoint. + const region = screen.getByRole('region', { name: /connect github/i }); + expect(region.querySelector('form[action="/api/auth/link-github"]')).not.toBeNull(); + expect(screen.getByRole('button', { name: /dismiss/i })).toBeInTheDocument(); + }); + + it('renders for a user whose session was minted via password reset', async () => { + mockMe({ ...baseLegacyPerson, lastLoginMethod: 'password_reset' }); + render(); + await waitFor(() => { + expect( + screen.getByRole('region', { name: /connect github/i }), + ).toBeInTheDocument(); + }); + }); + + it('does not render for a github-signed-in user', async () => { + mockMe({ + ...baseLegacyPerson, + hasGitHubLink: true, + lastLoginMethod: 'github', + }); + render(); + // Wait for /api/auth/me to settle (loading → resolved). The + // simplest signal is a tick: a known-true assertion plus a short + // microtask gap. + await new Promise((r) => setTimeout(r, 0)); + expect( + screen.queryByRole('region', { name: /connect github/i }), + ).not.toBeInTheDocument(); + }); + + it('does not render for anonymous viewers', async () => { + mockMe(anonMe); + render(); + await new Promise((r) => setTimeout(r, 0)); + expect( + screen.queryByRole('region', { name: /connect github/i }), + ).not.toBeInTheDocument(); + }); + + it('hides after the user clicks dismiss', async () => { + mockMe(baseLegacyPerson); + render(); + const dismissBtn = await screen.findByRole('button', { name: /dismiss/i }); + fireEvent.click(dismissBtn); + await waitFor(() => { + expect( + screen.queryByRole('region', { name: /connect github/i }), + ).not.toBeInTheDocument(); + }); + }); +});