Fix/311 header updates

apps/philly-311/frontend/src/App.vue

@@ -1,15 +1,16 @@
 <!-- ABOUTME: Root component. Wraps every route in PinboardShell chrome inside a
      phila .content region and supplies the header's nav links and CTAs. -->
 <script setup lang="ts">
-import { PinboardShell, PhilaLink } from '@pinboard/ui'
+import { PinboardShell } from '@pinboard/ui'
 import '@pinboard/ui/style.css'
 import '@/assets/a11y.css'
 import { PhilaButton } from '@phila/phila-ui-button'
 import { Callout } from '@phila/phila-ui-callout'
 import { useAuth } from '@phila/sso-vue'
+import { computed } from 'vue'
 import { useRoute } from 'vue-router'
-import type { NavLink } from '@pinboard/ui'
 import { useAccountProvisioning } from '@/composables/useAccountProvisioning'
+import ReportDocumentIcon from '@/components/ReportDocumentIcon'
 
 const route = useRoute()
 const { signIn, signOut, isAuthenticated, userName } = useAuth()
@@ -19,11 +20,28 @@ const {
   retry: retryAccountProvisioning,
 } = useAccountProvisioning()
 
-const navLinks: NavLink[] = [
-  { text: 'Map', href: '/' },
-  { text: 'My Requests', href: '/reports' },
-  { text: 'Answers', href: '/answers' },
-]
+// Marks a nav link selected when its href matches the current route, exactly
+// or as a path prefix (so e.g. "/answers" stays selected on "/answers/123").
+// "/" is exempt from prefix matching or it would match every route.
+const isSelectedHref = (href?: string) =>
+  href !== undefined && (href === route.path || (href !== '/' && route.path.startsWith(`${href}/`)))
+
+const navLinks = computed(() =>
+  [
+    {
+      text: 'Report an issue',
+      href: '/report',
+      icon: ReportDocumentIcon,
+      iconSize: 'large' as const,
+    },
+    { text: 'Map', href: '/' },
+    { text: 'My Requests', href: '/reports' },
+    { text: 'Answers', href: '/answers' },
+    ...(isAuthenticated.value
+      ? [{ text: userName.value ?? '' }, { text: 'Sign out', href: '#', onClick: () => signOut() }]
+      : [{ text: 'Login / Sign up', href: '#', onClick: () => login() }]),
+  ].map((link) => ({ ...link, selected: isSelectedHref(link.href) })),
+)
 
 const feedbackHref = 'https://www.phila.gov/feedback/'
 
