From a595593d97795f305e8614de7b5f7968391d8643 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 25 Oct 2026 05:28:52 -0400 Subject: [PATCH 1/8] Add a country-aware Tools menu to the PolicyEngine header Zone apps render ui-kit's header, which had no Tools item, so a visitor on CliffWatch couldn't reach the OBBBA explorer from it. policyengine.org's own header gained a Tools menu in PolicyEngine/policyengine-app-v2#1421; this adds the same menu to getPolicyEngineNavItems, right after Research. US: CliffWatch, OBBBA household explorer, State legislative tracker. UK: UK CliffWatch (/uk/uk-cliff-watch). Others: no Tools item. Each list ends with "All interactive tools". Rows carry a new `external` flag that renders a plain , skipping linkComponent and onNavigate, because each tool is its own zone. Dropdown panels now size to their longest row (width: max-content), as #1421 did, so "OBBBA household explorer" stays on one line. ui-kit keeps a copy of app-v2's featuredTools.json because app-v2 is on ui-kit 0.4 and can't import it yet; a parity test reads app-v2's file and fails when the two differ, on every PR and weekly. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/featured-tools-parity.yml | 28 ++ changelog.d/tools-nav-menu.added.md | 1 + demo/Demo.tsx | 13 + package.json | 1 + src/layout/PolicyEngineFeaturedTools.ts | 54 ++++ src/layout/PolicyEngineSiteConfig.ts | 69 ++++- src/layout/header/Header.tsx | 14 +- src/layout/header/HeaderMobileMenu.tsx | 30 +- src/layout/header/HeaderNavItem.tsx | 13 +- src/layout/index.ts | 6 + tests/consumer-types/fixture.ts | 23 ++ tests/layout/PolicyEngineSiteConfig.test.ts | 298 ++++++++++++++++++++ tests/layout/PolicyEngineToolsNav.test.tsx | 225 +++++++++++++++ tests/layout/featuredToolsParity.test.ts | 34 +++ 14 files changed, 781 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/featured-tools-parity.yml create mode 100644 changelog.d/tools-nav-menu.added.md create mode 100644 src/layout/PolicyEngineFeaturedTools.ts create mode 100644 tests/layout/PolicyEngineSiteConfig.test.ts create mode 100644 tests/layout/PolicyEngineToolsNav.test.tsx create mode 100644 tests/layout/featuredToolsParity.test.ts diff --git a/.github/workflows/featured-tools-parity.yml b/.github/workflows/featured-tools-parity.yml new file mode 100644 index 0000000..5741334 --- /dev/null +++ b/.github/workflows/featured-tools-parity.yml @@ -0,0 +1,28 @@ +name: Featured tools parity + +# The header's Tools menu copies policyengine.org's list; see +# tests/layout/featuredToolsParity.test.ts. PR checks run that test too. This +# weekly run catches a tool added on policyengine.org while nothing changes +# here. +on: + schedule: + - cron: "0 14 * * 1" + workflow_dispatch: + +jobs: + parity: + name: Featured tools parity + runs-on: ubuntu-latest + + steps: + - name: Check out repository + uses: actions/checkout@v6 + + - name: Set up Bun + uses: oven-sh/setup-bun@v2 + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Compare with policyengine.org's list + run: bunx vitest run tests/layout/featuredToolsParity.test.ts diff --git a/changelog.d/tools-nav-menu.added.md b/changelog.d/tools-nav-menu.added.md new file mode 100644 index 0000000..8be35bb --- /dev/null +++ b/changelog.d/tools-nav-menu.added.md @@ -0,0 +1 @@ +Add a country-aware Tools menu to the PolicyEngine header, right after Research, matching policyengine.org's own header: CliffWatch, the OBBBA household explorer and the state legislative tracker for the US, UK CliffWatch for the UK, and no menu for other countries, each followed by "All interactive tools". Every row is a full page load into the tool's own zone. Also export `policyEngineFeaturedTools`, `getPolicyEngineFeaturedTools` and `getPolicyEngineAllToolsUrl`; add an `external` flag to `NavItemConfig` items and dropdown rows that renders a plain link, skipping `linkComponent` and `onNavigate`; and size header dropdowns to their longest row, so labels and descriptions no longer wrap at 220px. diff --git a/demo/Demo.tsx b/demo/Demo.tsx index 31b66cc..7ccd9f5 100644 --- a/demo/Demo.tsx +++ b/demo/Demo.tsx @@ -100,6 +100,7 @@ import { SegmentedControl } from '../src/primitives/SegmentedControl'; // Layout import { DashboardShell } from '../src/layout/DashboardShell'; import { Header } from '../src/layout/header'; +import { PolicyEngineHeader } from '../src/layout/PolicyEngineHeader'; import { SidebarLayout } from '../src/layout/SidebarLayout'; import { SingleColumnLayout } from '../src/layout/SingleColumnLayout'; import { InputPanel } from '../src/layout/InputPanel'; @@ -1292,6 +1293,18 @@ export function Demo() { + + {/* No overflow-hidden here: it would clip the dropdowns. */} +
+ +
+ + diff --git a/package.json b/package.json index 155dbe8..39f3081 100644 --- a/package.json +++ b/package.json @@ -139,6 +139,7 @@ "@types/react": "^19.0.0", "@types/react-dom": "^19.0.0", "@vitejs/plugin-react": "^4.3.4", + "fast-check": "^4.10.2", "jsdom": "^25.0.1", "react": "^19.0.0", "react-dom": "^19.0.0", diff --git a/src/layout/PolicyEngineFeaturedTools.ts b/src/layout/PolicyEngineFeaturedTools.ts new file mode 100644 index 0000000..769e6b6 --- /dev/null +++ b/src/layout/PolicyEngineFeaturedTools.ts @@ -0,0 +1,54 @@ +/** + * An interactive tool listed in the PolicyEngine header's Tools menu. + * + * Each tool is its own zone on policyengine.org at `//`, so a + * link to one must be a full page load, never a client-side transition. + */ +export interface PolicyEngineFeaturedTool { + /** Path segment after the country: the tool lives at `//`. */ + slug: string; + label: string; + description: string; +} + +/** + * The featured tools for each country, in menu order. + * + * This mirrors `app/src/data/featuredTools.json` in + * PolicyEngine/policyengine-app-v2, which drives policyengine.org's own + * header and homepage. `tests/layout/featuredToolsParity.test.ts` fails when + * the two lists differ, so change both together. A country with no entry has + * no Tools menu. + */ +export const policyEngineFeaturedTools: Readonly< + Record +> = { + us: [ + { + slug: 'cliffwatch', + label: 'CliffWatch', + description: + "See where benefits, refundable tax credits, and taxes create cliffs as a household's wages rise, in all 50 states and DC.", + }, + { + slug: 'obbba-households', + label: 'OBBBA household explorer', + description: + "See how the One Big Beautiful Bill Act changes each modeled household's resources in 2026, and which provisions drive the change.", + }, + { + slug: 'bill-tracker', + label: 'State legislative tracker', + description: + "Monitor tax and benefit legislation across all 50 US states. See which states are in session, track active bills, and explore PolicyEngine's research on state-level policy reforms.", + }, + ], + uk: [ + { + slug: 'uk-cliff-watch', + label: 'UK CliffWatch', + description: + "See where benefit withdrawal and taxes create cliffs and high marginal rates as a UK household's earnings rise.", + }, + ], +}; diff --git a/src/layout/PolicyEngineSiteConfig.ts b/src/layout/PolicyEngineSiteConfig.ts index 17c1ade..6304673 100644 --- a/src/layout/PolicyEngineSiteConfig.ts +++ b/src/layout/PolicyEngineSiteConfig.ts @@ -1,5 +1,9 @@ import type { CountryConfig, NavItemConfig } from './header'; import type { FooterProps } from './Footer'; +import { + policyEngineFeaturedTools, + type PolicyEngineFeaturedTool, +} from './PolicyEngineFeaturedTools'; export type PolicyEngineCountryId = 'us' | 'uk' | (string & {}); @@ -59,15 +63,61 @@ export function getPolicyEngineUrl( return `${getPolicyEngineCountryUrl(country, baseUrl)}${normalizedPath}`; } +/** The featured tools for a country, with absolute links, or none. */ +export function getPolicyEngineFeaturedTools( + country: PolicyEngineCountryId = 'us', + baseUrl = DEFAULT_POLICYENGINE_BASE_URL, +): (PolicyEngineFeaturedTool & { href: string })[] { + const tools = Object.hasOwn(policyEngineFeaturedTools, country) + ? policyEngineFeaturedTools[country] + : []; + return tools.map((tool) => ({ + ...tool, + href: getPolicyEngineUrl(country, tool.slug, baseUrl), + })); +} + +/** The research listing filtered to interactive tools. */ +export function getPolicyEngineAllToolsUrl( + country: PolicyEngineCountryId = 'us', + baseUrl = DEFAULT_POLICYENGINE_BASE_URL, +) { + return getPolicyEngineUrl(country, 'research?types=interactive', baseUrl); +} + +function getPolicyEngineToolsNavItem( + country: PolicyEngineCountryId, + baseUrl: string, +): NavItemConfig | null { + const tools = getPolicyEngineFeaturedTools(country, baseUrl); + if (tools.length === 0) { + return null; + } + + const allToolsUrl = getPolicyEngineAllToolsUrl(country, baseUrl); + // Each tool is its own zone, and so is the research listing, so every row + // is a full page load rather than a client-side transition. + return { + label: 'Tools', + href: allToolsUrl, + children: [ + ...tools.map(({ label, href }) => ({ label, href, external: true })), + { label: 'All interactive tools', href: allToolsUrl, external: true }, + ], + }; +} + export function getPolicyEngineNavItems( country: PolicyEngineCountryId = 'us', baseUrl = DEFAULT_POLICYENGINE_BASE_URL, ): NavItemConfig[] { - const navItems: NavItemConfig[] = [ + const toolsNavItem = getPolicyEngineToolsNavItem(country, baseUrl); + return [ { label: 'Research', href: getPolicyEngineUrl(country, 'research', baseUrl), }, + ...(toolsNavItem ? [toolsNavItem] : []), { label: 'Model', href: getPolicyEngineUrl(country, 'model', baseUrl), @@ -76,6 +126,14 @@ export function getPolicyEngineNavItems( label: 'API', href: getPolicyEngineUrl(country, 'api', baseUrl), }, + ...(country === 'us' + ? [ + { + label: 'Python', + href: getPolicyEngineUrl(country, 'python', baseUrl), + }, + ] + : []), { label: 'About', href: getPolicyEngineUrl(country, 'team', baseUrl), @@ -102,15 +160,6 @@ export function getPolicyEngineNavItems( href: getPolicyEngineUrl(country, 'donate', baseUrl), }, ]; - - if (country === 'us') { - navItems.splice(3, 0, { - label: 'Python', - href: getPolicyEngineUrl(country, 'python', baseUrl), - }); - } - - return navItems; } export function getPolicyEngineFooterLinks( diff --git a/src/layout/header/Header.tsx b/src/layout/header/Header.tsx index 5684383..37c2b80 100644 --- a/src/layout/header/Header.tsx +++ b/src/layout/header/Header.tsx @@ -5,7 +5,19 @@ import { HeaderContent } from './HeaderContent'; export interface NavItemConfig { label: string; href: string; - children?: { label: string; href: string; description?: string }[]; + /** + * Render a plain `` that the browser follows as a full page load, + * skipping `linkComponent` and `onNavigate`. Use it for a page served by + * another zone, which a client-side router can't reach. + */ + external?: boolean; + children?: { + label: string; + href: string; + description?: string; + /** Same as the item-level `external`, for one dropdown row. */ + external?: boolean; + }[]; } export interface CountryConfig { diff --git a/src/layout/header/HeaderMobileMenu.tsx b/src/layout/header/HeaderMobileMenu.tsx index 1e87759..75224b9 100644 --- a/src/layout/header/HeaderMobileMenu.tsx +++ b/src/layout/header/HeaderMobileMenu.tsx @@ -99,7 +99,7 @@ export function HeaderMobileMenu({ style={{ gap: '8px', paddingLeft: '16px' }} > {item.children.map((child) => { - if (linkComponent) { + if (linkComponent && !child.external) { const LinkComp = linkComponent; return ( { - e.preventDefault(); - handleNavigate(child.href); - }} + onClick={ + child.external + ? onClose + : (e) => { + e.preventDefault(); + handleNavigate(child.href); + } + } style={{ color: 'var(--text-inverse)', textDecoration: 'none', @@ -143,7 +147,7 @@ export function HeaderMobileMenu({ ) : ( (() => { - if (linkComponent) { + if (linkComponent && !item.external) { const LinkComp = linkComponent; return ( { - e.preventDefault(); - handleNavigate(item.href); - } - : undefined + item.external + ? onClose + : onNavigate + ? (e: React.MouseEvent) => { + e.preventDefault(); + handleNavigate(item.href); + } + : undefined } style={{ color: 'var(--text-inverse)', diff --git a/src/layout/header/HeaderNavItem.tsx b/src/layout/header/HeaderNavItem.tsx index ae112af..8d47847 100644 --- a/src/layout/header/HeaderNavItem.tsx +++ b/src/layout/header/HeaderNavItem.tsx @@ -108,6 +108,9 @@ function AppleDropdown({ ? 'translateX(-50%) translateY(0)' : 'translateX(-50%) translateY(-8px)', marginTop: '10px', + // The trigger is a narrow box, so size the panel to its longest row + // instead of wrapping every row at the minimum width. + width: 'max-content', minWidth: '220px', overflow: 'hidden', maxHeight: visible ? `${contentHeight}px` : '0px', @@ -150,13 +153,13 @@ function AppleDropdown({ ); const handleClick = (e: React.MouseEvent) => { onClose(); - if (onNavigate) { + if (onNavigate && !item.external) { e.preventDefault(); onNavigate(item.href); } }; - if (linkComponent) { + if (linkComponent && !item.external) { const LinkComp = linkComponent; return ( (null); @@ -267,7 +270,7 @@ export function HeaderNavItem({ } // Simple link item - if (linkComponent) { + if (linkComponent && !external) { const LinkComp = linkComponent; return ( { e.preventDefault(); onNavigate(href); diff --git a/src/layout/index.ts b/src/layout/index.ts index 3cffa2e..cccf0eb 100644 --- a/src/layout/index.ts +++ b/src/layout/index.ts @@ -24,14 +24,20 @@ export { } from './PolicyEngineShell'; export { DEFAULT_POLICYENGINE_BASE_URL, + getPolicyEngineAllToolsUrl, getPolicyEngineCountrySwitchUrl, getPolicyEngineCountryUrl, + getPolicyEngineFeaturedTools, getPolicyEngineFooterLinks, getPolicyEngineNavItems, getPolicyEngineUrl, policyEngineCountries, type PolicyEngineCountryId, } from './PolicyEngineSiteConfig'; +export { + policyEngineFeaturedTools, + type PolicyEngineFeaturedTool, +} from './PolicyEngineFeaturedTools'; export { Header, type HeaderProps, diff --git a/tests/consumer-types/fixture.ts b/tests/consumer-types/fixture.ts index 7521c3c..e504ff9 100644 --- a/tests/consumer-types/fixture.ts +++ b/tests/consumer-types/fixture.ts @@ -47,6 +47,13 @@ import { Footer, Stack, Group, + PolicyEngineHeader, + getPolicyEngineNavItems, + getPolicyEngineFeaturedTools, + getPolicyEngineAllToolsUrl, + policyEngineFeaturedTools, + type NavItemConfig, + type PolicyEngineFeaturedTool, // Inputs CurrencyInput, NumberInput, @@ -108,6 +115,18 @@ export const _LegacyColorPaths = { grayMid: legacyColors.gray[500], }; +// The Tools menu surface zone apps read (0.13.0): the item-level and +// dropdown-row `external` flags and the featured-tool entries. +export const _ToolsNavPaths: { + external: NavItemConfig["external"]; + rowExternal: NonNullable[number]["external"]; + tool: PolicyEngineFeaturedTool; +} = { + external: true, + rowExternal: true, + tool: policyEngineFeaturedTools.us[0], +}; + // Touch each binding so unused-import settings don't strip them. export type _SmokeTest = | typeof Accordion @@ -142,6 +161,10 @@ export type _SmokeTest = | typeof Footer | typeof Stack | typeof Group + | typeof PolicyEngineHeader + | typeof getPolicyEngineNavItems + | typeof getPolicyEngineFeaturedTools + | typeof getPolicyEngineAllToolsUrl | typeof CurrencyInput | typeof NumberInput | typeof DataTable diff --git a/tests/layout/PolicyEngineSiteConfig.test.ts b/tests/layout/PolicyEngineSiteConfig.test.ts new file mode 100644 index 0000000..a6df52f --- /dev/null +++ b/tests/layout/PolicyEngineSiteConfig.test.ts @@ -0,0 +1,298 @@ +import { describe, expect, it } from 'vitest'; +import fc from 'fast-check'; +import type { NavItemConfig } from '../../src/layout/header'; +import { policyEngineFeaturedTools } from '../../src/layout/PolicyEngineFeaturedTools'; +import { + DEFAULT_POLICYENGINE_BASE_URL, + getPolicyEngineAllToolsUrl, + getPolicyEngineFeaturedTools, + getPolicyEngineNavItems, + getPolicyEngineUrl, +} from '../../src/layout/PolicyEngineSiteConfig'; + +/** + * The nav as it was in 0.12.1, before the Tools menu. Adding Tools must leave + * every other item, and its position relative to the others, unchanged. + */ +function navItemsWithoutTools(country: string, baseUrl: string): NavItemConfig[] { + const navItems: NavItemConfig[] = [ + { label: 'Research', href: getPolicyEngineUrl(country, 'research', baseUrl) }, + { label: 'Model', href: getPolicyEngineUrl(country, 'model', baseUrl) }, + { label: 'API', href: getPolicyEngineUrl(country, 'api', baseUrl) }, + { + label: 'About', + href: getPolicyEngineUrl(country, 'team', baseUrl), + children: [ + { + label: 'Team', + href: getPolicyEngineUrl(country, 'team', baseUrl), + description: 'Meet the PolicyEngine team', + }, + { + label: 'Supporters', + href: getPolicyEngineUrl(country, 'supporters', baseUrl), + description: 'Organizations supporting PolicyEngine', + }, + { + label: 'Citations', + href: getPolicyEngineUrl(country, 'citations', baseUrl), + description: 'How to cite PolicyEngine', + }, + ], + }, + { label: 'Donate', href: getPolicyEngineUrl(country, 'donate', baseUrl) }, + ]; + if (country === 'us') { + navItems.splice(3, 0, { + label: 'Python', + href: getPolicyEngineUrl(country, 'python', baseUrl), + }); + } + return navItems; +} + +function toolsItem(navItems: NavItemConfig[]) { + return navItems.find((item) => item.label === 'Tools'); +} + +const countries = fc.oneof( + fc.constantFrom('us', 'uk'), + // Other countries, including keys every object inherits. + fc.constantFrom('ca', 'ng', 'il', '', 'US', 'constructor', '__proto__', 'toString'), + fc.string(), +); + +// Origins as consumers pass them: scheme, host, optional port, any number of +// trailing slashes (which the site config strips). +const baseUrls = fc + .record({ + scheme: fc.constantFrom('http', 'https'), + host: fc.domain(), + port: fc.option(fc.integer({ min: 1, max: 65535 }), { nil: undefined }), + slashes: fc.integer({ min: 0, max: 3 }), + }) + .map( + ({ scheme, host, port, slashes }) => + `${scheme}://${host}${port === undefined ? '' : `:${port}`}${'/'.repeat(slashes)}`, + ); + +describe('PolicyEngine Tools menu', () => { + it('lists the US tools right after Research', () => { + const navItems = getPolicyEngineNavItems('us'); + + expect(navItems.map((item) => item.label)).toEqual([ + 'Research', + 'Tools', + 'Model', + 'API', + 'Python', + 'About', + 'Donate', + ]); + expect(toolsItem(navItems)).toEqual({ + label: 'Tools', + href: 'https://policyengine.org/us/research?types=interactive', + children: [ + { + label: 'CliffWatch', + href: 'https://policyengine.org/us/cliffwatch', + external: true, + }, + { + label: 'OBBBA household explorer', + href: 'https://policyengine.org/us/obbba-households', + external: true, + }, + { + label: 'State legislative tracker', + href: 'https://policyengine.org/us/bill-tracker', + external: true, + }, + { + label: 'All interactive tools', + href: 'https://policyengine.org/us/research?types=interactive', + external: true, + }, + ], + }); + }); + + it('lists UK CliffWatch at /uk/uk-cliff-watch for the UK', () => { + const navItems = getPolicyEngineNavItems('uk'); + + expect(navItems.map((item) => item.label)).toEqual([ + 'Research', + 'Tools', + 'Model', + 'API', + 'About', + 'Donate', + ]); + expect(toolsItem(navItems)).toEqual({ + label: 'Tools', + href: 'https://policyengine.org/uk/research?types=interactive', + children: [ + { + label: 'UK CliffWatch', + href: 'https://policyengine.org/uk/uk-cliff-watch', + external: true, + }, + { + label: 'All interactive tools', + href: 'https://policyengine.org/uk/research?types=interactive', + external: true, + }, + ], + }); + }); + + it.each(['ca', 'ng', 'il', 'US', '', 'constructor', '__proto__'])( + 'has no Tools menu for country %j', + (country) => { + expect(toolsItem(getPolicyEngineNavItems(country))).toBeUndefined(); + expect(getPolicyEngineNavItems(country)).toEqual( + navItemsWithoutTools(country, DEFAULT_POLICYENGINE_BASE_URL), + ); + expect(getPolicyEngineFeaturedTools(country)).toEqual([]); + }, + ); + + it('builds tool links on a custom base URL', () => { + expect( + getPolicyEngineFeaturedTools('uk', 'https://staging.policyengine.org//'), + ).toEqual([ + { + slug: 'uk-cliff-watch', + label: 'UK CliffWatch', + description: policyEngineFeaturedTools.uk[0].description, + href: 'https://staging.policyengine.org/uk/uk-cliff-watch', + }, + ]); + expect( + getPolicyEngineAllToolsUrl('us', 'https://staging.policyengine.org/'), + ).toBe('https://staging.policyengine.org/us/research?types=interactive'); + }); + + it('defaults to the US on policyengine.org', () => { + expect(getPolicyEngineFeaturedTools()).toEqual( + getPolicyEngineFeaturedTools('us', DEFAULT_POLICYENGINE_BASE_URL), + ); + expect(getPolicyEngineAllToolsUrl()).toBe( + 'https://policyengine.org/us/research?types=interactive', + ); + }); + + it('keeps every tool entry well formed', () => { + for (const [country, tools] of Object.entries(policyEngineFeaturedTools)) { + expect(tools.length, country).toBeGreaterThan(0); + for (const tool of tools) { + expect(tool.slug).toMatch(/^[a-z0-9]+(-[a-z0-9]+)*$/); + expect(tool.label.trim()).not.toBe(''); + expect(tool.description.trim()).not.toBe(''); + } + expect(new Set(tools.map((tool) => tool.slug)).size).toBe(tools.length); + expect(new Set(tools.map((tool) => tool.label)).size).toBe(tools.length); + expect(tools.some((tool) => tool.label === 'All interactive tools')).toBe( + false, + ); + } + }); +}); + +describe('PolicyEngine Tools menu properties', () => { + it('appears only for a country with tools, and only right after Research', () => { + fc.assert( + fc.property(countries, baseUrls, (country, baseUrl) => { + const navItems = getPolicyEngineNavItems(country, baseUrl); + const hasTools = country === 'us' || country === 'uk'; + + expect(navItems.filter((item) => item.label === 'Tools')).toHaveLength( + hasTools ? 1 : 0, + ); + expect(navItems[0].label).toBe('Research'); + if (hasTools) { + expect(navItems[1].label).toBe('Tools'); + } + }), + ); + }); + + it('leaves every other nav item as it was before Tools', () => { + fc.assert( + fc.property(countries, baseUrls, (country, baseUrl) => { + expect( + getPolicyEngineNavItems(country, baseUrl).filter( + (item) => item.label !== 'Tools', + ), + ).toEqual(navItemsWithoutTools(country, baseUrl)); + }), + ); + }); + + it('links every row to a full-page-load URL under the country on the base URL', () => { + fc.assert( + fc.property(fc.constantFrom('us', 'uk'), baseUrls, (country, baseUrl) => { + const origin = baseUrl.replace(/\/+$/, ''); + const tools = toolsItem(getPolicyEngineNavItems(country, baseUrl)); + const rows = tools?.children ?? []; + const expectedTools = policyEngineFeaturedTools[country]; + + expect(rows.map((row) => row.label)).toEqual([ + ...expectedTools.map((tool) => tool.label), + 'All interactive tools', + ]); + expect(new Set(rows.map((row) => row.href)).size).toBe(rows.length); + for (const row of rows) { + expect(row.external).toBe(true); + expect(row.href.startsWith(`${origin}/${country}/`)).toBe(true); + const url = new URL(row.href); + expect(`${url.protocol}//${url.host}`).toBe(new URL(origin).origin); + } + rows.slice(0, -1).forEach((row, i) => { + expect(new URL(row.href).pathname).toBe( + `/${country}/${expectedTools[i].slug}`, + ); + expect(new URL(row.href).search).toBe(''); + }); + const allTools = new URL(rows[rows.length - 1].href); + expect(allTools.pathname).toBe(`/${country}/research`); + expect(allTools.searchParams.get('types')).toBe('interactive'); + expect(tools?.href).toBe(rows[rows.length - 1].href); + }), + ); + }); + + it('never sends the UK menu to /uk/cliffwatch', () => { + fc.assert( + fc.property(baseUrls, (baseUrl) => { + const paths = ( + toolsItem(getPolicyEngineNavItems('uk', baseUrl))?.children ?? [] + ).map((row) => new URL(row.href).pathname); + + expect(paths).toContain('/uk/uk-cliff-watch'); + expect(paths.some((path) => /^\/uk\/cliffwatch(\/|$)/.test(path))).toBe( + false, + ); + }), + ); + }); + + it('returns fresh, equal results on every call', () => { + fc.assert( + fc.property(countries, baseUrls, (country, baseUrl) => { + const first = getPolicyEngineFeaturedTools(country, baseUrl); + const second = getPolicyEngineFeaturedTools(country, baseUrl); + expect(second).toEqual(first); + + for (const tool of first) { + tool.slug = 'mutated'; + tool.href = 'mutated'; + } + expect(getPolicyEngineFeaturedTools(country, baseUrl)).toEqual(second); + expect(getPolicyEngineNavItems(country, baseUrl)).toEqual( + getPolicyEngineNavItems(country, baseUrl), + ); + }), + ); + }); +}); diff --git a/tests/layout/PolicyEngineToolsNav.test.tsx b/tests/layout/PolicyEngineToolsNav.test.tsx new file mode 100644 index 0000000..befa078 --- /dev/null +++ b/tests/layout/PolicyEngineToolsNav.test.tsx @@ -0,0 +1,225 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { render, screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { Header } from '../../src/layout/header'; +import { PolicyEngineHeader } from '../../src/layout/PolicyEngineHeader'; + +/** A client-side link: marks what it renders so tests can tell it apart. */ +function ClientLink({ + children, + href, + to: _to, + ...props +}: React.AnchorHTMLAttributes & { href: string; to?: string }) { + return ( + + {children} + + ); +} + +// Records whether the header cancelled each click, then cancels it so jsdom +// doesn't try to navigate. Listeners on document run after React's handlers. +let clickWasPrevented: boolean[] = []; +function recordClick(event: MouseEvent) { + if ((event.target as Element | null)?.closest('a')) { + clickWasPrevented.push(event.defaultPrevented); + event.preventDefault(); + } +} + +beforeEach(() => { + clickWasPrevented = []; + document.addEventListener('click', recordClick); +}); + +afterEach(() => { + document.removeEventListener('click', recordClick); +}); + +async function openDesktopMenu(label: string) { + await userEvent.click(screen.getByRole('button', { name: label })); +} + +async function openMobileMenu() { + await userEvent.click(screen.getByRole('button', { name: 'Toggle navigation' })); + return screen.getByRole('dialog'); +} + +describe('PolicyEngineHeader Tools menu', () => { + it('opens the US tools as plain links even with a router link and onNavigate', async () => { + const onNavigate = vi.fn(); + render( + , + ); + + await openDesktopMenu('Tools'); + const rows = [ + ['CliffWatch', 'https://policyengine.org/us/cliffwatch'], + ['OBBBA household explorer', 'https://policyengine.org/us/obbba-households'], + ['State legislative tracker', 'https://policyengine.org/us/bill-tracker'], + ['All interactive tools', 'https://policyengine.org/us/research?types=interactive'], + ]; + for (const [name, href] of rows) { + const link = screen.getByRole('link', { name }); + expect(link).toHaveAttribute('href', href); + expect(link).not.toHaveAttribute('data-client-link'); + } + + await userEvent.click(screen.getByRole('link', { name: 'OBBBA household explorer' })); + expect(onNavigate).not.toHaveBeenCalled(); + expect(clickWasPrevented).toEqual([false]); + }); + + it('still routes the other dropdowns through the link component and onNavigate', async () => { + const onNavigate = vi.fn(); + render( + , + ); + + await openDesktopMenu('About'); + const team = screen.getByRole('link', { name: /Team/ }); + expect(team).toHaveAttribute('data-client-link'); + + await userEvent.click(team); + expect(onNavigate).toHaveBeenCalledWith('https://policyengine.org/us/team'); + expect(clickWasPrevented).toEqual([true]); + }); + + it('sizes the dropdown to its longest row so labels stay on one line', async () => { + render(); + + await openDesktopMenu('Tools'); + const panel = screen + .getByRole('link', { name: 'OBBBA household explorer' }) + .closest('div[style*="position: absolute"]'); + expect(panel?.style.width).toBe('max-content'); + expect(panel?.style.minWidth).toBe('220px'); + }); + + it('lists only UK CliffWatch and the full list for the UK', async () => { + render(); + + await openDesktopMenu('Tools'); + expect(screen.getByRole('link', { name: 'UK CliffWatch' })).toHaveAttribute( + 'href', + 'https://policyengine.org/uk/uk-cliff-watch', + ); + expect( + screen.getByRole('link', { name: 'All interactive tools' }), + ).toHaveAttribute('href', 'https://policyengine.org/uk/research?types=interactive'); + expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(); + expect(screen.queryByRole('link', { name: /OBBBA/ })).toBeNull(); + for (const link of document.querySelectorAll('a')) { + expect(link.getAttribute('href')).not.toMatch(/\/uk\/cliffwatch(\/|\?|$)/); + } + }); + + it('has no Tools menu for another country', async () => { + render(); + + expect(screen.queryByRole('button', { name: 'Tools' })).toBeNull(); + const menu = await openMobileMenu(); + expect(within(menu).queryByText('Tools')).toBeNull(); + expect(within(menu).getByText('Research')).toBeInTheDocument(); + }); + + it('lists the tools as plain links in the mobile menu', async () => { + const onNavigate = vi.fn(); + render( + , + ); + + const menu = await openMobileMenu(); + expect(within(menu).getByText('Tools')).toBeInTheDocument(); + const cliffWatch = within(menu).getByRole('link', { name: 'CliffWatch' }); + expect(cliffWatch).toHaveAttribute('href', 'https://policyengine.org/us/cliffwatch'); + expect(cliffWatch).not.toHaveAttribute('data-client-link'); + expect(within(menu).getByRole('link', { name: 'Team' })).toHaveAttribute( + 'data-client-link', + ); + + await userEvent.click(cliffWatch); + expect(onNavigate).not.toHaveBeenCalled(); + expect(clickWasPrevented).toEqual([false]); + expect(screen.queryByRole('dialog')).toBeNull(); + }); + + it('leaves mobile tool links to the browser when there is no router', async () => { + const onNavigate = vi.fn(); + render(); + + const menu = await openMobileMenu(); + await userEvent.click(within(menu).getByRole('link', { name: 'UK CliffWatch' })); + expect(onNavigate).not.toHaveBeenCalled(); + expect(clickWasPrevented).toEqual([false]); + + const menuAgain = await openMobileMenu(); + await userEvent.click(within(menuAgain).getByRole('link', { name: 'Team' })); + expect(onNavigate).toHaveBeenCalledWith('https://policyengine.org/uk/team'); + expect(clickWasPrevented).toEqual([false, true]); + }); +}); + +describe('Header external items', () => { + const navItems = [ + { label: 'Zone page', href: 'https://example.org/zone', external: true }, + { label: 'Local page', href: '/local' }, + ]; + + it('renders a top-level external item as a plain link on desktop', async () => { + const onNavigate = vi.fn(); + render( +
, + ); + + const nav = document.querySelector('.hidden.lg\\:flex')!; + const zone = within(nav).getByRole('link', { name: 'Zone page' }); + expect(zone).not.toHaveAttribute('data-client-link'); + expect(within(nav).getByRole('link', { name: 'Local page' })).toHaveAttribute( + 'data-client-link', + ); + + await userEvent.click(zone); + expect(onNavigate).not.toHaveBeenCalled(); + expect(clickWasPrevented).toEqual([false]); + }); + + it('renders a top-level external item as a plain link on mobile', async () => { + const { unmount } = render( +
, + ); + const routedMenu = await openMobileMenu(); + expect( + within(routedMenu).getByRole('link', { name: 'Zone page' }), + ).not.toHaveAttribute('data-client-link'); + expect( + within(routedMenu).getByRole('link', { name: 'Local page' }), + ).toHaveAttribute('data-client-link'); + unmount(); + + const onNavigate = vi.fn(); + render(
); + + const menu = await openMobileMenu(); + await userEvent.click(within(menu).getByRole('link', { name: 'Zone page' })); + expect(onNavigate).not.toHaveBeenCalled(); + + const menuAgain = await openMobileMenu(); + await userEvent.click(within(menuAgain).getByRole('link', { name: 'Local page' })); + expect(onNavigate).toHaveBeenCalledWith('/local'); + expect(clickWasPrevented).toEqual([false, true]); + }); +}); diff --git a/tests/layout/featuredToolsParity.test.ts b/tests/layout/featuredToolsParity.test.ts new file mode 100644 index 0000000..6501679 --- /dev/null +++ b/tests/layout/featuredToolsParity.test.ts @@ -0,0 +1,34 @@ +// @vitest-environment node +import { describe, expect, it } from 'vitest'; +import { policyEngineFeaturedTools } from '../../src/layout/PolicyEngineFeaturedTools'; + +/** + * policyengine.org's own header and homepage read their Tools list from this + * file in PolicyEngine/policyengine-app-v2. ui-kit keeps a copy because + * app-v2 is still on ui-kit 0.4, so it can't import the list from here yet. + * Once app-v2 reads it from ui-kit and deletes its own file, delete this test. + * + * This reads app-v2's main branch over the network, so it also runs on a + * weekly schedule (.github/workflows/featured-tools-parity.yml) to catch a + * tool added on policyengine.org when nothing changes here. + */ +const APP_V2_FEATURED_TOOLS_URL = + 'https://raw.githubusercontent.com/PolicyEngine/policyengine-app-v2/main/app/src/data/featuredTools.json'; + +describe('featured tools parity with policyengine.org', () => { + it('matches app-v2’s featuredTools.json exactly', async () => { + const response = await fetch(APP_V2_FEATURED_TOOLS_URL, { + signal: AbortSignal.timeout(15_000), + }); + expect(response.ok, `GET ${APP_V2_FEATURED_TOOLS_URL}: ${response.status}`).toBe( + true, + ); + + expect( + policyEngineFeaturedTools, + 'src/layout/PolicyEngineFeaturedTools.ts differs from app-v2’s ' + + 'app/src/data/featuredTools.json; change them together so zone ' + + 'apps and policyengine.org show the same Tools menu', + ).toStrictEqual(await response.json()); + }, 20_000); +}); From 1c6a33b7aec44b173c291b01e7a0272a7f2a9838 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 25 Oct 2026 05:48:46 -0400 Subject: [PATCH 2/8] fixup! Add a country-aware Tools menu to the PolicyEngine header --- src/layout/PolicyEngineFeaturedTools.ts | 29 +++++++++++++-------- src/layout/PolicyEngineSiteConfig.ts | 10 +++++--- src/layout/header/Header.tsx | 34 ++++++++++++++++--------- src/layout/header/HeaderMobileMenu.tsx | 14 +++++++--- src/layout/header/HeaderNavItem.tsx | 9 +++++-- src/layout/header/index.ts | 7 ++++- src/layout/index.ts | 1 + 7 files changed, 71 insertions(+), 33 deletions(-) diff --git a/src/layout/PolicyEngineFeaturedTools.ts b/src/layout/PolicyEngineFeaturedTools.ts index 769e6b6..26d5ab6 100644 --- a/src/layout/PolicyEngineFeaturedTools.ts +++ b/src/layout/PolicyEngineFeaturedTools.ts @@ -6,9 +6,15 @@ */ export interface PolicyEngineFeaturedTool { /** Path segment after the country: the tool lives at `//`. */ - slug: string; - label: string; - description: string; + readonly slug: string; + readonly label: string; + readonly description: string; +} + +function freezeTools( + tools: PolicyEngineFeaturedTool[], +): readonly PolicyEngineFeaturedTool[] { + return Object.freeze(tools.map((tool) => Object.freeze({ ...tool }))); } /** @@ -18,12 +24,13 @@ export interface PolicyEngineFeaturedTool { * PolicyEngine/policyengine-app-v2, which drives policyengine.org's own * header and homepage. `tests/layout/featuredToolsParity.test.ts` fails when * the two lists differ, so change both together. A country with no entry has - * no Tools menu. + * no Tools menu. The data is frozen because every header in a bundle shares + * it. */ export const policyEngineFeaturedTools: Readonly< - Record -> = { - us: [ + Partial> +> = Object.freeze({ + us: freezeTools([ { slug: 'cliffwatch', label: 'CliffWatch', @@ -42,13 +49,13 @@ export const policyEngineFeaturedTools: Readonly< description: "Monitor tax and benefit legislation across all 50 US states. See which states are in session, track active bills, and explore PolicyEngine's research on state-level policy reforms.", }, - ], - uk: [ + ]), + uk: freezeTools([ { slug: 'uk-cliff-watch', label: 'UK CliffWatch', description: "See where benefit withdrawal and taxes create cliffs and high marginal rates as a UK household's earnings rise.", }, - ], -}; + ]), +}); diff --git a/src/layout/PolicyEngineSiteConfig.ts b/src/layout/PolicyEngineSiteConfig.ts index 6304673..c467ded 100644 --- a/src/layout/PolicyEngineSiteConfig.ts +++ b/src/layout/PolicyEngineSiteConfig.ts @@ -67,9 +67,9 @@ export function getPolicyEngineUrl( export function getPolicyEngineFeaturedTools( country: PolicyEngineCountryId = 'us', baseUrl = DEFAULT_POLICYENGINE_BASE_URL, -): (PolicyEngineFeaturedTool & { href: string })[] { +): (PolicyEngineFeaturedTool & { readonly href: string })[] { const tools = Object.hasOwn(policyEngineFeaturedTools, country) - ? policyEngineFeaturedTools[country] + ? (policyEngineFeaturedTools[country] ?? []) : []; return tools.map((tool) => ({ ...tool, @@ -95,11 +95,13 @@ function getPolicyEngineToolsNavItem( } const allToolsUrl = getPolicyEngineAllToolsUrl(country, baseUrl); - // Each tool is its own zone, and so is the research listing, so every row - // is a full page load rather than a client-side transition. + // Each tool is its own zone, and the research listing belongs to the + // website's, so no app rendering this header can reach them with a + // client-side transition: every row is a full page load. return { label: 'Tools', href: allToolsUrl, + external: true, children: [ ...tools.map(({ label, href }) => ({ label, href, external: true })), { label: 'All interactive tools', href: allToolsUrl, external: true }, diff --git a/src/layout/header/Header.tsx b/src/layout/header/Header.tsx index 37c2b80..7910e67 100644 --- a/src/layout/header/Header.tsx +++ b/src/layout/header/Header.tsx @@ -2,22 +2,26 @@ import { forwardRef, useState } from 'react'; import { cn } from '../../utils/cn'; import { HeaderContent } from './HeaderContent'; +/** A row in a nav item's dropdown. */ +export interface NavItemChildConfig { + label: string; + href: string; + description?: string; + /** Overrides the parent item's `external` for this row. */ + external?: boolean; +} + export interface NavItemConfig { label: string; href: string; /** - * Render a plain `` that the browser follows as a full page load, - * skipping `linkComponent` and `onNavigate`. Use it for a page served by - * another zone, which a client-side router can't reach. + * Render a plain `` that the browser follows as a full page load in + * the same tab, skipping `linkComponent` and `onNavigate`. Use it for a + * page served by another zone, which a client-side router can't reach. + * Dropdown rows inherit it unless they set their own. */ external?: boolean; - children?: { - label: string; - href: string; - description?: string; - /** Same as the item-level `external`, for one dropdown row. */ - external?: boolean; - }[]; + children?: NavItemChildConfig[]; } export interface CountryConfig { @@ -35,9 +39,15 @@ export interface HeaderProps { currentCountry?: string; /** Callback when a country is selected */ onCountryChange?: (id: string) => void; - /** Navigation callback — called when any nav link is clicked */ + /** + * Navigation callback — called when a nav link is clicked, except + * `external` items, which the browser follows itself + */ onNavigate?: (href: string) => void; - /** Custom link component (e.g. React Router Link, Next.js Link) */ + /** + * Custom link component (e.g. React Router Link, Next.js Link), used for + * every nav link except `external` items + */ linkComponent?: React.ElementType; /** Logo image source URL */ logoSrc?: string; diff --git a/src/layout/header/HeaderMobileMenu.tsx b/src/layout/header/HeaderMobileMenu.tsx index 75224b9..050707b 100644 --- a/src/layout/header/HeaderMobileMenu.tsx +++ b/src/layout/header/HeaderMobileMenu.tsx @@ -70,7 +70,14 @@ export function HeaderMobileMenu({ Menu @@ -99,7 +106,8 @@ export function HeaderMobileMenu({ style={{ gap: '8px', paddingLeft: '16px' }} > {item.children.map((child) => { - if (linkComponent && !child.external) { + const childExternal = child.external ?? item.external; + if (linkComponent && !childExternal) { const LinkComp = linkComponent; return ( { e.preventDefault(); diff --git a/src/layout/header/HeaderNavItem.tsx b/src/layout/header/HeaderNavItem.tsx index 8d47847..61881b3 100644 --- a/src/layout/header/HeaderNavItem.tsx +++ b/src/layout/header/HeaderNavItem.tsx @@ -109,9 +109,11 @@ function AppleDropdown({ : 'translateX(-50%) translateY(-8px)', marginTop: '10px', // The trigger is a narrow box, so size the panel to its longest row - // instead of wrapping every row at the minimum width. + // instead of wrapping every row at the minimum width, up to a cap + // that keeps long descriptions from widening the page. width: 'max-content', minWidth: '220px', + maxWidth: 'min(28rem, calc(100vw - 32px))', overflow: 'hidden', maxHeight: visible ? `${contentHeight}px` : '0px', opacity: visible ? 1 : 0, @@ -259,7 +261,10 @@ export function HeaderNavItem({ /> ({ + ...child, + external: child.external ?? external, + }))} open={dropdownOpen} onClose={() => setDropdownOpen(false)} onNavigate={onNavigate} diff --git a/src/layout/header/index.ts b/src/layout/header/index.ts index a14f46c..0aed440 100644 --- a/src/layout/header/index.ts +++ b/src/layout/header/index.ts @@ -1,5 +1,10 @@ export { Header } from './Header'; -export type { HeaderProps, NavItemConfig, CountryConfig } from './Header'; +export type { + HeaderProps, + NavItemConfig, + NavItemChildConfig, + CountryConfig, +} from './Header'; export { HeaderContent } from './HeaderContent'; export { HeaderLogo } from './HeaderLogo'; export { HeaderNav } from './HeaderNav'; diff --git a/src/layout/index.ts b/src/layout/index.ts index cccf0eb..b7fd14e 100644 --- a/src/layout/index.ts +++ b/src/layout/index.ts @@ -42,5 +42,6 @@ export { Header, type HeaderProps, type NavItemConfig, + type NavItemChildConfig, type CountryConfig, } from './header'; From 926297ced4ea268817a959d591ee7eb70803ca60 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 25 Oct 2026 05:58:07 -0400 Subject: [PATCH 3/8] Address review of the Tools menu - Rows inherit `external` from their item unless they set their own, on desktop and mobile; the Tools item is external too. `NavItemChildConfig` names the row type. - Cap dropdown width at min(28rem, 100vw - 32px). - Let the mobile sheet scroll on short phones and sit above the sticky header. - Freeze the featured-tools data and type it as read-only and partial. - Move the network parity check out of `bun run test` into `bun run test:network`; its workflow runs on changes to the copy and weekly, and opens an issue on drift outside a pull request. - Split the changelog into added, changed and fixed entries. - Tests for each of the above; all 31 mutants are killed. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/featured-tools-parity.yml | 46 +++++++- changelog.d/tools-nav-menu.added.md | 2 +- changelog.d/tools-nav-menu.changed.md | 1 + changelog.d/tools-nav-menu.fixed.md | 1 + package.json | 1 + src/layout/PolicyEngineFeaturedTools.ts | 8 +- src/layout/PolicyEngineSiteConfig.ts | 7 +- tests/consumer-types/fixture.ts | 13 ++- tests/layout/PolicyEngineSiteConfig.test.ts | 18 ++- tests/layout/PolicyEngineToolsNav.test.tsx | 106 +++++++++++++++++- ...ts => featuredToolsParity.network.test.ts} | 18 ++- vitest.config.ts | 7 +- 12 files changed, 197 insertions(+), 31 deletions(-) create mode 100644 changelog.d/tools-nav-menu.changed.md create mode 100644 changelog.d/tools-nav-menu.fixed.md rename tests/layout/{featuredToolsParity.test.ts => featuredToolsParity.network.test.ts} (69%) diff --git a/.github/workflows/featured-tools-parity.yml b/.github/workflows/featured-tools-parity.yml index 5741334..4b8e6ed 100644 --- a/.github/workflows/featured-tools-parity.yml +++ b/.github/workflows/featured-tools-parity.yml @@ -1,18 +1,37 @@ name: Featured tools parity # The header's Tools menu copies policyengine.org's list; see -# tests/layout/featuredToolsParity.test.ts. PR checks run that test too. This -# weekly run catches a tool added on policyengine.org while nothing changes -# here. +# tests/layout/featuredToolsParity.network.test.ts. This runs when the copy +# or its check changes here, and weekly to catch a change on policyengine.org +# while nothing changes here. A failure outside a pull request opens an issue, +# or comments on the one already open. on: + pull_request: + paths: + - src/layout/PolicyEngineFeaturedTools.ts + - tests/layout/featuredToolsParity.network.test.ts + - .github/workflows/featured-tools-parity.yml + push: + branches: [main] + paths: + - src/layout/PolicyEngineFeaturedTools.ts + - tests/layout/featuredToolsParity.network.test.ts + - .github/workflows/featured-tools-parity.yml schedule: - cron: "0 14 * * 1" workflow_dispatch: +permissions: + contents: read + jobs: parity: name: Featured tools parity runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + issues: write steps: - name: Check out repository @@ -25,4 +44,23 @@ jobs: run: bun install --frozen-lockfile - name: Compare with policyengine.org's list - run: bunx vitest run tests/layout/featuredToolsParity.test.ts + run: bun run test:network + + - name: Open or update the drift issue + if: failure() && github.event_name != 'pull_request' + env: + GH_TOKEN: ${{ github.token }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + title="Tools menu differs from policyengine.org" + body="The featured tools parity check failed: $RUN_URL + + src/layout/PolicyEngineFeaturedTools.ts no longer matches app/src/data/featuredTools.json in PolicyEngine/policyengine-app-v2 (or that file could not be read). Change the copy here to match, so zone apps and policyengine.org show the same Tools menu." + number=$(gh issue list --repo "$GITHUB_REPOSITORY" --state open \ + --search "in:title \"$title\"" --json number,title \ + --jq "map(select(.title == \"$title\")) | .[0].number // empty") + if [ -n "$number" ]; then + gh issue comment "$number" --repo "$GITHUB_REPOSITORY" --body "Still failing: $RUN_URL" + else + gh issue create --repo "$GITHUB_REPOSITORY" --title "$title" --body "$body" + fi diff --git a/changelog.d/tools-nav-menu.added.md b/changelog.d/tools-nav-menu.added.md index 8be35bb..dee0fc7 100644 --- a/changelog.d/tools-nav-menu.added.md +++ b/changelog.d/tools-nav-menu.added.md @@ -1 +1 @@ -Add a country-aware Tools menu to the PolicyEngine header, right after Research, matching policyengine.org's own header: CliffWatch, the OBBBA household explorer and the state legislative tracker for the US, UK CliffWatch for the UK, and no menu for other countries, each followed by "All interactive tools". Every row is a full page load into the tool's own zone. Also export `policyEngineFeaturedTools`, `getPolicyEngineFeaturedTools` and `getPolicyEngineAllToolsUrl`; add an `external` flag to `NavItemConfig` items and dropdown rows that renders a plain link, skipping `linkComponent` and `onNavigate`; and size header dropdowns to their longest row, so labels and descriptions no longer wrap at 220px. +Add a Tools menu to the PolicyEngine header, right after Research, with the same tools as policyengine.org's header. The US menu lists CliffWatch, the OBBBA household explorer and the state legislative tracker; the UK menu lists UK CliffWatch; other countries get no menu. Each menu ends with "All interactive tools". Every row is a full page load, because each tool runs as its own app on policyengine.org. New exports: `policyEngineFeaturedTools`, `getPolicyEngineFeaturedTools`, `getPolicyEngineAllToolsUrl` and the `NavItemChildConfig` type. `NavItemConfig` items and their dropdown rows take a new `external` flag that renders a plain link, bypassing `linkComponent` and `onNavigate`; rows inherit it from their item unless they set their own. diff --git a/changelog.d/tools-nav-menu.changed.md b/changelog.d/tools-nav-menu.changed.md new file mode 100644 index 0000000..7c69d56 --- /dev/null +++ b/changelog.d/tools-nav-menu.changed.md @@ -0,0 +1 @@ +Header dropdowns now widen to fit their longest row, up to 28rem or the viewport width less 32px, instead of wrapping every row at 220px. diff --git a/changelog.d/tools-nav-menu.fixed.md b/changelog.d/tools-nav-menu.fixed.md new file mode 100644 index 0000000..919f4a0 --- /dev/null +++ b/changelog.d/tools-nav-menu.fixed.md @@ -0,0 +1 @@ +The mobile header menu now scrolls when it is taller than the screen, so Donate and other low items stay reachable on short phones, and it sits above the sticky header instead of under it. diff --git a/package.json b/package.json index 39f3081..38803cc 100644 --- a/package.json +++ b/package.json @@ -105,6 +105,7 @@ "prebuild": "tsx scripts/generate-css.ts", "build": "vite build && mv dist/ui-kit.css dist/styles.css && tsc -p tsconfig.build.json --emitDeclarationOnly", "test": "vitest run", + "test:network": "VITEST_NETWORK=1 vitest run .network.test", "test:watch": "vitest", "typecheck": "tsc --noEmit", "lint": "tsc --noEmit", diff --git a/src/layout/PolicyEngineFeaturedTools.ts b/src/layout/PolicyEngineFeaturedTools.ts index 26d5ab6..6ad47e4 100644 --- a/src/layout/PolicyEngineFeaturedTools.ts +++ b/src/layout/PolicyEngineFeaturedTools.ts @@ -22,10 +22,10 @@ function freezeTools( * * This mirrors `app/src/data/featuredTools.json` in * PolicyEngine/policyengine-app-v2, which drives policyengine.org's own - * header and homepage. `tests/layout/featuredToolsParity.test.ts` fails when - * the two lists differ, so change both together. A country with no entry has - * no Tools menu. The data is frozen because every header in a bundle shares - * it. + * header and homepage, so change both together. + * `tests/layout/featuredToolsParity.network.test.ts` fails when the two lists + * differ. A country with no entry has no Tools menu. The data is frozen + * because every header in a bundle shares it. */ export const policyEngineFeaturedTools: Readonly< Partial> diff --git a/src/layout/PolicyEngineSiteConfig.ts b/src/layout/PolicyEngineSiteConfig.ts index c467ded..101fb14 100644 --- a/src/layout/PolicyEngineSiteConfig.ts +++ b/src/layout/PolicyEngineSiteConfig.ts @@ -1,9 +1,6 @@ import type { CountryConfig, NavItemConfig } from './header'; import type { FooterProps } from './Footer'; -import { - policyEngineFeaturedTools, - type PolicyEngineFeaturedTool, -} from './PolicyEngineFeaturedTools'; +import { policyEngineFeaturedTools } from './PolicyEngineFeaturedTools'; export type PolicyEngineCountryId = 'us' | 'uk' | (string & {}); @@ -67,7 +64,7 @@ export function getPolicyEngineUrl( export function getPolicyEngineFeaturedTools( country: PolicyEngineCountryId = 'us', baseUrl = DEFAULT_POLICYENGINE_BASE_URL, -): (PolicyEngineFeaturedTool & { readonly href: string })[] { +): { slug: string; label: string; description: string; href: string }[] { const tools = Object.hasOwn(policyEngineFeaturedTools, country) ? (policyEngineFeaturedTools[country] ?? []) : []; diff --git a/tests/consumer-types/fixture.ts b/tests/consumer-types/fixture.ts index e504ff9..ba947ea 100644 --- a/tests/consumer-types/fixture.ts +++ b/tests/consumer-types/fixture.ts @@ -53,6 +53,7 @@ import { getPolicyEngineAllToolsUrl, policyEngineFeaturedTools, type NavItemConfig, + type NavItemChildConfig, type PolicyEngineFeaturedTool, // Inputs CurrencyInput, @@ -115,16 +116,16 @@ export const _LegacyColorPaths = { grayMid: legacyColors.gray[500], }; -// The Tools menu surface zone apps read (0.13.0): the item-level and -// dropdown-row `external` flags and the featured-tool entries. +// The Tools menu surface zone apps read: the item-level and dropdown-row +// `external` flags and the featured-tool entries. export const _ToolsNavPaths: { external: NavItemConfig["external"]; - rowExternal: NonNullable[number]["external"]; - tool: PolicyEngineFeaturedTool; + row: NavItemChildConfig; + tool: PolicyEngineFeaturedTool | undefined; } = { external: true, - rowExternal: true, - tool: policyEngineFeaturedTools.us[0], + row: { label: "CliffWatch", href: "/us/cliffwatch", external: true }, + tool: policyEngineFeaturedTools.us?.[0], }; // Touch each binding so unused-import settings don't strip them. diff --git a/tests/layout/PolicyEngineSiteConfig.test.ts b/tests/layout/PolicyEngineSiteConfig.test.ts index a6df52f..eacbb5c 100644 --- a/tests/layout/PolicyEngineSiteConfig.test.ts +++ b/tests/layout/PolicyEngineSiteConfig.test.ts @@ -92,6 +92,7 @@ describe('PolicyEngine Tools menu', () => { expect(toolsItem(navItems)).toEqual({ label: 'Tools', href: 'https://policyengine.org/us/research?types=interactive', + external: true, children: [ { label: 'CliffWatch', @@ -131,6 +132,7 @@ describe('PolicyEngine Tools menu', () => { expect(toolsItem(navItems)).toEqual({ label: 'Tools', href: 'https://policyengine.org/uk/research?types=interactive', + external: true, children: [ { label: 'UK CliffWatch', @@ -164,7 +166,7 @@ describe('PolicyEngine Tools menu', () => { { slug: 'uk-cliff-watch', label: 'UK CliffWatch', - description: policyEngineFeaturedTools.uk[0].description, + description: policyEngineFeaturedTools.uk![0].description, href: 'https://staging.policyengine.org/uk/uk-cliff-watch', }, ]); @@ -182,8 +184,18 @@ describe('PolicyEngine Tools menu', () => { ); }); + it('freezes the shared data so one header cannot change another', () => { + expect(Object.isFrozen(policyEngineFeaturedTools)).toBe(true); + for (const tools of Object.values(policyEngineFeaturedTools)) { + expect(Object.isFrozen(tools)).toBe(true); + for (const tool of tools ?? []) { + expect(Object.isFrozen(tool)).toBe(true); + } + } + }); + it('keeps every tool entry well formed', () => { - for (const [country, tools] of Object.entries(policyEngineFeaturedTools)) { + for (const [country, tools = []] of Object.entries(policyEngineFeaturedTools)) { expect(tools.length, country).toBeGreaterThan(0); for (const tool of tools) { expect(tool.slug).toMatch(/^[a-z0-9]+(-[a-z0-9]+)*$/); @@ -235,7 +247,7 @@ describe('PolicyEngine Tools menu properties', () => { const origin = baseUrl.replace(/\/+$/, ''); const tools = toolsItem(getPolicyEngineNavItems(country, baseUrl)); const rows = tools?.children ?? []; - const expectedTools = policyEngineFeaturedTools[country]; + const expectedTools = policyEngineFeaturedTools[country]!; expect(rows.map((row) => row.label)).toEqual([ ...expectedTools.map((tool) => tool.label), diff --git a/tests/layout/PolicyEngineToolsNav.test.tsx b/tests/layout/PolicyEngineToolsNav.test.tsx index befa078..ee56b45 100644 --- a/tests/layout/PolicyEngineToolsNav.test.tsx +++ b/tests/layout/PolicyEngineToolsNav.test.tsx @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { render, screen, within } from '@testing-library/react'; +import { render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Header } from '../../src/layout/header'; import { PolicyEngineHeader } from '../../src/layout/PolicyEngineHeader'; @@ -73,6 +73,11 @@ describe('PolicyEngineHeader Tools menu', () => { await userEvent.click(screen.getByRole('link', { name: 'OBBBA household explorer' })); expect(onNavigate).not.toHaveBeenCalled(); expect(clickWasPrevented).toEqual([false]); + // The click also closes the dropdown, for a browser that keeps the page + // (a middle click, or a click the browser refuses to follow). + await waitFor(() => + expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(), + ); }); it('still routes the other dropdowns through the link component and onNavigate', async () => { @@ -101,8 +106,13 @@ describe('PolicyEngineHeader Tools menu', () => { const panel = screen .getByRole('link', { name: 'OBBBA household explorer' }) .closest('div[style*="position: absolute"]'); - expect(panel?.style.width).toBe('max-content'); - expect(panel?.style.minWidth).toBe('220px'); + expect(panel).not.toBeNull(); + expect(panel!.contains(screen.getByRole('link', { name: 'CliffWatch' }))).toBe( + true, + ); + expect(panel!.style.width).toBe('max-content'); + expect(panel!.style.minWidth).toBe('220px'); + expect(panel!.style.maxWidth).toBe('min(28rem, calc(100vw - 32px))'); }); it('lists only UK CliffWatch and the full list for the UK', async () => { @@ -157,6 +167,15 @@ describe('PolicyEngineHeader Tools menu', () => { expect(screen.queryByRole('dialog')).toBeNull(); }); + it('lets the mobile menu scroll above the sticky header', async () => { + render(); + + const menu = await openMobileMenu(); + expect(menu.style.overflowY).toBe('auto'); + expect(Number(menu.style.zIndex)).toBeGreaterThan(1000); + expect(within(menu).getByRole('link', { name: 'Donate' })).toBeInTheDocument(); + }); + it('leaves mobile tool links to the browser when there is no router', async () => { const onNavigate = vi.fn(); render(); @@ -173,6 +192,87 @@ describe('PolicyEngineHeader Tools menu', () => { }); }); +describe('Header external dropdowns', () => { + const navItems = [ + { + label: 'Zones', + href: '/zones', + external: true, + children: [ + { label: 'Zone row', href: 'https://example.org/zone' }, + { label: 'Local row', href: '/local', external: false }, + ], + }, + { + label: 'Mixed', + href: '/mixed', + children: [ + { label: 'Routed row', href: '/routed' }, + { label: 'Other zone row', href: 'https://example.org/other', external: true }, + ], + }, + ]; + + it('passes the item flag to rows on desktop unless a row sets its own', async () => { + const onNavigate = vi.fn(); + render( +
, + ); + + await openDesktopMenu('Zones'); + expect(screen.getByRole('link', { name: 'Zone row' })).not.toHaveAttribute( + 'data-client-link', + ); + expect(screen.getByRole('link', { name: 'Local row' })).toHaveAttribute( + 'data-client-link', + ); + await userEvent.click(screen.getByRole('link', { name: 'Zone row' })); + expect(onNavigate).not.toHaveBeenCalled(); + + await openDesktopMenu('Mixed'); + expect(screen.getByRole('link', { name: 'Routed row' })).toHaveAttribute( + 'data-client-link', + ); + await userEvent.click(screen.getByRole('link', { name: 'Other zone row' })); + expect(onNavigate).not.toHaveBeenCalled(); + expect(clickWasPrevented).toEqual([false, false]); + }); + + it('passes the item flag to rows on mobile unless a row sets its own', async () => { + const { unmount } = render( +
, + ); + const routedMenu = await openMobileMenu(); + expect( + within(routedMenu).getByRole('link', { name: 'Zone row' }), + ).not.toHaveAttribute('data-client-link'); + expect( + within(routedMenu).getByRole('link', { name: 'Local row' }), + ).toHaveAttribute('data-client-link'); + expect( + within(routedMenu).getByRole('link', { name: 'Routed row' }), + ).toHaveAttribute('data-client-link'); + expect( + within(routedMenu).getByRole('link', { name: 'Other zone row' }), + ).not.toHaveAttribute('data-client-link'); + unmount(); + + const onNavigate = vi.fn(); + render(
); + for (const name of ['Zone row', 'Other zone row']) { + const menu = await openMobileMenu(); + await userEvent.click(within(menu).getByRole('link', { name })); + expect(screen.queryByRole('dialog')).toBeNull(); + } + expect(onNavigate).not.toHaveBeenCalled(); + + const menu = await openMobileMenu(); + await userEvent.click(within(menu).getByRole('link', { name: 'Local row' })); + expect(onNavigate).toHaveBeenCalledWith('/local'); + expect(clickWasPrevented).toEqual([false, false, true]); + }); +}); + describe('Header external items', () => { const navItems = [ { label: 'Zone page', href: 'https://example.org/zone', external: true }, diff --git a/tests/layout/featuredToolsParity.test.ts b/tests/layout/featuredToolsParity.network.test.ts similarity index 69% rename from tests/layout/featuredToolsParity.test.ts rename to tests/layout/featuredToolsParity.network.test.ts index 6501679..9edd5d5 100644 --- a/tests/layout/featuredToolsParity.test.ts +++ b/tests/layout/featuredToolsParity.network.test.ts @@ -8,9 +8,10 @@ import { policyEngineFeaturedTools } from '../../src/layout/PolicyEngineFeatured * app-v2 is still on ui-kit 0.4, so it can't import the list from here yet. * Once app-v2 reads it from ui-kit and deletes its own file, delete this test. * - * This reads app-v2's main branch over the network, so it also runs on a - * weekly schedule (.github/workflows/featured-tools-parity.yml) to catch a - * tool added on policyengine.org when nothing changes here. + * This reads app-v2's main branch over the network, so `bun run test` skips + * it. .github/workflows/featured-tools-parity.yml runs it when this copy + * changes and weekly, to catch a change on policyengine.org while nothing + * changes here. */ const APP_V2_FEATURED_TOOLS_URL = 'https://raw.githubusercontent.com/PolicyEngine/policyengine-app-v2/main/app/src/data/featuredTools.json'; @@ -23,12 +24,21 @@ describe('featured tools parity with policyengine.org', () => { expect(response.ok, `GET ${APP_V2_FEATURED_TOOLS_URL}: ${response.status}`).toBe( true, ); + const text = await response.text(); + let appV2Tools: unknown; + try { + appV2Tools = JSON.parse(text); + } catch (error) { + throw new Error( + `${APP_V2_FEATURED_TOOLS_URL} is not JSON (${String(error)}): ${text.slice(0, 200)}`, + ); + } expect( policyEngineFeaturedTools, 'src/layout/PolicyEngineFeaturedTools.ts differs from app-v2’s ' + 'app/src/data/featuredTools.json; change them together so zone ' + 'apps and policyengine.org show the same Tools menu', - ).toStrictEqual(await response.json()); + ).toStrictEqual(appV2Tools); }, 20_000); }); diff --git a/vitest.config.ts b/vitest.config.ts index 03b2d3e..615b51b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,6 +1,6 @@ import { resolve } from 'path'; import react from '@vitejs/plugin-react'; -import { defineConfig } from 'vitest/config'; +import { configDefaults, defineConfig } from 'vitest/config'; export default defineConfig({ plugins: [react()], @@ -13,5 +13,10 @@ export default defineConfig({ globals: true, environment: 'jsdom', setupFiles: ['./vitest.setup.ts'], + // *.network.test.ts files read other repositories over the internet, so + // `bun run test` skips them; `bun run test:network` runs them. + exclude: process.env.VITEST_NETWORK + ? configDefaults.exclude + : [...configDefaults.exclude, '**/*.network.test.ts'], }, }); From 4f78f0a596b12d9e841de50c53d2ef098e36f1b4 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 26 Oct 2026 05:31:48 -0700 Subject: [PATCH 4/8] Build the Tools menu by policyengine.org's rule PolicyEngine/policyengine-app-v2#1424 replaced the hand-picked list with a rule: the PolicyEngine app first, then every calculator that works for any household under current law, sorted by label, from the `toolsMenu` labels in apps.json. Follow it. - PolicyEngineTools.ts holds each country's tools (slug and label), the main app's label and its default URL. US: CliffWatch, Coverage Compass, GiveCalc, Marriage calculator, QBI deduction calculator, SPM threshold calculator, TANF calculator. UK: Marriage calculator, Student loan calculator, UK CliffWatch. - The Tools menu lists the main app at /, then the tools. "All interactive tools" is gone, as on policyengine.org. - getPolicyEngineNavItems takes an optional app URL; PolicyEngineHeader takes an appUrl prop. - The parity check applies app-v2's rule to apps.json on main and also compares the main app's label and URL; it retries a failed connection once. The workflow is renamed tools-menu-parity.yml. - Exports renamed: policyEngineTools, getPolicyEngineTools, getPolicyEngineAppUrl, POLICYENGINE_APP_LABEL, DEFAULT_POLICYENGINE_APP_URL, PolicyEngineTool. - Tests for the new rule; all 50 mutants are killed. Co-Authored-By: Claude Opus 5.5 --- ...tools-parity.yml => tools-menu-parity.yml} | 32 +- changelog.d/tools-nav-menu.added.md | 2 +- src/layout/PolicyEngineFeaturedTools.ts | 61 ---- src/layout/PolicyEngineHeader.tsx | 6 +- src/layout/PolicyEngineSiteConfig.ts | 43 +-- src/layout/PolicyEngineTools.ts | 55 ++++ src/layout/index.ts | 12 +- tests/consumer-types/fixture.ts | 28 +- tests/layout/PolicyEngineSiteConfig.test.ts | 296 +++++++++++------- tests/layout/PolicyEngineToolsNav.test.tsx | 76 +++-- .../featuredToolsParity.network.test.ts | 44 --- tests/layout/toolsMenuParity.network.test.ts | 113 +++++++ 12 files changed, 473 insertions(+), 295 deletions(-) rename .github/workflows/{featured-tools-parity.yml => tools-menu-parity.yml} (54%) delete mode 100644 src/layout/PolicyEngineFeaturedTools.ts create mode 100644 src/layout/PolicyEngineTools.ts delete mode 100644 tests/layout/featuredToolsParity.network.test.ts create mode 100644 tests/layout/toolsMenuParity.network.test.ts diff --git a/.github/workflows/featured-tools-parity.yml b/.github/workflows/tools-menu-parity.yml similarity index 54% rename from .github/workflows/featured-tools-parity.yml rename to .github/workflows/tools-menu-parity.yml index 4b8e6ed..f0a578d 100644 --- a/.github/workflows/featured-tools-parity.yml +++ b/.github/workflows/tools-menu-parity.yml @@ -1,22 +1,22 @@ -name: Featured tools parity +name: Tools menu parity -# The header's Tools menu copies policyengine.org's list; see -# tests/layout/featuredToolsParity.network.test.ts. This runs when the copy -# or its check changes here, and weekly to catch a change on policyengine.org -# while nothing changes here. A failure outside a pull request opens an issue, -# or comments on the one already open. +# The header's Tools menu copies the toolsMenu entries in policyengine.org's +# apps.json; see tests/layout/toolsMenuParity.network.test.ts. This runs when +# the copy or its check changes here, and weekly to catch a change on +# policyengine.org while nothing changes here. A failure outside a pull +# request opens an issue, or comments on the one already open. on: pull_request: paths: - - src/layout/PolicyEngineFeaturedTools.ts - - tests/layout/featuredToolsParity.network.test.ts - - .github/workflows/featured-tools-parity.yml + - src/layout/PolicyEngineTools.ts + - tests/layout/toolsMenuParity.network.test.ts + - .github/workflows/tools-menu-parity.yml push: branches: [main] paths: - - src/layout/PolicyEngineFeaturedTools.ts - - tests/layout/featuredToolsParity.network.test.ts - - .github/workflows/featured-tools-parity.yml + - src/layout/PolicyEngineTools.ts + - tests/layout/toolsMenuParity.network.test.ts + - .github/workflows/tools-menu-parity.yml schedule: - cron: "0 14 * * 1" workflow_dispatch: @@ -26,7 +26,7 @@ permissions: jobs: parity: - name: Featured tools parity + name: Tools menu parity runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -43,7 +43,7 @@ jobs: - name: Install dependencies run: bun install --frozen-lockfile - - name: Compare with policyengine.org's list + - name: Compare with policyengine.org's apps.json run: bun run test:network - name: Open or update the drift issue @@ -53,9 +53,9 @@ jobs: RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | title="Tools menu differs from policyengine.org" - body="The featured tools parity check failed: $RUN_URL + body="The Tools menu parity check failed: $RUN_URL - src/layout/PolicyEngineFeaturedTools.ts no longer matches app/src/data/featuredTools.json in PolicyEngine/policyengine-app-v2 (or that file could not be read). Change the copy here to match, so zone apps and policyengine.org show the same Tools menu." + src/layout/PolicyEngineTools.ts no longer matches the toolsMenu entries in app/src/data/apps/apps.json in PolicyEngine/policyengine-app-v2, or the main app's label or URL changed there, or those files could not be read. Update the copy here to match, so zone apps and policyengine.org show the same Tools menu." number=$(gh issue list --repo "$GITHUB_REPOSITORY" --state open \ --search "in:title \"$title\"" --json number,title \ --jq "map(select(.title == \"$title\")) | .[0].number // empty") diff --git a/changelog.d/tools-nav-menu.added.md b/changelog.d/tools-nav-menu.added.md index dee0fc7..5db0ddb 100644 --- a/changelog.d/tools-nav-menu.added.md +++ b/changelog.d/tools-nav-menu.added.md @@ -1 +1 @@ -Add a Tools menu to the PolicyEngine header, right after Research, with the same tools as policyengine.org's header. The US menu lists CliffWatch, the OBBBA household explorer and the state legislative tracker; the UK menu lists UK CliffWatch; other countries get no menu. Each menu ends with "All interactive tools". Every row is a full page load, because each tool runs as its own app on policyengine.org. New exports: `policyEngineFeaturedTools`, `getPolicyEngineFeaturedTools`, `getPolicyEngineAllToolsUrl` and the `NavItemChildConfig` type. `NavItemConfig` items and their dropdown rows take a new `external` flag that renders a plain link, bypassing `linkComponent` and `onNavigate`; rows inherit it from their item unless they set their own. +Add a Tools menu to the PolicyEngine header, right after Research, built by the same rule as policyengine.org's: the PolicyEngine app first, then every calculator that works for any household under current law, sorted by label. The US menu lists CliffWatch, Coverage Compass, GiveCalc, the marriage calculator, the QBI deduction calculator, the SPM threshold calculator and the TANF calculator; the UK menu lists the marriage calculator, the student loan calculator and UK CliffWatch; other countries get no menu. Every row is a full page load, because the PolicyEngine app is on its own domain and each tool runs as its own app on policyengine.org. New exports: `policyEngineTools`, `getPolicyEngineTools`, `getPolicyEngineAppUrl`, `POLICYENGINE_APP_LABEL`, `DEFAULT_POLICYENGINE_APP_URL` and the `PolicyEngineTool` and `NavItemChildConfig` types. `getPolicyEngineNavItems` takes an optional app URL as its third argument, and `PolicyEngineHeader` an `appUrl` prop. `NavItemConfig` items and their dropdown rows take a new `external` flag that renders a plain link, bypassing `linkComponent` and `onNavigate`; rows inherit it from their item unless they set their own. diff --git a/src/layout/PolicyEngineFeaturedTools.ts b/src/layout/PolicyEngineFeaturedTools.ts deleted file mode 100644 index 6ad47e4..0000000 --- a/src/layout/PolicyEngineFeaturedTools.ts +++ /dev/null @@ -1,61 +0,0 @@ -/** - * An interactive tool listed in the PolicyEngine header's Tools menu. - * - * Each tool is its own zone on policyengine.org at `//`, so a - * link to one must be a full page load, never a client-side transition. - */ -export interface PolicyEngineFeaturedTool { - /** Path segment after the country: the tool lives at `//`. */ - readonly slug: string; - readonly label: string; - readonly description: string; -} - -function freezeTools( - tools: PolicyEngineFeaturedTool[], -): readonly PolicyEngineFeaturedTool[] { - return Object.freeze(tools.map((tool) => Object.freeze({ ...tool }))); -} - -/** - * The featured tools for each country, in menu order. - * - * This mirrors `app/src/data/featuredTools.json` in - * PolicyEngine/policyengine-app-v2, which drives policyengine.org's own - * header and homepage, so change both together. - * `tests/layout/featuredToolsParity.network.test.ts` fails when the two lists - * differ. A country with no entry has no Tools menu. The data is frozen - * because every header in a bundle shares it. - */ -export const policyEngineFeaturedTools: Readonly< - Partial> -> = Object.freeze({ - us: freezeTools([ - { - slug: 'cliffwatch', - label: 'CliffWatch', - description: - "See where benefits, refundable tax credits, and taxes create cliffs as a household's wages rise, in all 50 states and DC.", - }, - { - slug: 'obbba-households', - label: 'OBBBA household explorer', - description: - "See how the One Big Beautiful Bill Act changes each modeled household's resources in 2026, and which provisions drive the change.", - }, - { - slug: 'bill-tracker', - label: 'State legislative tracker', - description: - "Monitor tax and benefit legislation across all 50 US states. See which states are in session, track active bills, and explore PolicyEngine's research on state-level policy reforms.", - }, - ]), - uk: freezeTools([ - { - slug: 'uk-cliff-watch', - label: 'UK CliffWatch', - description: - "See where benefit withdrawal and taxes create cliffs and high marginal rates as a UK household's earnings rise.", - }, - ]), -}); diff --git a/src/layout/PolicyEngineHeader.tsx b/src/layout/PolicyEngineHeader.tsx index 4b20f71..a04a3b5 100644 --- a/src/layout/PolicyEngineHeader.tsx +++ b/src/layout/PolicyEngineHeader.tsx @@ -1,6 +1,7 @@ 'use client'; import { Header, type CountryConfig, type HeaderProps, type NavItemConfig } from './header'; +import { DEFAULT_POLICYENGINE_APP_URL } from './PolicyEngineTools'; import { DEFAULT_POLICYENGINE_BASE_URL, getPolicyEngineCountrySwitchUrl, @@ -17,6 +18,8 @@ export interface PolicyEngineHeaderProps > { country?: PolicyEngineCountryId; baseUrl?: string; + /** The main app's origin, which the Tools menu's first row links to. */ + appUrl?: string; navItems?: NavItemConfig[]; countries?: CountryConfig[]; logoHref?: string; @@ -26,6 +29,7 @@ export interface PolicyEngineHeaderProps export function PolicyEngineHeader({ country = 'us', baseUrl = DEFAULT_POLICYENGINE_BASE_URL, + appUrl = DEFAULT_POLICYENGINE_APP_URL, navItems, countries = policyEngineCountries, logoHref, @@ -45,7 +49,7 @@ export function PolicyEngineHeader({ return (
({ ...tool, @@ -74,34 +78,36 @@ export function getPolicyEngineFeaturedTools( })); } -/** The research listing filtered to interactive tools. */ -export function getPolicyEngineAllToolsUrl( +/** The main app for a country. */ +export function getPolicyEngineAppUrl( country: PolicyEngineCountryId = 'us', - baseUrl = DEFAULT_POLICYENGINE_BASE_URL, + appUrl = DEFAULT_POLICYENGINE_APP_URL, ) { - return getPolicyEngineUrl(country, 'research?types=interactive', baseUrl); + return `${normalizeBaseUrl(appUrl)}/${country}`; } function getPolicyEngineToolsNavItem( country: PolicyEngineCountryId, baseUrl: string, + appUrl: string, ): NavItemConfig | null { - const tools = getPolicyEngineFeaturedTools(country, baseUrl); + const tools = getPolicyEngineTools(country, baseUrl); + // The main app never stands alone in the menu. if (tools.length === 0) { return null; } - const allToolsUrl = getPolicyEngineAllToolsUrl(country, baseUrl); - // Each tool is its own zone, and the research listing belongs to the - // website's, so no app rendering this header can reach them with a - // client-side transition: every row is a full page load. + const mainAppHref = getPolicyEngineAppUrl(country, appUrl); + // The main app is on another domain and each tool is its own zone, so no + // app rendering this header can reach them with a client-side transition: + // every row is a full page load. return { label: 'Tools', - href: allToolsUrl, + href: mainAppHref, external: true, children: [ + { label: POLICYENGINE_APP_LABEL, href: mainAppHref, external: true }, ...tools.map(({ label, href }) => ({ label, href, external: true })), - { label: 'All interactive tools', href: allToolsUrl, external: true }, ], }; } @@ -109,8 +115,9 @@ function getPolicyEngineToolsNavItem( export function getPolicyEngineNavItems( country: PolicyEngineCountryId = 'us', baseUrl = DEFAULT_POLICYENGINE_BASE_URL, + appUrl = DEFAULT_POLICYENGINE_APP_URL, ): NavItemConfig[] { - const toolsNavItem = getPolicyEngineToolsNavItem(country, baseUrl); + const toolsNavItem = getPolicyEngineToolsNavItem(country, baseUrl, appUrl); return [ { label: 'Research', diff --git a/src/layout/PolicyEngineTools.ts b/src/layout/PolicyEngineTools.ts new file mode 100644 index 0000000..646e0d2 --- /dev/null +++ b/src/layout/PolicyEngineTools.ts @@ -0,0 +1,55 @@ +/** + * A tool listed in the PolicyEngine header's Tools menu: a calculator that + * works for any household under current law. Research about one policy, bill + * or event stays out of the menu, however interactive it is. + * + * Each tool is its own zone on policyengine.org at `//`, so a + * link to one must be a full page load, never a client-side transition. + */ +export interface PolicyEngineTool { + /** Path segment after the country: the tool lives at `//`. */ + readonly slug: string; + readonly label: string; +} + +/** The Tools menu's first row, which links to the main app. */ +export const POLICYENGINE_APP_LABEL = 'PolicyEngine app'; + +/** Where the main app lives; it serves `/`. */ +export const DEFAULT_POLICYENGINE_APP_URL = 'https://app.policyengine.org'; + +function freezeTools( + tools: PolicyEngineTool[], +): readonly PolicyEngineTool[] { + return Object.freeze(tools.map((tool) => Object.freeze({ ...tool }))); +} + +/** + * Each country's tools, sorted by label. A country with no entry has no + * Tools menu. + * + * policyengine.org builds the same menu from the `toolsMenu` labels in + * PolicyEngine/policyengine-app-v2's `app/src/data/apps/apps.json`, which + * ui-kit can't import. `tests/layout/toolsMenuParity.network.test.ts` fails + * when this copy, the main app's label or its URL differ from app-v2's, so + * change them when apps.json changes. The data is frozen because every + * header in a bundle shares it. + */ +export const policyEngineTools: Readonly< + Partial> +> = Object.freeze({ + us: freezeTools([ + { slug: 'cliffwatch', label: 'CliffWatch' }, + { slug: 'coverage-compass', label: 'Coverage Compass' }, + { slug: 'givecalc', label: 'GiveCalc' }, + { slug: 'marriage', label: 'Marriage calculator' }, + { slug: 'qbi-calculator', label: 'QBI deduction calculator' }, + { slug: 'spm-calculator', label: 'SPM threshold calculator' }, + { slug: 'tanf-calculator', label: 'TANF calculator' }, + ]), + uk: freezeTools([ + { slug: 'marriage', label: 'Marriage calculator' }, + { slug: 'uk-student-loan-calculator', label: 'Student loan calculator' }, + { slug: 'uk-cliff-watch', label: 'UK CliffWatch' }, + ]), +}); diff --git a/src/layout/index.ts b/src/layout/index.ts index b7fd14e..981f317 100644 --- a/src/layout/index.ts +++ b/src/layout/index.ts @@ -24,20 +24,22 @@ export { } from './PolicyEngineShell'; export { DEFAULT_POLICYENGINE_BASE_URL, - getPolicyEngineAllToolsUrl, + getPolicyEngineAppUrl, getPolicyEngineCountrySwitchUrl, getPolicyEngineCountryUrl, - getPolicyEngineFeaturedTools, getPolicyEngineFooterLinks, getPolicyEngineNavItems, + getPolicyEngineTools, getPolicyEngineUrl, policyEngineCountries, type PolicyEngineCountryId, } from './PolicyEngineSiteConfig'; export { - policyEngineFeaturedTools, - type PolicyEngineFeaturedTool, -} from './PolicyEngineFeaturedTools'; + DEFAULT_POLICYENGINE_APP_URL, + POLICYENGINE_APP_LABEL, + policyEngineTools, + type PolicyEngineTool, +} from './PolicyEngineTools'; export { Header, type HeaderProps, diff --git a/tests/consumer-types/fixture.ts b/tests/consumer-types/fixture.ts index ba947ea..9202359 100644 --- a/tests/consumer-types/fixture.ts +++ b/tests/consumer-types/fixture.ts @@ -49,12 +49,14 @@ import { Group, PolicyEngineHeader, getPolicyEngineNavItems, - getPolicyEngineFeaturedTools, - getPolicyEngineAllToolsUrl, - policyEngineFeaturedTools, + getPolicyEngineTools, + getPolicyEngineAppUrl, + policyEngineTools, + POLICYENGINE_APP_LABEL, + DEFAULT_POLICYENGINE_APP_URL, type NavItemConfig, type NavItemChildConfig, - type PolicyEngineFeaturedTool, + type PolicyEngineTool, // Inputs CurrencyInput, NumberInput, @@ -117,15 +119,23 @@ export const _LegacyColorPaths = { }; // The Tools menu surface zone apps read: the item-level and dropdown-row -// `external` flags and the featured-tool entries. +// `external` flags, the tool entries and the main app's label and URL. export const _ToolsNavPaths: { external: NavItemConfig["external"]; row: NavItemChildConfig; - tool: PolicyEngineFeaturedTool | undefined; + tool: PolicyEngineTool | undefined; + mainApp: NavItemChildConfig; + appUrlProp: Parameters[0]["appUrl"]; } = { external: true, row: { label: "CliffWatch", href: "/us/cliffwatch", external: true }, - tool: policyEngineFeaturedTools.us?.[0], + tool: policyEngineTools.us?.[0], + mainApp: { + label: POLICYENGINE_APP_LABEL, + href: `${DEFAULT_POLICYENGINE_APP_URL}/us`, + external: true, + }, + appUrlProp: DEFAULT_POLICYENGINE_APP_URL, }; // Touch each binding so unused-import settings don't strip them. @@ -164,8 +174,8 @@ export type _SmokeTest = | typeof Group | typeof PolicyEngineHeader | typeof getPolicyEngineNavItems - | typeof getPolicyEngineFeaturedTools - | typeof getPolicyEngineAllToolsUrl + | typeof getPolicyEngineTools + | typeof getPolicyEngineAppUrl | typeof CurrencyInput | typeof NumberInput | typeof DataTable diff --git a/tests/layout/PolicyEngineSiteConfig.test.ts b/tests/layout/PolicyEngineSiteConfig.test.ts index eacbb5c..72ac79a 100644 --- a/tests/layout/PolicyEngineSiteConfig.test.ts +++ b/tests/layout/PolicyEngineSiteConfig.test.ts @@ -1,12 +1,16 @@ import { describe, expect, it } from 'vitest'; import fc from 'fast-check'; import type { NavItemConfig } from '../../src/layout/header'; -import { policyEngineFeaturedTools } from '../../src/layout/PolicyEngineFeaturedTools'; +import { + DEFAULT_POLICYENGINE_APP_URL, + POLICYENGINE_APP_LABEL, + policyEngineTools, +} from '../../src/layout/PolicyEngineTools'; import { DEFAULT_POLICYENGINE_BASE_URL, - getPolicyEngineAllToolsUrl, - getPolicyEngineFeaturedTools, + getPolicyEngineAppUrl, getPolicyEngineNavItems, + getPolicyEngineTools, getPolicyEngineUrl, } from '../../src/layout/PolicyEngineSiteConfig'; @@ -55,6 +59,18 @@ function toolsItem(navItems: NavItemConfig[]) { return navItems.find((item) => item.label === 'Tools'); } +/** + * Invariants, for every country C, base URL B and app URL A: + * 1. The nav has a Tools item exactly when C has tools (the US and the UK), + * once, right after Research. + * 2. Every other item is as it was in 0.12.1. + * 3. Tools lists the main app at `/C` first, then exactly C's tools in + * label order, each at `/C/`. Every row is a full page load. + * 4. The OBBBA explorer and the bill tracker are research, not tools, and + * the UK menu never links to `/uk/cliffwatch`, the US app's path. + * 5. Calls are deterministic and return fresh objects; the shared data is + * frozen. + */ const countries = fc.oneof( fc.constantFrom('us', 'uk'), // Other countries, including keys every object inherits. @@ -64,7 +80,7 @@ const countries = fc.oneof( // Origins as consumers pass them: scheme, host, optional port, any number of // trailing slashes (which the site config strips). -const baseUrls = fc +const origins = fc .record({ scheme: fc.constantFrom('http', 'https'), host: fc.domain(), @@ -76,8 +92,41 @@ const baseUrls = fc `${scheme}://${host}${port === undefined ? '' : `:${port}`}${'/'.repeat(slashes)}`, ); +const US_TOOLS = [ + ['CliffWatch', 'cliffwatch'], + ['Coverage Compass', 'coverage-compass'], + ['GiveCalc', 'givecalc'], + ['Marriage calculator', 'marriage'], + ['QBI deduction calculator', 'qbi-calculator'], + ['SPM threshold calculator', 'spm-calculator'], + ['TANF calculator', 'tanf-calculator'], +]; + +const UK_TOOLS = [ + ['Marriage calculator', 'marriage'], + ['Student loan calculator', 'uk-student-loan-calculator'], + ['UK CliffWatch', 'uk-cliff-watch'], +]; + +function expectedToolsItem(country: string, tools: string[][]): NavItemConfig { + const mainApp = `https://app.policyengine.org/${country}`; + return { + label: 'Tools', + href: mainApp, + external: true, + children: [ + { label: 'PolicyEngine app', href: mainApp, external: true }, + ...tools.map(([label, slug]) => ({ + label, + href: `https://policyengine.org/${country}/${slug}`, + external: true, + })), + ], + }; +} + describe('PolicyEngine Tools menu', () => { - it('lists the US tools right after Research', () => { + it('lists the main app, then the US tools by label, right after Research', () => { const navItems = getPolicyEngineNavItems('us'); expect(navItems.map((item) => item.label)).toEqual([ @@ -89,36 +138,10 @@ describe('PolicyEngine Tools menu', () => { 'About', 'Donate', ]); - expect(toolsItem(navItems)).toEqual({ - label: 'Tools', - href: 'https://policyengine.org/us/research?types=interactive', - external: true, - children: [ - { - label: 'CliffWatch', - href: 'https://policyengine.org/us/cliffwatch', - external: true, - }, - { - label: 'OBBBA household explorer', - href: 'https://policyengine.org/us/obbba-households', - external: true, - }, - { - label: 'State legislative tracker', - href: 'https://policyengine.org/us/bill-tracker', - external: true, - }, - { - label: 'All interactive tools', - href: 'https://policyengine.org/us/research?types=interactive', - external: true, - }, - ], - }); + expect(toolsItem(navItems)).toEqual(expectedToolsItem('us', US_TOOLS)); }); - it('lists UK CliffWatch at /uk/uk-cliff-watch for the UK', () => { + it('lists the main app, then the UK tools by label, with UK CliffWatch at /uk/uk-cliff-watch', () => { const navItems = getPolicyEngineNavItems('uk'); expect(navItems.map((item) => item.label)).toEqual([ @@ -129,23 +152,7 @@ describe('PolicyEngine Tools menu', () => { 'About', 'Donate', ]); - expect(toolsItem(navItems)).toEqual({ - label: 'Tools', - href: 'https://policyengine.org/uk/research?types=interactive', - external: true, - children: [ - { - label: 'UK CliffWatch', - href: 'https://policyengine.org/uk/uk-cliff-watch', - external: true, - }, - { - label: 'All interactive tools', - href: 'https://policyengine.org/uk/research?types=interactive', - external: true, - }, - ], - }); + expect(toolsItem(navItems)).toEqual(expectedToolsItem('uk', UK_TOOLS)); }); it.each(['ca', 'ng', 'il', 'US', '', 'constructor', '__proto__'])( @@ -155,38 +162,58 @@ describe('PolicyEngine Tools menu', () => { expect(getPolicyEngineNavItems(country)).toEqual( navItemsWithoutTools(country, DEFAULT_POLICYENGINE_BASE_URL), ); - expect(getPolicyEngineFeaturedTools(country)).toEqual([]); + expect(getPolicyEngineTools(country)).toEqual([]); }, ); - it('builds tool links on a custom base URL', () => { - expect( - getPolicyEngineFeaturedTools('uk', 'https://staging.policyengine.org//'), - ).toEqual([ - { - slug: 'uk-cliff-watch', - label: 'UK CliffWatch', - description: policyEngineFeaturedTools.uk![0].description, - href: 'https://staging.policyengine.org/uk/uk-cliff-watch', - }, - ]); + it('builds links on a custom base URL and app URL', () => { expect( - getPolicyEngineAllToolsUrl('us', 'https://staging.policyengine.org/'), - ).toBe('https://staging.policyengine.org/us/research?types=interactive'); + getPolicyEngineTools('uk', 'https://staging.policyengine.org//')[2], + ).toEqual({ + slug: 'uk-cliff-watch', + label: 'UK CliffWatch', + href: 'https://staging.policyengine.org/uk/uk-cliff-watch', + }); + expect(getPolicyEngineAppUrl('uk', 'https://app.staging.policyengine.org/')).toBe( + 'https://app.staging.policyengine.org/uk', + ); + const tools = toolsItem( + getPolicyEngineNavItems( + 'us', + 'https://staging.policyengine.org', + 'https://app.staging.policyengine.org', + ), + ); + expect(tools?.href).toBe('https://app.staging.policyengine.org/us'); + expect(tools?.children?.[0]).toEqual({ + label: 'PolicyEngine app', + href: 'https://app.staging.policyengine.org/us', + external: true, + }); + expect(tools?.children?.[1].href).toBe( + 'https://staging.policyengine.org/us/cliffwatch', + ); }); - it('defaults to the US on policyengine.org', () => { - expect(getPolicyEngineFeaturedTools()).toEqual( - getPolicyEngineFeaturedTools('us', DEFAULT_POLICYENGINE_BASE_URL), + it('defaults to the US on policyengine.org and app.policyengine.org', () => { + expect(getPolicyEngineTools()).toEqual( + getPolicyEngineTools('us', DEFAULT_POLICYENGINE_BASE_URL), ); - expect(getPolicyEngineAllToolsUrl()).toBe( - 'https://policyengine.org/us/research?types=interactive', + expect(getPolicyEngineAppUrl()).toBe('https://app.policyengine.org/us'); + expect(DEFAULT_POLICYENGINE_APP_URL).toBe('https://app.policyengine.org'); + expect(POLICYENGINE_APP_LABEL).toBe('PolicyEngine app'); + expect(getPolicyEngineNavItems('uk')).toEqual( + getPolicyEngineNavItems( + 'uk', + DEFAULT_POLICYENGINE_BASE_URL, + DEFAULT_POLICYENGINE_APP_URL, + ), ); }); it('freezes the shared data so one header cannot change another', () => { - expect(Object.isFrozen(policyEngineFeaturedTools)).toBe(true); - for (const tools of Object.values(policyEngineFeaturedTools)) { + expect(Object.isFrozen(policyEngineTools)).toBe(true); + for (const tools of Object.values(policyEngineTools)) { expect(Object.isFrozen(tools)).toBe(true); for (const tool of tools ?? []) { expect(Object.isFrozen(tool)).toBe(true); @@ -194,19 +221,22 @@ describe('PolicyEngine Tools menu', () => { } }); - it('keeps every tool entry well formed', () => { - for (const [country, tools = []] of Object.entries(policyEngineFeaturedTools)) { + it('keeps every country’s tools well formed and sorted by label', () => { + expect(Object.keys(policyEngineTools).sort()).toEqual(['uk', 'us']); + for (const [country, tools = []] of Object.entries(policyEngineTools)) { expect(tools.length, country).toBeGreaterThan(0); for (const tool of tools) { expect(tool.slug).toMatch(/^[a-z0-9]+(-[a-z0-9]+)*$/); expect(tool.label.trim()).not.toBe(''); - expect(tool.description.trim()).not.toBe(''); + expect(tool.label).not.toBe(POLICYENGINE_APP_LABEL); } expect(new Set(tools.map((tool) => tool.slug)).size).toBe(tools.length); - expect(new Set(tools.map((tool) => tool.label)).size).toBe(tools.length); - expect(tools.some((tool) => tool.label === 'All interactive tools')).toBe( - false, - ); + for (let i = 1; i < tools.length; i++) { + expect( + tools[i - 1].label.localeCompare(tools[i].label, 'en'), + `${country}: ${tools[i - 1].label} before ${tools[i].label}`, + ).toBeLessThan(0); + } } }); }); @@ -214,8 +244,8 @@ describe('PolicyEngine Tools menu', () => { describe('PolicyEngine Tools menu properties', () => { it('appears only for a country with tools, and only right after Research', () => { fc.assert( - fc.property(countries, baseUrls, (country, baseUrl) => { - const navItems = getPolicyEngineNavItems(country, baseUrl); + fc.property(countries, origins, origins, (country, baseUrl, appUrl) => { + const navItems = getPolicyEngineNavItems(country, baseUrl, appUrl); const hasTools = country === 'us' || country === 'uk'; expect(navItems.filter((item) => item.label === 'Tools')).toHaveLength( @@ -231,9 +261,9 @@ describe('PolicyEngine Tools menu properties', () => { it('leaves every other nav item as it was before Tools', () => { fc.assert( - fc.property(countries, baseUrls, (country, baseUrl) => { + fc.property(countries, origins, origins, (country, baseUrl, appUrl) => { expect( - getPolicyEngineNavItems(country, baseUrl).filter( + getPolicyEngineNavItems(country, baseUrl, appUrl).filter( (item) => item.label !== 'Tools', ), ).toEqual(navItemsWithoutTools(country, baseUrl)); @@ -241,69 +271,93 @@ describe('PolicyEngine Tools menu properties', () => { ); }); - it('links every row to a full-page-load URL under the country on the base URL', () => { + it('lists the main app, then exactly the country’s tools by label, each a full page load', () => { fc.assert( - fc.property(fc.constantFrom('us', 'uk'), baseUrls, (country, baseUrl) => { - const origin = baseUrl.replace(/\/+$/, ''); - const tools = toolsItem(getPolicyEngineNavItems(country, baseUrl)); - const rows = tools?.children ?? []; - const expectedTools = policyEngineFeaturedTools[country]!; + fc.property( + fc.constantFrom('us', 'uk'), + origins, + origins, + (country, baseUrl, appUrl) => { + const base = baseUrl.replace(/\/+$/, ''); + const app = appUrl.replace(/\/+$/, ''); + const tools = toolsItem(getPolicyEngineNavItems(country, baseUrl, appUrl)); + const [mainApp, ...rows] = tools?.children ?? []; + const expectedTools = policyEngineTools[country]!; - expect(rows.map((row) => row.label)).toEqual([ - ...expectedTools.map((tool) => tool.label), - 'All interactive tools', - ]); - expect(new Set(rows.map((row) => row.href)).size).toBe(rows.length); - for (const row of rows) { - expect(row.external).toBe(true); - expect(row.href.startsWith(`${origin}/${country}/`)).toBe(true); - const url = new URL(row.href); - expect(`${url.protocol}//${url.host}`).toBe(new URL(origin).origin); - } - rows.slice(0, -1).forEach((row, i) => { - expect(new URL(row.href).pathname).toBe( - `/${country}/${expectedTools[i].slug}`, + expect(tools?.external).toBe(true); + expect(mainApp).toEqual({ + label: 'PolicyEngine app', + href: `${app}/${country}`, + external: true, + }); + expect(tools?.href).toBe(mainApp.href); + expect(rows.map((row) => row.label)).toEqual( + expectedTools.map((tool) => tool.label), ); - expect(new URL(row.href).search).toBe(''); - }); - const allTools = new URL(rows[rows.length - 1].href); - expect(allTools.pathname).toBe(`/${country}/research`); - expect(allTools.searchParams.get('types')).toBe('interactive'); - expect(tools?.href).toBe(rows[rows.length - 1].href); - }), + rows.forEach((row, i) => { + expect(row.external).toBe(true); + expect(row.href).toBe(`${base}/${country}/${expectedTools[i].slug}`); + const url = new URL(row.href); + expect(url.origin).toBe(new URL(base).origin); + expect(url.pathname).toBe(`/${country}/${expectedTools[i].slug}`); + expect(url.search).toBe(''); + if (i > 0) { + expect(rows[i - 1].label.localeCompare(row.label, 'en')).toBeLessThan(0); + } + }); + const labels = tools!.children!.map((row) => row.label); + expect(new Set(labels).size).toBe(labels.length); + expect(new Set(rows.map((row) => row.href)).size).toBe(rows.length); + }, + ), ); }); - it('never sends the UK menu to /uk/cliffwatch', () => { + it('leaves out research and never sends the UK menu to /uk/cliffwatch', () => { fc.assert( - fc.property(baseUrls, (baseUrl) => { + fc.property(countries, origins, (country, baseUrl) => { const paths = ( - toolsItem(getPolicyEngineNavItems('uk', baseUrl))?.children ?? [] - ).map((row) => new URL(row.href).pathname); + toolsItem(getPolicyEngineNavItems(country, baseUrl))?.children ?? [] + ) + .slice(1) + .map((row) => new URL(row.href).pathname); - expect(paths).toContain('/uk/uk-cliff-watch'); + for (const research of ['obbba-households', 'bill-tracker']) { + expect(paths).not.toContain(`/${country}/${research}`); + } expect(paths.some((path) => /^\/uk\/cliffwatch(\/|$)/.test(path))).toBe( false, ); + if (country === 'uk') { + expect(paths).toContain('/uk/uk-cliff-watch'); + } }), ); }); it('returns fresh, equal results on every call', () => { fc.assert( - fc.property(countries, baseUrls, (country, baseUrl) => { - const first = getPolicyEngineFeaturedTools(country, baseUrl); - const second = getPolicyEngineFeaturedTools(country, baseUrl); + fc.property(countries, origins, origins, (country, baseUrl, appUrl) => { + const first = getPolicyEngineTools(country, baseUrl); + const second = getPolicyEngineTools(country, baseUrl); expect(second).toEqual(first); for (const tool of first) { tool.slug = 'mutated'; tool.href = 'mutated'; } - expect(getPolicyEngineFeaturedTools(country, baseUrl)).toEqual(second); - expect(getPolicyEngineNavItems(country, baseUrl)).toEqual( - getPolicyEngineNavItems(country, baseUrl), - ); + expect(getPolicyEngineTools(country, baseUrl)).toEqual(second); + + const navItems = getPolicyEngineNavItems(country, baseUrl, appUrl); + const expected = getPolicyEngineNavItems(country, baseUrl, appUrl); + expect(navItems).toEqual(expected); + for (const item of navItems) { + item.href = 'mutated'; + for (const row of item.children ?? []) { + row.href = 'mutated'; + } + } + expect(getPolicyEngineNavItems(country, baseUrl, appUrl)).toEqual(expected); }), ); }); diff --git a/tests/layout/PolicyEngineToolsNav.test.tsx b/tests/layout/PolicyEngineToolsNav.test.tsx index ee56b45..743c93d 100644 --- a/tests/layout/PolicyEngineToolsNav.test.tsx +++ b/tests/layout/PolicyEngineToolsNav.test.tsx @@ -47,7 +47,7 @@ async function openMobileMenu() { } describe('PolicyEngineHeader Tools menu', () => { - it('opens the US tools as plain links even with a router link and onNavigate', async () => { + it('opens the main app and the US tools as plain links even with a router link and onNavigate', async () => { const onNavigate = vi.fn(); render( { await openDesktopMenu('Tools'); const rows = [ + ['PolicyEngine app', 'https://app.policyengine.org/us'], ['CliffWatch', 'https://policyengine.org/us/cliffwatch'], - ['OBBBA household explorer', 'https://policyengine.org/us/obbba-households'], - ['State legislative tracker', 'https://policyengine.org/us/bill-tracker'], - ['All interactive tools', 'https://policyengine.org/us/research?types=interactive'], + ['Coverage Compass', 'https://policyengine.org/us/coverage-compass'], + ['GiveCalc', 'https://policyengine.org/us/givecalc'], + ['Marriage calculator', 'https://policyengine.org/us/marriage'], + ['QBI deduction calculator', 'https://policyengine.org/us/qbi-calculator'], + ['SPM threshold calculator', 'https://policyengine.org/us/spm-calculator'], + ['TANF calculator', 'https://policyengine.org/us/tanf-calculator'], ]; - for (const [name, href] of rows) { - const link = screen.getByRole('link', { name }); - expect(link).toHaveAttribute('href', href); + const panel = screen + .getByRole('link', { name: 'CliffWatch' }) + .closest('div[style*="position: absolute"]')!; + expect( + within(panel) + .getAllByRole('link') + .map((link) => [link.textContent, link.getAttribute('href')]), + ).toEqual(rows); + for (const link of within(panel).getAllByRole('link')) { expect(link).not.toHaveAttribute('data-client-link'); } - await userEvent.click(screen.getByRole('link', { name: 'OBBBA household explorer' })); + await userEvent.click(screen.getByRole('link', { name: 'QBI deduction calculator' })); expect(onNavigate).not.toHaveBeenCalled(); expect(clickWasPrevented).toEqual([false]); // The click also closes the dropdown, for a browser that keeps the page // (a middle click, or a click the browser refuses to follow). await waitFor(() => - expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(), + expect(screen.queryByRole('link', { name: 'PolicyEngine app' })).toBeNull(), ); }); @@ -104,7 +114,7 @@ describe('PolicyEngineHeader Tools menu', () => { await openDesktopMenu('Tools'); const panel = screen - .getByRole('link', { name: 'OBBBA household explorer' }) + .getByRole('link', { name: 'QBI deduction calculator' }) .closest('div[style*="position: absolute"]'); expect(panel).not.toBeNull(); expect(panel!.contains(screen.getByRole('link', { name: 'CliffWatch' }))).toBe( @@ -115,24 +125,49 @@ describe('PolicyEngineHeader Tools menu', () => { expect(panel!.style.maxWidth).toBe('min(28rem, calc(100vw - 32px))'); }); - it('lists only UK CliffWatch and the full list for the UK', async () => { + it('lists the main app and the UK tools for the UK', async () => { render(); await openDesktopMenu('Tools'); - expect(screen.getByRole('link', { name: 'UK CliffWatch' })).toHaveAttribute( - 'href', - 'https://policyengine.org/uk/uk-cliff-watch', - ); + const panel = screen + .getByRole('link', { name: 'UK CliffWatch' }) + .closest('div[style*="position: absolute"]')!; expect( - screen.getByRole('link', { name: 'All interactive tools' }), - ).toHaveAttribute('href', 'https://policyengine.org/uk/research?types=interactive'); + within(panel) + .getAllByRole('link') + .map((link) => [link.textContent, link.getAttribute('href')]), + ).toEqual([ + ['PolicyEngine app', 'https://app.policyengine.org/uk'], + ['Marriage calculator', 'https://policyengine.org/uk/marriage'], + ['Student loan calculator', 'https://policyengine.org/uk/uk-student-loan-calculator'], + ['UK CliffWatch', 'https://policyengine.org/uk/uk-cliff-watch'], + ]); expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(); - expect(screen.queryByRole('link', { name: /OBBBA/ })).toBeNull(); for (const link of document.querySelectorAll('a')) { expect(link.getAttribute('href')).not.toMatch(/\/uk\/cliffwatch(\/|\?|$)/); } }); + it('links the main app on the appUrl it is given', async () => { + render( + , + ); + + await openDesktopMenu('Tools'); + expect(screen.getByRole('link', { name: 'PolicyEngine app' })).toHaveAttribute( + 'href', + 'https://app.staging.policyengine.org/uk', + ); + expect(screen.getByRole('link', { name: 'UK CliffWatch' })).toHaveAttribute( + 'href', + 'https://staging.policyengine.org/uk/uk-cliff-watch', + ); + }); + it('has no Tools menu for another country', async () => { render(); @@ -142,7 +177,7 @@ describe('PolicyEngineHeader Tools menu', () => { expect(within(menu).getByText('Research')).toBeInTheDocument(); }); - it('lists the tools as plain links in the mobile menu', async () => { + it('lists the main app and the tools as plain links in the mobile menu', async () => { const onNavigate = vi.fn(); render( { const menu = await openMobileMenu(); expect(within(menu).getByText('Tools')).toBeInTheDocument(); + const mainApp = within(menu).getByRole('link', { name: 'PolicyEngine app' }); + expect(mainApp).toHaveAttribute('href', 'https://app.policyengine.org/us'); + expect(mainApp).not.toHaveAttribute('data-client-link'); const cliffWatch = within(menu).getByRole('link', { name: 'CliffWatch' }); expect(cliffWatch).toHaveAttribute('href', 'https://policyengine.org/us/cliffwatch'); expect(cliffWatch).not.toHaveAttribute('data-client-link'); diff --git a/tests/layout/featuredToolsParity.network.test.ts b/tests/layout/featuredToolsParity.network.test.ts deleted file mode 100644 index 9edd5d5..0000000 --- a/tests/layout/featuredToolsParity.network.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -// @vitest-environment node -import { describe, expect, it } from 'vitest'; -import { policyEngineFeaturedTools } from '../../src/layout/PolicyEngineFeaturedTools'; - -/** - * policyengine.org's own header and homepage read their Tools list from this - * file in PolicyEngine/policyengine-app-v2. ui-kit keeps a copy because - * app-v2 is still on ui-kit 0.4, so it can't import the list from here yet. - * Once app-v2 reads it from ui-kit and deletes its own file, delete this test. - * - * This reads app-v2's main branch over the network, so `bun run test` skips - * it. .github/workflows/featured-tools-parity.yml runs it when this copy - * changes and weekly, to catch a change on policyengine.org while nothing - * changes here. - */ -const APP_V2_FEATURED_TOOLS_URL = - 'https://raw.githubusercontent.com/PolicyEngine/policyengine-app-v2/main/app/src/data/featuredTools.json'; - -describe('featured tools parity with policyengine.org', () => { - it('matches app-v2’s featuredTools.json exactly', async () => { - const response = await fetch(APP_V2_FEATURED_TOOLS_URL, { - signal: AbortSignal.timeout(15_000), - }); - expect(response.ok, `GET ${APP_V2_FEATURED_TOOLS_URL}: ${response.status}`).toBe( - true, - ); - const text = await response.text(); - let appV2Tools: unknown; - try { - appV2Tools = JSON.parse(text); - } catch (error) { - throw new Error( - `${APP_V2_FEATURED_TOOLS_URL} is not JSON (${String(error)}): ${text.slice(0, 200)}`, - ); - } - - expect( - policyEngineFeaturedTools, - 'src/layout/PolicyEngineFeaturedTools.ts differs from app-v2’s ' + - 'app/src/data/featuredTools.json; change them together so zone ' + - 'apps and policyengine.org show the same Tools menu', - ).toStrictEqual(appV2Tools); - }, 20_000); -}); diff --git a/tests/layout/toolsMenuParity.network.test.ts b/tests/layout/toolsMenuParity.network.test.ts new file mode 100644 index 0000000..4e05d6a --- /dev/null +++ b/tests/layout/toolsMenuParity.network.test.ts @@ -0,0 +1,113 @@ +// @vitest-environment node +import { describe, expect, it } from 'vitest'; +import { + DEFAULT_POLICYENGINE_APP_URL, + POLICYENGINE_APP_LABEL, + policyEngineTools, +} from '../../src/layout/PolicyEngineTools'; + +/** + * policyengine.org builds its Tools menu from app-v2's apps.json: the main + * app, then every entry with a `toolsMenu` label, by country, sorted by + * label (app/src/utils/toolsMenu.ts and website/src/lib/toolsMenu.ts). + * ui-kit can't import apps.json, so src/layout/PolicyEngineTools.ts keeps a + * copy, and this test applies the same rule to app-v2's main branch and + * compares. + * + * It reads over the network, so `bun run test` skips it. + * .github/workflows/tools-menu-parity.yml runs it when the copy changes and + * weekly, to catch a change on policyengine.org while nothing changes here. + */ +const APP_V2 = + 'https://raw.githubusercontent.com/PolicyEngine/policyengine-app-v2/main'; +const APPS_JSON_URL = `${APP_V2}/app/src/data/apps/apps.json`; +const TOOLS_MENU_URL = `${APP_V2}/app/src/utils/toolsMenu.ts`; +const CONSTANTS_URL = `${APP_V2}/app/src/constants.ts`; + +/** GET `url`, retrying once if the connection fails (not on an HTTP error). */ +async function fetchText(url: string): Promise { + let response: Response; + try { + response = await fetch(url, { signal: AbortSignal.timeout(8_000) }); + } catch { + response = await fetch(url, { signal: AbortSignal.timeout(8_000) }); + } + if (!response.ok) { + throw new Error(`GET ${url}: ${response.status}`); + } + return response.text(); +} + +/** The first capture of `pattern` in app-v2's `url`, or a readable failure. */ +async function readFromSource(url: string, pattern: RegExp): Promise { + const match = (await fetchText(url)).match(pattern); + if (!match) { + throw new Error(`${url} no longer matches ${pattern}; update this test`); + } + return match[1]; +} + +interface AppEntry { + slug: string; + countryId: string; + toolsMenu?: { label: string } | null; +} + +/** app-v2's rule (getToolsMenuEntries) applied to its apps.json. */ +function toolsByCountry(apps: AppEntry[]) { + const byCountry: Record = {}; + for (const app of apps) { + if (!app.toolsMenu) continue; + if (typeof app.toolsMenu.label !== 'string') { + throw new Error(`apps.json ${app.countryId}/${app.slug}: toolsMenu has no label`); + } + (byCountry[app.countryId] ??= []).push({ + slug: app.slug, + label: app.toolsMenu.label, + }); + } + for (const tools of Object.values(byCountry)) { + tools.sort((a, b) => a.label.localeCompare(b.label, 'en')); + } + return byCountry; +} + +describe('Tools menu parity with policyengine.org', () => { + it('lists the same tools as app-v2’s apps.json, in the same order', async () => { + const text = await fetchText(APPS_JSON_URL); + let apps: unknown; + try { + apps = JSON.parse(text); + } catch (error) { + throw new Error( + `${APPS_JSON_URL} is not JSON (${String(error)}): ${text.slice(0, 200)}`, + ); + } + expect(Array.isArray(apps), `${APPS_JSON_URL} is not an array`).toBe(true); + + expect( + policyEngineTools, + 'src/layout/PolicyEngineTools.ts differs from the toolsMenu entries in ' + + 'app-v2’s app/src/data/apps/apps.json; update the copy so zone apps ' + + 'and policyengine.org show the same Tools menu', + ).toStrictEqual(toolsByCountry(apps as AppEntry[])); + }, 20_000); + + it('names and links the main app as app-v2 does', async () => { + expect( + POLICYENGINE_APP_LABEL, + 'POLICYENGINE_APP_LABEL differs from MAIN_APP_LABEL in app-v2', + ).toBe( + await readFromSource(TOOLS_MENU_URL, /MAIN_APP_LABEL\s*=\s*['"]([^'"]+)['"]/), + ); + expect( + DEFAULT_POLICYENGINE_APP_URL, + 'DEFAULT_POLICYENGINE_APP_URL differs from CALCULATOR_URL’s default in app-v2', + ).toBe( + await readFromSource( + CONSTANTS_URL, + /CALCULATOR_URL\s*=[^\n]*\|\|\s*['"]([^'"]+)['"]/, + ), + ); + }, 35_000); +}); From ae7480855da307f3862b6e7ad90c7352d143fcbc Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 26 Oct 2026 06:20:50 -0700 Subject: [PATCH 5/8] fixup! Build the Tools menu by policyengine.org's rule --- src/layout/PolicyEngineShell.tsx | 13 ++++++++-- src/layout/PolicyEngineSiteConfig.ts | 13 ++++++++-- src/layout/PolicyEngineTools.ts | 3 +++ src/layout/header/HeaderMobileMenu.tsx | 15 ++++++++--- src/layout/header/HeaderNavItem.tsx | 36 +++++++++++++++++++++++--- src/layout/index.ts | 1 + src/primitives/Sheet.tsx | 8 +++++- 7 files changed, 76 insertions(+), 13 deletions(-) diff --git a/src/layout/PolicyEngineShell.tsx b/src/layout/PolicyEngineShell.tsx index f17dc59..3840989 100644 --- a/src/layout/PolicyEngineShell.tsx +++ b/src/layout/PolicyEngineShell.tsx @@ -5,13 +5,16 @@ import { cn } from '../utils/cn'; import { PolicyEngineFooter, type PolicyEngineFooterProps } from './PolicyEngineFooter'; import { PolicyEngineHeader, type PolicyEngineHeaderProps } from './PolicyEngineHeader'; import { DEFAULT_POLICYENGINE_BASE_URL, type PolicyEngineCountryId } from './PolicyEngineSiteConfig'; +import { DEFAULT_POLICYENGINE_APP_URL } from './PolicyEngineTools'; export interface PolicyEngineShellProps { children: ReactNode; country?: PolicyEngineCountryId; baseUrl?: string; + /** The main app's origin, which the header's Tools menu links to first. */ + appUrl?: string; showFooter?: boolean; - headerProps?: Omit; + headerProps?: Omit; footerProps?: Omit; className?: string; mainClassName?: string; @@ -21,6 +24,7 @@ export function PolicyEngineShell({ children, country = 'us', baseUrl = DEFAULT_POLICYENGINE_BASE_URL, + appUrl = DEFAULT_POLICYENGINE_APP_URL, showFooter = true, headerProps, footerProps, @@ -29,7 +33,12 @@ export function PolicyEngineShell({ }: PolicyEngineShellProps) { return (
- +
{children}
{showFooter && ( diff --git a/src/layout/PolicyEngineSiteConfig.ts b/src/layout/PolicyEngineSiteConfig.ts index 372cdd7..9b91e92 100644 --- a/src/layout/PolicyEngineSiteConfig.ts +++ b/src/layout/PolicyEngineSiteConfig.ts @@ -64,12 +64,21 @@ export function getPolicyEngineUrl( return `${getPolicyEngineCountryUrl(country, baseUrl)}${normalizedPath}`; } +/** A tool with its absolute link. */ +export interface PolicyEngineToolLink { + slug: string; + label: string; + href: string; +} + /** A country's tools, sorted by label, with absolute links, or none. */ export function getPolicyEngineTools( country: PolicyEngineCountryId = 'us', baseUrl = DEFAULT_POLICYENGINE_BASE_URL, -): { slug: string; label: string; href: string }[] { - const tools = Object.hasOwn(policyEngineTools, country) +): PolicyEngineToolLink[] { + // Own keys only, so `__proto__` or `constructor` never finds a list. + // (Object.hasOwn would do, but Safari added it only in 15.4.) + const tools = Object.prototype.hasOwnProperty.call(policyEngineTools, country) ? (policyEngineTools[country] ?? []) : []; return tools.map((tool) => ({ diff --git a/src/layout/PolicyEngineTools.ts b/src/layout/PolicyEngineTools.ts index 646e0d2..01f1913 100644 --- a/src/layout/PolicyEngineTools.ts +++ b/src/layout/PolicyEngineTools.ts @@ -34,6 +34,9 @@ function freezeTools( * when this copy, the main app's label or its URL differ from app-v2's, so * change them when apps.json changes. The data is frozen because every * header in a bundle shares it. + * + * This export may move: the plan is for app-v2 and ui-kit to read one list, + * after which this copy goes. */ export const policyEngineTools: Readonly< Partial> diff --git a/src/layout/header/HeaderMobileMenu.tsx b/src/layout/header/HeaderMobileMenu.tsx index 050707b..10a0801 100644 --- a/src/layout/header/HeaderMobileMenu.tsx +++ b/src/layout/header/HeaderMobileMenu.tsx @@ -72,19 +72,26 @@ export function HeaderMobileMenu({ className="w-[300px]" style={{ backgroundColor: 'var(--color-teal-600)', - // The full nav is taller than a short phone, so let it scroll. - overflowY: 'auto', // Above the sticky header (zIndex 1000), which would otherwise - // cover the sheet's title and close button. + // cover the sheet's title and close button and stay undimmed. zIndex: 1001, }} + overlayStyle={{ zIndex: 1001 }} > Menu + {/* The full nav is taller than a short phone, so the links scroll + under the title and close button, which stay put. */}
{navItems.map((item) => item.children && item.children.length > 0 ? ( diff --git a/src/layout/header/HeaderNavItem.tsx b/src/layout/header/HeaderNavItem.tsx index 61881b3..f864286 100644 --- a/src/layout/header/HeaderNavItem.tsx +++ b/src/layout/header/HeaderNavItem.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useLayoutEffect, useRef, useState } from 'react'; import { ChevronDown } from 'lucide-react'; import type { NavItemConfig } from './Header'; @@ -29,6 +29,9 @@ const hoverHandlers = { }, }; +/** The closest the dropdown panel comes to either side of the viewport. */ +const VIEWPORT_MARGIN = 16; + const dropdownItemStyle: React.CSSProperties = { display: 'flex', alignItems: 'center', @@ -67,8 +70,32 @@ function AppleDropdown({ linkComponent?: React.ElementType; }) { const contentRef = useRef(null); + const panelRef = useRef(null); const [contentHeight, setContentHeight] = useState(0); const [visible, setVisible] = useState(false); + // How far to move the panel sideways from centred under its trigger. + const [shift, setShift] = useState(0); + + // A panel wider than its trigger, under an item near the edge, would run + // off the viewport and add a horizontal scrollbar, so keep it on screen. + useLayoutEffect(() => { + const panel = panelRef.current; + if (!open || !panel) return; + const rect = panel.getBoundingClientRect(); + if (rect.width === 0) return; // Not laid out. + const left = rect.left - shift; + const right = rect.right - shift; + const viewportWidth = document.documentElement.clientWidth; + let next = 0; + if (right > viewportWidth - VIEWPORT_MARGIN) { + next = viewportWidth - VIEWPORT_MARGIN - right; + } + if (left + next < VIEWPORT_MARGIN) { + next = VIEWPORT_MARGIN - left; + } + setShift(next); + // Measure once per opening; `shift` is read only to undo the last one. + }, [open]); useEffect(() => { if (open && contentRef.current) { @@ -100,13 +127,14 @@ function AppleDropdown({ /> {/* Dropdown panel */}
& { side?: 'top' | 'right' | 'bottom' | 'left'; showCloseButton?: boolean; + /** Class names for the dimmed overlay behind the sheet. */ + overlayClassName?: string; + /** Style overrides for the dimmed overlay behind the sheet. */ + overlayStyle?: React.CSSProperties; }) { return ( - + Date: Mon, 26 Oct 2026 06:34:44 -0700 Subject: [PATCH 6/8] Address the second review of the Tools menu - Keep a dropdown panel 16px inside the viewport when it would run past either edge, measuring afresh on each opening. - Scroll the mobile menu's links under a fixed title and close button, and lift the sheet's overlay above the sticky header too (SheetContent gains overlayClassName and overlayStyle). - PolicyEngineShell takes appUrl; export PolicyEngineToolLink; avoid Object.hasOwn for Safari before 15.4. - The parity check also reads the website header's MAIN_APP_LABEL and CALCULATOR_URL, and says what it can't see. - Tests for each, including the mobile rows in order. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/tools-menu-parity.yml | 6 + changelog.d/tools-nav-menu.added.md | 2 +- changelog.d/tools-nav-menu.changed.md | 2 +- changelog.d/tools-nav-menu.fixed.md | 2 +- tests/consumer-types/fixture.ts | 9 + tests/layout/PolicyEngineToolsNav.test.tsx | 239 +++++++++++++++++-- tests/layout/toolsMenuParity.network.test.ts | 39 +-- 7 files changed, 259 insertions(+), 40 deletions(-) diff --git a/.github/workflows/tools-menu-parity.yml b/.github/workflows/tools-menu-parity.yml index f0a578d..abef03a 100644 --- a/.github/workflows/tools-menu-parity.yml +++ b/.github/workflows/tools-menu-parity.yml @@ -11,12 +11,18 @@ on: - src/layout/PolicyEngineTools.ts - tests/layout/toolsMenuParity.network.test.ts - .github/workflows/tools-menu-parity.yml + # How the test runs: the test:network script and the vitest exclude. + - package.json + - vitest.config.ts push: branches: [main] paths: - src/layout/PolicyEngineTools.ts - tests/layout/toolsMenuParity.network.test.ts - .github/workflows/tools-menu-parity.yml + # How the test runs: the test:network script and the vitest exclude. + - package.json + - vitest.config.ts schedule: - cron: "0 14 * * 1" workflow_dispatch: diff --git a/changelog.d/tools-nav-menu.added.md b/changelog.d/tools-nav-menu.added.md index 5db0ddb..7708c0d 100644 --- a/changelog.d/tools-nav-menu.added.md +++ b/changelog.d/tools-nav-menu.added.md @@ -1 +1 @@ -Add a Tools menu to the PolicyEngine header, right after Research, built by the same rule as policyengine.org's: the PolicyEngine app first, then every calculator that works for any household under current law, sorted by label. The US menu lists CliffWatch, Coverage Compass, GiveCalc, the marriage calculator, the QBI deduction calculator, the SPM threshold calculator and the TANF calculator; the UK menu lists the marriage calculator, the student loan calculator and UK CliffWatch; other countries get no menu. Every row is a full page load, because the PolicyEngine app is on its own domain and each tool runs as its own app on policyengine.org. New exports: `policyEngineTools`, `getPolicyEngineTools`, `getPolicyEngineAppUrl`, `POLICYENGINE_APP_LABEL`, `DEFAULT_POLICYENGINE_APP_URL` and the `PolicyEngineTool` and `NavItemChildConfig` types. `getPolicyEngineNavItems` takes an optional app URL as its third argument, and `PolicyEngineHeader` an `appUrl` prop. `NavItemConfig` items and their dropdown rows take a new `external` flag that renders a plain link, bypassing `linkComponent` and `onNavigate`; rows inherit it from their item unless they set their own. +Add a Tools menu to the PolicyEngine header, right after Research, built by the same rule as policyengine.org's: the PolicyEngine app first, then every calculator that works for any household under current law, sorted by label. The US menu lists CliffWatch, Coverage Compass, GiveCalc, the marriage calculator, the QBI deduction calculator, the SPM threshold calculator and the TANF calculator; the UK menu lists the marriage calculator, the student loan calculator and UK CliffWatch; other countries get no menu. Every row is a full page load, because the PolicyEngine app is on its own domain and each tool runs as its own app on policyengine.org. New exports: `policyEngineTools`, `getPolicyEngineTools`, `getPolicyEngineAppUrl`, `POLICYENGINE_APP_LABEL`, `DEFAULT_POLICYENGINE_APP_URL` and the `PolicyEngineTool`, `PolicyEngineToolLink` and `NavItemChildConfig` types. `getPolicyEngineNavItems` takes an optional app URL as its third argument, and `PolicyEngineHeader` and `PolicyEngineShell` an `appUrl` prop. `SheetContent` takes `overlayClassName` and `overlayStyle` for its dimmed overlay. `NavItemConfig` items and their dropdown rows take a new `external` flag that renders a plain link, bypassing `linkComponent` and `onNavigate`; rows inherit it from their item unless they set their own. diff --git a/changelog.d/tools-nav-menu.changed.md b/changelog.d/tools-nav-menu.changed.md index 7c69d56..ca80f90 100644 --- a/changelog.d/tools-nav-menu.changed.md +++ b/changelog.d/tools-nav-menu.changed.md @@ -1 +1 @@ -Header dropdowns now widen to fit their longest row, up to 28rem or the viewport width less 32px, instead of wrapping every row at 220px. +Header dropdowns now widen to fit their longest row, up to 28rem or the viewport width less 32px, instead of wrapping every row at 220px. A dropdown that would run past either edge of the viewport moves sideways to stay 16px inside it. diff --git a/changelog.d/tools-nav-menu.fixed.md b/changelog.d/tools-nav-menu.fixed.md index 919f4a0..b608921 100644 --- a/changelog.d/tools-nav-menu.fixed.md +++ b/changelog.d/tools-nav-menu.fixed.md @@ -1 +1 @@ -The mobile header menu now scrolls when it is taller than the screen, so Donate and other low items stay reachable on short phones, and it sits above the sticky header instead of under it. +The mobile header menu now scrolls its links when they are taller than the screen, so Donate and other low items stay reachable on short phones, while the menu's title and close button stay in place. The menu and its dimmed overlay now sit above the sticky header instead of under it. diff --git a/tests/consumer-types/fixture.ts b/tests/consumer-types/fixture.ts index 9202359..949f76c 100644 --- a/tests/consumer-types/fixture.ts +++ b/tests/consumer-types/fixture.ts @@ -33,6 +33,7 @@ import { Select, Separator, Sheet, + SheetContent, Skeleton, Spinner, Switch, @@ -48,6 +49,7 @@ import { Stack, Group, PolicyEngineHeader, + PolicyEngineShell, getPolicyEngineNavItems, getPolicyEngineTools, getPolicyEngineAppUrl, @@ -57,6 +59,7 @@ import { type NavItemConfig, type NavItemChildConfig, type PolicyEngineTool, + type PolicyEngineToolLink, // Inputs CurrencyInput, NumberInput, @@ -126,6 +129,9 @@ export const _ToolsNavPaths: { tool: PolicyEngineTool | undefined; mainApp: NavItemChildConfig; appUrlProp: Parameters[0]["appUrl"]; + shellAppUrlProp: Parameters[0]["appUrl"]; + toolLink: PolicyEngineToolLink | undefined; + sheetOverlayStyle: Parameters[0]["overlayStyle"]; } = { external: true, row: { label: "CliffWatch", href: "/us/cliffwatch", external: true }, @@ -136,6 +142,9 @@ export const _ToolsNavPaths: { external: true, }, appUrlProp: DEFAULT_POLICYENGINE_APP_URL, + shellAppUrlProp: DEFAULT_POLICYENGINE_APP_URL, + toolLink: getPolicyEngineTools("uk")[0], + sheetOverlayStyle: { zIndex: 1001 }, }; // Touch each binding so unused-import settings don't strip them. diff --git a/tests/layout/PolicyEngineToolsNav.test.tsx b/tests/layout/PolicyEngineToolsNav.test.tsx index 743c93d..79f8056 100644 --- a/tests/layout/PolicyEngineToolsNav.test.tsx +++ b/tests/layout/PolicyEngineToolsNav.test.tsx @@ -3,6 +3,7 @@ import { render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Header } from '../../src/layout/header'; import { PolicyEngineHeader } from '../../src/layout/PolicyEngineHeader'; +import { PolicyEngineShell } from '../../src/layout/PolicyEngineShell'; /** A client-side link: marks what it renders so tests can tell it apart. */ function ClientLink({ @@ -46,6 +47,29 @@ async function openMobileMenu() { return screen.getByRole('dialog'); } +/** Each link in a mobile menu group as [text, href, routed by the app]. */ +function mobileGroupRows(menu: HTMLElement, label: string) { + const rows = within(menu).getByText(label).nextElementSibling as HTMLElement; + return within(rows) + .getAllByRole('link') + .map((link) => [ + link.textContent, + link.getAttribute('href'), + link.hasAttribute('data-client-link'), + ]); +} + +const US_ROWS = [ + ['PolicyEngine app', 'https://app.policyengine.org/us'], + ['CliffWatch', 'https://policyengine.org/us/cliffwatch'], + ['Coverage Compass', 'https://policyengine.org/us/coverage-compass'], + ['GiveCalc', 'https://policyengine.org/us/givecalc'], + ['Marriage calculator', 'https://policyengine.org/us/marriage'], + ['QBI deduction calculator', 'https://policyengine.org/us/qbi-calculator'], + ['SPM threshold calculator', 'https://policyengine.org/us/spm-calculator'], + ['TANF calculator', 'https://policyengine.org/us/tanf-calculator'], +]; + describe('PolicyEngineHeader Tools menu', () => { it('opens the main app and the US tools as plain links even with a router link and onNavigate', async () => { const onNavigate = vi.fn(); @@ -58,16 +82,6 @@ describe('PolicyEngineHeader Tools menu', () => { ); await openDesktopMenu('Tools'); - const rows = [ - ['PolicyEngine app', 'https://app.policyengine.org/us'], - ['CliffWatch', 'https://policyengine.org/us/cliffwatch'], - ['Coverage Compass', 'https://policyengine.org/us/coverage-compass'], - ['GiveCalc', 'https://policyengine.org/us/givecalc'], - ['Marriage calculator', 'https://policyengine.org/us/marriage'], - ['QBI deduction calculator', 'https://policyengine.org/us/qbi-calculator'], - ['SPM threshold calculator', 'https://policyengine.org/us/spm-calculator'], - ['TANF calculator', 'https://policyengine.org/us/tanf-calculator'], - ]; const panel = screen .getByRole('link', { name: 'CliffWatch' }) .closest('div[style*="position: absolute"]')!; @@ -75,7 +89,7 @@ describe('PolicyEngineHeader Tools menu', () => { within(panel) .getAllByRole('link') .map((link) => [link.textContent, link.getAttribute('href')]), - ).toEqual(rows); + ).toEqual(US_ROWS); for (const link of within(panel).getAllByRole('link')) { expect(link).not.toHaveAttribute('data-client-link'); } @@ -168,6 +182,36 @@ describe('PolicyEngineHeader Tools menu', () => { ); }); + it('passes the shell appUrl to the header', async () => { + const { unmount } = render(Tool content); + await openDesktopMenu('Tools'); + expect(screen.getByRole('link', { name: 'PolicyEngine app' })).toHaveAttribute( + 'href', + 'https://app.policyengine.org/uk', + ); + unmount(); + + render( + + Tool content + , + ); + + await openDesktopMenu('Tools'); + expect(screen.getByRole('link', { name: 'PolicyEngine app' })).toHaveAttribute( + 'href', + 'https://app.staging.policyengine.org/us', + ); + expect(screen.getByRole('link', { name: 'CliffWatch' })).toHaveAttribute( + 'href', + 'https://staging.policyengine.org/us/cliffwatch', + ); + }); + it('has no Tools menu for another country', async () => { render(); @@ -188,30 +232,60 @@ describe('PolicyEngineHeader Tools menu', () => { ); const menu = await openMobileMenu(); - expect(within(menu).getByText('Tools')).toBeInTheDocument(); - const mainApp = within(menu).getByRole('link', { name: 'PolicyEngine app' }); - expect(mainApp).toHaveAttribute('href', 'https://app.policyengine.org/us'); - expect(mainApp).not.toHaveAttribute('data-client-link'); - const cliffWatch = within(menu).getByRole('link', { name: 'CliffWatch' }); - expect(cliffWatch).toHaveAttribute('href', 'https://policyengine.org/us/cliffwatch'); - expect(cliffWatch).not.toHaveAttribute('data-client-link'); + expect(mobileGroupRows(menu, 'Tools')).toEqual( + US_ROWS.map(([label, href]) => [label, href, false]), + ); expect(within(menu).getByRole('link', { name: 'Team' })).toHaveAttribute( 'data-client-link', ); - await userEvent.click(cliffWatch); + await userEvent.click(within(menu).getByRole('link', { name: 'CliffWatch' })); expect(onNavigate).not.toHaveBeenCalled(); expect(clickWasPrevented).toEqual([false]); expect(screen.queryByRole('dialog')).toBeNull(); }); - it('lets the mobile menu scroll above the sticky header', async () => { + it('lists the UK tools in order in the mobile menu', async () => { + render(); + + const menu = await openMobileMenu(); + expect(mobileGroupRows(menu, 'Tools')).toEqual([ + ['PolicyEngine app', 'https://app.policyengine.org/uk', false], + ['Marriage calculator', 'https://policyengine.org/uk/marriage', false], + [ + 'Student loan calculator', + 'https://policyengine.org/uk/uk-student-loan-calculator', + false, + ], + ['UK CliffWatch', 'https://policyengine.org/uk/uk-cliff-watch', false], + ]); + }); + + it('scrolls the mobile links under a fixed title, above the sticky header', async () => { render(); + const headerZ = Number( + document.querySelector('div[style*="position: sticky"]')!.style.zIndex, + ); + expect(headerZ).toBe(1000); const menu = await openMobileMenu(); - expect(menu.style.overflowY).toBe('auto'); - expect(Number(menu.style.zIndex)).toBeGreaterThan(1000); - expect(within(menu).getByRole('link', { name: 'Donate' })).toBeInTheDocument(); + expect(Number(menu.style.zIndex)).toBeGreaterThan(headerZ); + const overlay = document.querySelector('[data-slot="sheet-overlay"]')!; + expect(Number(overlay.style.zIndex)).toBeGreaterThan(headerZ); + + // The sheet itself doesn't scroll; the list under its title does. + expect(menu.style.overflowY).toBe(''); + const list = within(menu) + .getByRole('link', { name: 'Donate' }) + .closest('div[style*="overflow-y: auto"]')!; + expect(list).not.toBeNull(); + expect(menu.contains(list)).toBe(true); + expect(parseFloat(list.style.minHeight)).toBe(0); + expect(list.style.flex).toMatch(/^1( |$)/); + expect(list.contains(within(menu).getByText('Menu'))).toBe(false); + expect(list.contains(within(menu).getByRole('button', { name: 'Close' }))).toBe( + false, + ); }); it('leaves mobile tool links to the browser when there is no router', async () => { @@ -230,6 +304,125 @@ describe('PolicyEngineHeader Tools menu', () => { }); }); +describe('Header dropdown position', () => { + // Not jsdom's window.innerWidth (1024), which counts the scrollbar. + const VIEWPORT_WIDTH = 1000; + // Where the dropdown panel sits, unshifted, before the header moves it. + let panelLeft = 0; + let panelWidth = 0; + + /** The shift the panel's transform applies, in px. */ + function shiftOf(element: HTMLElement) { + const match = element.style.transform.match(/calc\(-50% \+ (-?[\d.]+)px\)/); + return match ? Number(match[1]) : 0; + } + + function isPanel(element: Element): element is HTMLElement { + return ( + element instanceof HTMLElement && + element.style.position === 'absolute' && + element.style.width === 'max-content' + ); + } + + beforeEach(() => { + Object.defineProperty(document.documentElement, 'clientWidth', { + configurable: true, + value: VIEWPORT_WIDTH, + }); + vi.spyOn(Element.prototype, 'getBoundingClientRect').mockImplementation( + function (this: Element) { + if (!isPanel(this)) return new DOMRect(0, 0, 0, 0); + return new DOMRect(panelLeft + shiftOf(this), 0, panelWidth, 100); + }, + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + delete (document.documentElement as { clientWidth?: number }).clientWidth; + }); + + function panel() { + return screen + .getByRole('link', { name: 'CliffWatch' }) + .closest('div[style*="position: absolute"]')!; + } + + it('stays centred under its trigger when it fits', async () => { + panelLeft = 400; + panelWidth = 300; + render(); + + await openDesktopMenu('Tools'); + expect(panel().style.transform).toMatch(/^translateX\(-50%\) /); + }); + + it('leaves a panel it cannot measure centred', async () => { + panelLeft = -50; + panelWidth = 0; + render(); + + await openDesktopMenu('Tools'); + expect(panel().style.transform).toMatch(/^translateX\(-50%\) /); + }); + + it('moves left to keep its right edge 16px inside the viewport', async () => { + panelLeft = 900; + panelWidth = 300; + render(); + + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(VIEWPORT_WIDTH - 16 - 1200); + expect(panel().style.transform).toMatch(/^translateX\(calc\(-50% \+ -216px\)\) /); + }); + + it('moves right to keep its left edge 16px inside the viewport', async () => { + panelLeft = -50; + panelWidth = 300; + render(); + + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(66); + }); + + it('keeps the left edge on screen when the panel is wider than the viewport', async () => { + panelLeft = -40; + panelWidth = 1100; + render(); + + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(56); + }); + + it('measures afresh each time it opens', async () => { + panelLeft = 900; + panelWidth = 300; + render(); + + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(-216); + await userEvent.keyboard('{Escape}'); + await waitFor(() => + expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(), + ); + + // Reopened with the old shift still applied, it lands in the same place. + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(-216); + await userEvent.keyboard('{Escape}'); + await waitFor(() => + expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(), + ); + + // After the window grows, the panel fits and goes back to centred. + panelLeft = 400; + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(0); + expect(panel().style.transform).toMatch(/^translateX\(-50%\) /); + }); +}); + describe('Header external dropdowns', () => { const navItems = [ { diff --git a/tests/layout/toolsMenuParity.network.test.ts b/tests/layout/toolsMenuParity.network.test.ts index 4e05d6a..65ec5df 100644 --- a/tests/layout/toolsMenuParity.network.test.ts +++ b/tests/layout/toolsMenuParity.network.test.ts @@ -17,12 +17,27 @@ import { * It reads over the network, so `bun run test` skips it. * .github/workflows/tools-menu-parity.yml runs it when the copy changes and * weekly, to catch a change on policyengine.org while nothing changes here. + * + * What it can't see: + * - It applies its own copy of the rule rather than running app-v2's code. + * If app-v2 changes the rule itself (another filter, a different sort), + * this test can pass while the menus differ. app-v2's own tests + * (website/src/__tests__/lib/toolsMenu.test.ts) pin its side of the rule. + * - It reads CALCULATOR_URL's default in the code. A deployment that sets + * NEXT_PUBLIC_CALCULATOR_URL or VITE_CALCULATOR_URL links the main app + * somewhere else, and this test can't tell. */ const APP_V2 = 'https://raw.githubusercontent.com/PolicyEngine/policyengine-app-v2/main'; const APPS_JSON_URL = `${APP_V2}/app/src/data/apps/apps.json`; -const TOOLS_MENU_URL = `${APP_V2}/app/src/utils/toolsMenu.ts`; -const CONSTANTS_URL = `${APP_V2}/app/src/constants.ts`; +// The calculator's header and the website's header each name the main app +// and give its URL, so check both. +const APP_TOOLS_MENU_URL = `${APP_V2}/app/src/utils/toolsMenu.ts`; +const APP_CONSTANTS_URL = `${APP_V2}/app/src/constants.ts`; +const WEBSITE_TOOLS_MENU_URL = `${APP_V2}/website/src/lib/toolsMenu.ts`; +const MAIN_APP_LABEL_PATTERN = /MAIN_APP_LABEL\s*=\s*['"]([^'"]+)['"]/; +// `[^;]*?` spans the line break in the website's two-line declaration. +const CALCULATOR_URL_PATTERN = /CALCULATOR_URL\s*=[^;]*?\|\|\s*['"]([^'"]+)['"]/; /** GET `url`, retrying once if the connection fails (not on an HTTP error). */ async function fetchText(url: string): Promise { @@ -93,21 +108,17 @@ describe('Tools menu parity with policyengine.org', () => { ).toStrictEqual(toolsByCountry(apps as AppEntry[])); }, 20_000); - it('names and links the main app as app-v2 does', async () => { + it.each([ + ['the calculator', APP_TOOLS_MENU_URL, APP_CONSTANTS_URL], + ['the website', WEBSITE_TOOLS_MENU_URL, WEBSITE_TOOLS_MENU_URL], + ])('names and links the main app as %s’s header does', async (_, labelUrl, appUrl) => { expect( POLICYENGINE_APP_LABEL, - 'POLICYENGINE_APP_LABEL differs from MAIN_APP_LABEL in app-v2', - ).toBe( - await readFromSource(TOOLS_MENU_URL, /MAIN_APP_LABEL\s*=\s*['"]([^'"]+)['"]/), - ); + `POLICYENGINE_APP_LABEL differs from MAIN_APP_LABEL in ${labelUrl}`, + ).toBe(await readFromSource(labelUrl, MAIN_APP_LABEL_PATTERN)); expect( DEFAULT_POLICYENGINE_APP_URL, - 'DEFAULT_POLICYENGINE_APP_URL differs from CALCULATOR_URL’s default in app-v2', - ).toBe( - await readFromSource( - CONSTANTS_URL, - /CALCULATOR_URL\s*=[^\n]*\|\|\s*['"]([^'"]+)['"]/, - ), - ); + `DEFAULT_POLICYENGINE_APP_URL differs from CALCULATOR_URL’s default in ${appUrl}`, + ).toBe(await readFromSource(appUrl, CALCULATOR_URL_PATTERN)); }, 35_000); }); From 96fdde5dc8617f2b65b0dd4904267570576b3bc9 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 26 Oct 2026 06:55:18 -0700 Subject: [PATCH 7/8] Keep a dropdown on screen as the window changes The delta review found the clamp measured only when the panel opened, so a resize while open could leave it off screen; that a scaled ancestor made the shift the wrong size; and that a shift inside the animated transform could be measured mid-slide. - Place the panel from its trigger's position, on opening and on every resize or scroll while open, removing the listeners on close. - Shift it with `left`, which doesn't animate. - Convert the shift to the panel's own pixels under a scaled ancestor. - Test SheetContent's overlay props and the sheet's column layout. Co-Authored-By: Claude Opus 5.5 --- src/layout/header/HeaderNavItem.tsx | 57 +++++--- tests/layout/PolicyEngineToolsNav.test.tsx | 154 ++++++++++++++++----- tests/primitives/Sheet.test.tsx | 46 ++++++ 3 files changed, 202 insertions(+), 55 deletions(-) create mode 100644 tests/primitives/Sheet.test.tsx diff --git a/src/layout/header/HeaderNavItem.tsx b/src/layout/header/HeaderNavItem.tsx index f864286..54a80e0 100644 --- a/src/layout/header/HeaderNavItem.tsx +++ b/src/layout/header/HeaderNavItem.tsx @@ -73,28 +73,43 @@ function AppleDropdown({ const panelRef = useRef(null); const [contentHeight, setContentHeight] = useState(0); const [visible, setVisible] = useState(false); - // How far to move the panel sideways from centred under its trigger. + // How far to move the panel sideways from centred under its trigger, in + // the panel's own CSS pixels. const [shift, setShift] = useState(0); // A panel wider than its trigger, under an item near the edge, would run - // off the viewport and add a horizontal scrollbar, so keep it on screen. + // off the viewport and add a horizontal scrollbar, so keep it on screen, + // placing it again whenever the window resizes or scrolls while it's open. useLayoutEffect(() => { const panel = panelRef.current; - if (!open || !panel) return; - const rect = panel.getBoundingClientRect(); - if (rect.width === 0) return; // Not laid out. - const left = rect.left - shift; - const right = rect.right - shift; - const viewportWidth = document.documentElement.clientWidth; - let next = 0; - if (right > viewportWidth - VIEWPORT_MARGIN) { - next = viewportWidth - VIEWPORT_MARGIN - right; - } - if (left + next < VIEWPORT_MARGIN) { - next = VIEWPORT_MARGIN - left; + const trigger = panel?.parentElement; + if (!open || !panel || !trigger) return; + function place() { + const { width } = panel!.getBoundingClientRect(); + if (width === 0) return; // Not laid out. + // Where the panel sits centred under its trigger, whatever its shift. + const box = trigger!.getBoundingClientRect(); + const left = (box.left + box.right - width) / 2; + const right = left + width; + const viewportWidth = document.documentElement.clientWidth; + let next = 0; + if (right > viewportWidth - VIEWPORT_MARGIN) { + next = viewportWidth - VIEWPORT_MARGIN - right; + } + if (left + next < VIEWPORT_MARGIN) { + next = VIEWPORT_MARGIN - left; + } + // Under a scaled ancestor, the panel's CSS pixels aren't the viewport's. + const scale = panel!.offsetWidth > 0 ? width / panel!.offsetWidth : 1; + setShift(next / scale); } - setShift(next); - // Measure once per opening; `shift` is read only to undo the last one. + place(); + window.addEventListener('resize', place); + window.addEventListener('scroll', place, { capture: true, passive: true }); + return () => { + window.removeEventListener('resize', place); + window.removeEventListener('scroll', place, { capture: true }); + }; }, [open]); useEffect(() => { @@ -131,10 +146,12 @@ function AppleDropdown({ style={{ position: 'absolute', top: '100%', - left: '50%', - transform: `${ - shift === 0 ? 'translateX(-50%)' : `translateX(calc(-50% + ${shift}px))` - } ${visible ? 'translateY(0)' : 'translateY(-8px)'}`, + // Shifted with `left`, which doesn't animate, so the panel never + // slides sideways and a measurement never catches it mid-slide. + left: shift === 0 ? '50%' : `calc(50% + ${shift}px)`, + transform: visible + ? 'translateX(-50%) translateY(0)' + : 'translateX(-50%) translateY(-8px)', marginTop: '10px', // The trigger is a narrow box, so size the panel to its longest row // instead of wrapping every row at the minimum width, up to a cap diff --git a/tests/layout/PolicyEngineToolsNav.test.tsx b/tests/layout/PolicyEngineToolsNav.test.tsx index 79f8056..4ef8075 100644 --- a/tests/layout/PolicyEngineToolsNav.test.tsx +++ b/tests/layout/PolicyEngineToolsNav.test.tsx @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { render, screen, waitFor, within } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Header } from '../../src/layout/header'; import { PolicyEngineHeader } from '../../src/layout/PolicyEngineHeader'; @@ -273,8 +273,10 @@ describe('PolicyEngineHeader Tools menu', () => { const overlay = document.querySelector('[data-slot="sheet-overlay"]')!; expect(Number(overlay.style.zIndex)).toBeGreaterThan(headerZ); - // The sheet itself doesn't scroll; the list under its title does. + // The sheet itself doesn't scroll; the list under its title does. jsdom + // has no layout, so this checks the wiring: a column the list fills. expect(menu.style.overflowY).toBe(''); + expect(menu).toHaveClass('flex', 'flex-col', 'h-full'); const list = within(menu) .getByRole('link', { name: 'Donate' }) .closest('div[style*="overflow-y: auto"]')!; @@ -306,15 +308,21 @@ describe('PolicyEngineHeader Tools menu', () => { describe('Header dropdown position', () => { // Not jsdom's window.innerWidth (1024), which counts the scrollbar. - const VIEWPORT_WIDTH = 1000; - // Where the dropdown panel sits, unshifted, before the header moves it. - let panelLeft = 0; + let viewportWidth = 1000; + // The centre of the Tools trigger, and the panel's width, in viewport px. + let triggerCentre = 0; let panelWidth = 0; + // The panel's width in its own CSS px is panelWidth / scale. + let scale = 1; - /** The shift the panel's transform applies, in px. */ + /** The shift the panel's `left` applies, in its own CSS px. */ function shiftOf(element: HTMLElement) { - const match = element.style.transform.match(/calc\(-50% \+ (-?[\d.]+)px\)/); - return match ? Number(match[1]) : 0; + const match = element.style.left.match(/^calc\(50% ([+-]) (-?[\d.]+)px\)$/); + if (!match) { + expect(element.style.left).toBe('50%'); + return 0; + } + return (match[1] === '-' ? -1 : 1) * Number(match[2]); } function isPanel(element: Element): element is HTMLElement { @@ -326,14 +334,28 @@ describe('Header dropdown position', () => { } beforeEach(() => { + viewportWidth = 1000; + scale = 1; Object.defineProperty(document.documentElement, 'clientWidth', { configurable: true, - value: VIEWPORT_WIDTH, + get: () => viewportWidth, }); vi.spyOn(Element.prototype, 'getBoundingClientRect').mockImplementation( function (this: Element) { - if (!isPanel(this)) return new DOMRect(0, 0, 0, 0); - return new DOMRect(panelLeft + shiftOf(this), 0, panelWidth, 100); + if (isPanel(this)) { + const left = triggerCentre - panelWidth / 2 + shiftOf(this) * scale; + return new DOMRect(left, 50, panelWidth, 100); + } + // The trigger's box, which the panel hangs from. + if ([...this.children].some(isPanel)) { + return new DOMRect(triggerCentre - 40, 0, 80, 40); + } + return new DOMRect(0, 0, 0, 0); + }, + ); + vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation( + function (this: HTMLElement) { + return isPanel(this) ? panelWidth / scale : 0; }, ); }); @@ -349,45 +371,57 @@ describe('Header dropdown position', () => { .closest('div[style*="position: absolute"]')!; } + async function closeMenu() { + await userEvent.keyboard('{Escape}'); + await waitFor(() => + expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(), + ); + } + it('stays centred under its trigger when it fits', async () => { - panelLeft = 400; + triggerCentre = 550; panelWidth = 300; render(); await openDesktopMenu('Tools'); - expect(panel().style.transform).toMatch(/^translateX\(-50%\) /); + expect(panel().style.left).toBe('50%'); + // Sideways placement stays out of the animated transform. + expect(panel().style.transform).toMatch(/^translateX\(-50%\) translateY\((0|-8px)\)$/); }); it('leaves a panel it cannot measure centred', async () => { - panelLeft = -50; + // Laid out, a panel here would move right; unmeasured, it shouldn't. + triggerCentre = 0; panelWidth = 0; render(); await openDesktopMenu('Tools'); - expect(panel().style.transform).toMatch(/^translateX\(-50%\) /); + expect(panel().style.left).toBe('50%'); }); it('moves left to keep its right edge 16px inside the viewport', async () => { - panelLeft = 900; + // Centred, the panel would span 900 to 1200. + triggerCentre = 1050; panelWidth = 300; render(); await openDesktopMenu('Tools'); - expect(shiftOf(panel())).toBe(VIEWPORT_WIDTH - 16 - 1200); - expect(panel().style.transform).toMatch(/^translateX\(calc\(-50% \+ -216px\)\) /); + expect(shiftOf(panel())).toBe(viewportWidth - 16 - 1200); + expect(panel().getBoundingClientRect().right).toBe(viewportWidth - 16); }); it('moves right to keep its left edge 16px inside the viewport', async () => { - panelLeft = -50; + triggerCentre = 100; panelWidth = 300; render(); await openDesktopMenu('Tools'); expect(shiftOf(panel())).toBe(66); + expect(panel().getBoundingClientRect().left).toBe(16); }); it('keeps the left edge on screen when the panel is wider than the viewport', async () => { - panelLeft = -40; + triggerCentre = 510; panelWidth = 1100; render(); @@ -395,31 +429,81 @@ describe('Header dropdown position', () => { expect(shiftOf(panel())).toBe(56); }); - it('measures afresh each time it opens', async () => { - panelLeft = 900; + it('moves in its own pixels under a scaled ancestor', async () => { + triggerCentre = 1050; + panelWidth = 300; + scale = 0.8; + render(); + + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(-216 / 0.8); + expect(panel().getBoundingClientRect().right).toBeCloseTo(viewportWidth - 16); + }); + + it('places itself again when the window resizes while it is open', async () => { + triggerCentre = 1050; panelWidth = 300; render(); await openDesktopMenu('Tools'); expect(shiftOf(panel())).toBe(-216); - await userEvent.keyboard('{Escape}'); - await waitFor(() => - expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(), - ); - // Reopened with the old shift still applied, it lands in the same place. + // The same window: the panel stays where it is. + fireEvent(window, new Event('resize')); + expect(shiftOf(panel())).toBe(-216); + + viewportWidth = 900; + fireEvent(window, new Event('resize')); + expect(shiftOf(panel())).toBe(900 - 16 - 1200); + + viewportWidth = 1400; + fireEvent(window, new Event('resize')); + expect(panel().style.left).toBe('50%'); + }); + + it('places itself again when a container scrolls while it is open', async () => { + triggerCentre = 550; + panelWidth = 300; + render(); + await openDesktopMenu('Tools'); + expect(panel().style.left).toBe('50%'); + + triggerCentre = 1050; + fireEvent.scroll(document.body); expect(shiftOf(panel())).toBe(-216); - await userEvent.keyboard('{Escape}'); - await waitFor(() => - expect(screen.queryByRole('link', { name: 'CliffWatch' })).toBeNull(), - ); + }); + + it('stops listening once it closes', async () => { + triggerCentre = 550; + panelWidth = 300; + render(); + const add = vi.spyOn(window, 'addEventListener'); + const remove = vi.spyOn(window, 'removeEventListener'); + + await openDesktopMenu('Tools'); + await closeMenu(); + const placed = add.mock.calls.filter(([, listener]) => { + return typeof listener === 'function' && listener.name === 'place'; + }); + expect(placed.map(([type]) => type).sort()).toEqual(['resize', 'scroll']); + for (const [type, listener, options] of placed) { + expect(remove).toHaveBeenCalledWith( + type, + listener, + ...(type === 'scroll' ? [expect.objectContaining({ capture: true })] : []), + ); + expect(options === undefined || type === 'scroll').toBe(true); + } - // After the window grows, the panel fits and goes back to centred. - panelLeft = 400; + // Reopened, it measures afresh. + triggerCentre = 1050; + await openDesktopMenu('Tools'); + expect(shiftOf(panel())).toBe(-216); + await closeMenu(); + triggerCentre = 550; await openDesktopMenu('Tools'); - expect(shiftOf(panel())).toBe(0); - expect(panel().style.transform).toMatch(/^translateX\(-50%\) /); + expect(panel().style.left).toBe('50%'); }); }); diff --git a/tests/primitives/Sheet.test.tsx b/tests/primitives/Sheet.test.tsx new file mode 100644 index 0000000..60139f6 --- /dev/null +++ b/tests/primitives/Sheet.test.tsx @@ -0,0 +1,46 @@ +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { Sheet, SheetContent, SheetDescription, SheetTitle } from '../../src/primitives/Sheet'; + +function overlay() { + return document.querySelector('[data-slot="sheet-overlay"]')!; +} + +describe('SheetContent', () => { + it('styles its overlay without passing the overlay props to the sheet', () => { + render( + + + Title + Description + + , + ); + + expect(overlay()).toHaveClass('custom-overlay', 'fixed', 'inset-0', 'bg-black/50'); + expect(overlay().style.zIndex).toBe('1234'); + const sheet = screen.getByRole('dialog'); + expect(sheet.style.zIndex).toBe('1235'); + expect(sheet).not.toHaveClass('custom-overlay'); + expect(sheet).not.toHaveAttribute('overlayclassname'); + expect(sheet).not.toHaveAttribute('overlaystyle'); + }); + + it('leaves the overlay as it was without them', () => { + render( + + + Title + Description + + , + ); + + expect(overlay()).toHaveClass('fixed', 'inset-0', 'z-50', 'bg-black/50'); + expect(overlay().getAttribute('style') ?? '').not.toMatch(/z-index/); + }); +}); From 4e9098fe076f4bbcc9d08486f40972c1a90a36cd Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 26 Oct 2026 07:05:56 -0700 Subject: [PATCH 8/8] Check that a dropdown's sideways shift never animates Co-Authored-By: Claude Opus 5.5 --- tests/layout/PolicyEngineToolsNav.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/layout/PolicyEngineToolsNav.test.tsx b/tests/layout/PolicyEngineToolsNav.test.tsx index 4ef8075..c60c471 100644 --- a/tests/layout/PolicyEngineToolsNav.test.tsx +++ b/tests/layout/PolicyEngineToolsNav.test.tsx @@ -385,8 +385,11 @@ describe('Header dropdown position', () => { await openDesktopMenu('Tools'); expect(panel().style.left).toBe('50%'); - // Sideways placement stays out of the animated transform. + // Sideways placement stays out of the animated transform, and `left` + // doesn't animate, so the panel never slides past the viewport's edge. expect(panel().style.transform).toMatch(/^translateX\(-50%\) translateY\((0|-8px)\)$/); + expect(panel().style.transition).toContain('transform'); + expect(panel().style.transition).not.toMatch(/(^|,)\s*(left|all)\b/); }); it('leaves a panel it cannot measure centred', async () => {