@@ -50,18 +68,6 @@ function login() {
     :show-header-tooltip="false"
     :feedback-href="feedbackHref"
   >
-    <template #navbar-left-end>
-      <PhilaButton variant="primary" to="/report" class="navbar-cta">Report an issue</PhilaButton>
-    </template>
-    <template #navbar-end>
-      <template v-if="isAuthenticated">
-        <span class="navbar-user has-text-label-default">{{ userName }}</span>
-        <PhilaLink href="#" variant="on-primary" @click.prevent="signOut()"> Sign out </PhilaLink>
-      </template>
-      <PhilaLink v-else href="#" variant="on-primary" @click.prevent="login()">
-        Login / Sign up
-      </PhilaLink>
-    </template>
     <div class="content app-content">
       <div v-if="accountStatus === 'pending'" class="account-provisioning-gate" role="status">
         <span class="spinner" aria-hidden="true" />
@@ -90,15 +96,6 @@ function login() {
   display: contents;
 }
 
-.navbar-cta {
-  align-self: center;
-}
-
-.navbar-user {
-  color: var(--Schemes-On-Inverse-Surface-Bright, #fff);
-  padding-right: var(--spacing-3xl);
-}
-
 .account-provisioning-gate {
   display: flex;
   align-items: center;

apps/philly-311/frontend/src/__tests__/App.test.ts

@@ -1,6 +1,7 @@
-// ABOUTME: Tests for App — Map/My Requests/Answers header nav links, the "Report an
-// ABOUTME: issue" CTA, the login/signed-in states, the sub-footer links, and the
-// ABOUTME: phila .content wrapper.
+// ABOUTME: Tests for App — the header's Report-an-issue/Map/My Requests/Answers/
+// ABOUTME: login nav links (now all passed through PinboardShell's `links` prop
+// ABOUTME: rather than app-provided slots), the sub-footer links, and the phila
+// ABOUTME: .content wrapper.
 import { describe, it, expect, vi, beforeEach } from 'vitest'
 import { defineComponent, h, ref, computed } from 'vue'
 import { mount, RouterLinkStub } from '@vue/test-utils'
@@ -14,23 +15,12 @@ vi.mock('@pinboard/ui', () => ({
     setup(_, { slots }) {
       return () =>
         h('div', [
-          h('div', { 'data-test': 'navbar-left-end' }, slots['navbar-left-end']?.()),
-          h('div', { 'data-test': 'navbar-end' }, slots['navbar-end']?.()),
           h('div', { 'data-test': 'mobile-nav' }, slots['mobile-nav']?.()),
           h('div', { 'data-test': 'sub-footer' }, slots['sub-footer']?.()),
           slots.default?.(),
         ])
     },
   }),
-  // Minimal stand-in: renders as a plain <a>, same as the real component's non-router
-  // path (App.vue only ever passes href, never `to`), so href/text/click assertions hold.
-  PhilaLink: defineComponent({
-    name: 'PhilaLink',
-    props: { href: { type: String, default: undefined } },
-    setup(props, { slots }) {
-      return () => h('a', { href: props.href }, slots.default?.())
-    },
-  }),
 }))
 
 const signIn = vi.fn()
@@ -84,40 +74,47 @@ beforeEach(() => {
   retryAccountProvisioning.mockClear()
 })
 
+// Shape of the objects App.vue passes as PinboardShell's `links` prop; `icon` is
+// omitted from most assertions below since it's a component reference, not data.
+type TestNavLink = { text: string; href?: string; selected?: boolean; onClick?: () => void }
+
 describe('App', () => {
-  it('passes Map/My Requests/Answers as the header nav links', async () => {
+  it('passes Report an issue, Map, My Requests, Answers, and Login / Sign up as the header nav links', async () => {
     const w = await mountApp()
     const shell = w.findComponent({ name: 'PinboardShell' })
-    expect(shell.props('links')).toEqual([
+    const links = shell.props('links') as TestNavLink[]
+    expect(links.map(({ text, href }) => ({ text, href }))).toEqual([
+      { text: 'Report an issue', href: '/report' },
       { text: 'Map', href: '/' },
       { text: 'My Requests', href: '/reports' },
       { text: 'Answers', href: '/answers' },
+      { text: 'Login / Sign up', href: '#' },
     ])
   })
 
-  it('puts a "Report an issue" button routing to /report in the navbar-left-end slot', async () => {
+  it('gives the "Report an issue" link a large icon', async () => {
     const w = await mountApp()
-    const navbarLeftEnd = w.find('[data-test="navbar-left-end"]')
-    const link = navbarLeftEnd.find('a')
-    expect(link.exists()).toBe(true)
-    expect(link.attributes('href')).toBe('/report')
-    expect(link.text()).toBe('Report an issue')
+    const shell = w.findComponent({ name: 'PinboardShell' })
+    const links = shell.props('links') as (TestNavLink & { icon?: unknown; iconSize?: string })[]
+    const reportLink = links.find((link) => link.text === 'Report an issue')
+    expect(reportLink?.icon).toBeTruthy()
+    expect(reportLink?.iconSize).toBe('large')
   })
 
-  it('puts a Login / Sign up trigger in the navbar-end slot that starts the sso-vue login flow', async () => {
-    const w = await mountApp()
-    const navbarEnd = w.find('[data-test="navbar-end"]')
-    const loginLink = navbarEnd.findAll('a').find((a) => a.text() === 'Login / Sign up')
-    expect(loginLink).toBeTruthy()
-    await loginLink?.trigger('click')
-    expect(signIn).toHaveBeenCalledOnce()
+  it('marks the nav link matching the current route as selected', async () => {
+    const w = await mountApp('/answers')
+    const shell = w.findComponent({ name: 'PinboardShell' })
+    const links = shell.props('links') as TestNavLink[]
+    expect(links.find((link) => link.text === 'Answers')?.selected).toBe(true)
+    expect(links.find((link) => link.text === 'Map')?.selected).toBe(false)
   })
 
-  it('records the current route as the post-login redirect before starting sign-in', async () => {
+  it('starts the sso-vue login flow and records the current route as the post-login redirect when Login / Sign up is triggered', async () => {
     const w = await mountApp('/report/location')
-    const navbarEnd = w.find('[data-test="navbar-end"]')
-    const loginLink = navbarEnd.findAll('a').find((a) => a.text() === 'Login / Sign up')
-    await loginLink?.trigger('click')
+    const shell = w.findComponent({ name: 'PinboardShell' })
+    const links = shell.props('links') as TestNavLink[]
+    links.find((link) => link.text === 'Login / Sign up')?.onClick?.()
+    expect(signIn).toHaveBeenCalledOnce()
     expect(sessionStorage.getItem('auth:redirectTo')).toBe('/report/location')
   })
 
@@ -152,14 +149,15 @@ describe('App', () => {
     ])
   })
 
-  it('shows the user name and a Sign out button instead of Login when authenticated', async () => {
+  it('shows the user name and a Sign out link instead of Login when authenticated', async () => {
     isAuthenticated.value = true
     const w = await mountApp()
-    const navbarEnd = w.find('[data-test="navbar-end"]')
-    expect(navbarEnd.text()).toContain('Ben Franklin')
-    expect(navbarEnd.findAll('a').find((a) => a.text() === 'Login / Sign up')).toBeFalsy()
-    const signOutLink = navbarEnd.findAll('a').find((a) => a.text() === 'Sign out')
-    await signOutLink?.trigger('click')
+    const shell = w.findComponent({ name: 'PinboardShell' })
+    const links = shell.props('links') as TestNavLink[]
+    expect(links.find((link) => link.text === 'Ben Franklin')).toBeTruthy()
+    expect(links.find((link) => link.text === 'Login / Sign up')).toBeFalsy()
+    const signOutLink = links.find((link) => link.text === 'Sign out')
+    signOutLink?.onClick?.()
     expect(signOut).toHaveBeenCalledOnce()
   })
 })

apps/philly-311/frontend/src/components/ReportCallout.vue

@@ -1,7 +1,8 @@
 <!-- ABOUTME: Landing-header callout: heading + lede + the primary "Submit request" CTA
-     (filled pill with an overlapping document illustration) routing to /report. -->
+     routing to /report. The CTA used to carry its own document illustration, but that
+     glyph now lives on the header's "Report an issue" link, so it's dropped here. -->
 <script setup lang="ts">
-import reportDocument from '@/assets/report-document.svg'
+import { PhilaButton } from '@phila/phila-ui-button'
 </script>
 
 <template>
@@ -18,10 +19,9 @@ import reportDocument from '@/assets/report-document.svg'
         >About Philly311.</a
       >
     </p>
-    <div class="report-callout__cta-row">
-      <RouterLink to="/report" class="report-callout__cta">Submit request</RouterLink>
-      <img class="report-callout__cta-icon" :src="reportDocument" alt="" aria-hidden="true" />
-    </div>
+    <PhilaButton variant="primary" size="large" to="/report" class="report-callout__cta">
+      Submit request
+    </PhilaButton>
   </section>
 </template>
 
@@ -43,35 +43,8 @@ import reportDocument from '@/assets/report-document.svg'
   font-weight: 700;
   text-decoration: underline;
 }
-.report-callout__cta-row {
-  position: relative;
-  display: flex;
-}
 .report-callout__cta {
-  flex: 1;
-  display: inline-flex;
-  align-items: center;
-  justify-content: center;
-  min-height: 48px;
-  padding: 0 var(--spacing-l, 1.5rem);
-  background: var(--Schemes-Primary, #1034f4);
-  color: #fff;
-  border-radius: 9999px;
-  font-weight: 600;
-  font-size: 1.125rem;
-  line-height: 1.75rem;
-  text-decoration: none;
-  box-shadow:
-    0 1px 3px 1px rgba(0, 0, 0, 0.15),
-    0 1px 2px 0 rgba(0, 0, 0, 0.3);
-}
-.report-callout__cta-icon {
-  position: absolute;
-  left: -4px;
-  top: 50%;
-  transform: translateY(-50%);
-  width: 60px;
-  height: 60px;
-  pointer-events: none;
+  width: 100%;
+  max-width: none;
 }
 </style>

apps/philly-311/frontend/src/components/ReportDocumentIcon.ts

@@ -0,0 +1,11 @@
+import { h } from 'vue'
+import type { IconComponent } from '@phila/phila-ui-core'
+import rawSvg from '@/assets/report-document.svg?raw'
+
+const sourceSvg = new DOMParser().parseFromString(rawSvg, 'image/svg+xml').documentElement
+const sourceAttrs = Object.fromEntries(Array.from(sourceSvg.attributes, (a) => [a.name, a.value]))
+const innerMarkup = sourceSvg.innerHTML
+
+const ReportDocumentIcon: IconComponent = () => h('svg', { ...sourceAttrs, innerHTML: innerMarkup })
+
+export default ReportDocumentIcon

apps/philly-311/frontend/src/components/__tests__/ReportCallout.test.ts

@@ -1,18 +1,27 @@
 import { describe, it, expect } from 'vitest'
-import { mount, RouterLinkStub } from '@vue/test-utils'
+import { mount } from '@vue/test-utils'
 import ReportCallout from '../ReportCallout.vue'
 
+const RouterLinkStub = {
+  template: '<a :href="String(to)"><slot /></a>',
+  props: ['to'],
+}
+
+function mountCallout() {
+  return mount(ReportCallout, { global: { stubs: { RouterLink: RouterLinkStub } } })
+}
+
 describe('ReportCallout', () => {
   it('renders the heading and a CTA linking to /report', () => {
-    const w = mount(ReportCallout, { global: { stubs: { RouterLink: RouterLinkStub } } })
+    const w = mountCallout()
     expect(w.text()).toContain('Submit a report to 311')
-    const cta = w.findComponent(RouterLinkStub)
-    expect(cta.props('to')).toBe('/report')
+    const cta = w.find('a[href="/report"]')
+    expect(cta.exists()).toBe(true)
     expect(cta.text()).toContain('Submit request')
   })
 
   it('renders the intro copy with an About Philly311 link opening in a new tab', () => {
-    const w = mount(ReportCallout, { global: { stubs: { RouterLink: RouterLinkStub } } })
+    const w = mountCallout()
     expect(w.text()).toContain(
       "Let us know if there's a non-emergency issue that needs attention. The Philly311 team will direct your report to the right department.",
     )
@@ -23,12 +32,4 @@ describe('ReportCallout', () => {
     expect(link.attributes('target')).toBe('_blank')
     expect(link.attributes('rel')).toBe('noopener')
   })
-
-  it('renders the decorative document illustration hidden from assistive tech', () => {
-    const w = mount(ReportCallout, { global: { stubs: { RouterLink: RouterLinkStub } } })
-    const icon = w.find('img.report-callout__cta-icon')
-    expect(icon.exists()).toBe(true)
-    expect(icon.attributes('alt')).toBe('')
-    expect(icon.attributes('aria-hidden')).toBe('true')
-  })
 })

apps/philly-311/frontend/vitest.config.ts

@@ -7,20 +7,28 @@ import vue from '@vitejs/plugin-vue'
 export default defineConfig({
   plugins: [vue()],
   resolve: {
-    alias: { '@': fileURLToPath(new URL('./src', import.meta.url)) },
+    alias: {
+      '@': fileURLToPath(new URL('./src', import.meta.url)),
+      // Mirrors vite.config.ts: reportSubmission.ts imports @pinboard/core directly,
+      // and Vitest doesn't share the app's build config.
+      '@pinboard/core': fileURLToPath(
+        new URL('../../../packages/core/src/index.ts', import.meta.url),
+      ),
+    },
     conditions: ['import', 'module', 'browser', 'default'],
   },
   test: {
     environment: 'jsdom',
     globals: true,
     setupFiles: ['./src/__tests__/setup.ts'],
-    // @phila/phila-ui-core's main entry has a side-effect `import './index.css'`.
-    // Left externalized, Vitest loads it via Node's own resolver, which can't
-    // handle a bare .css import; inlining routes it through Vite's transform
-    // instead, which strips/no-ops CSS imports like it does for the real app build.
+    // @phila/phila-ui-core's and @phila/phila-ui-breadcrumbs' main entries have a
+    // side-effect `import './index.css'`. Left externalized, Vitest loads them via
+    // Node's own resolver, which can't handle a bare .css import; inlining routes
+    // it through Vite's transform instead, which strips/no-ops CSS imports like it
+    // does for the real app build.
     server: {
       deps: {
-        inline: ['@phila/phila-ui-core'],
+        inline: ['@phila/phila-ui-core', '@phila/phila-ui-breadcrumbs'],
       },
     },
   },