From 618efb0b269c4f0268d5513dac480fc7f700949c Mon Sep 17 00:00:00 2001 From: Tim Hostetler <6970899+thostetler@users.noreply.github.com> Date: Tue, 10 Feb 2026 12:10:37 -0500 Subject: [PATCH 1/2] SCIX-826 fix(nav): render feedback menu items as links for new-tab support --- src/components/NavBar/FeedbackDropdown.tsx | 69 +++++++--- .../__tests__/FeedbackDropdown.test.tsx | 128 ++++++++++++++++++ 2 files changed, 181 insertions(+), 16 deletions(-) create mode 100644 src/components/NavBar/__tests__/FeedbackDropdown.test.tsx diff --git a/src/components/NavBar/FeedbackDropdown.tsx b/src/components/NavBar/FeedbackDropdown.tsx index bcbc17c10..70cb47976 100644 --- a/src/components/NavBar/FeedbackDropdown.tsx +++ b/src/components/NavBar/FeedbackDropdown.tsx @@ -1,8 +1,8 @@ +import { ChevronDownIcon } from '@chakra-ui/icons'; +import { HStack, List, ListItem, Menu, MenuButton, MenuItem, MenuList } from '@chakra-ui/react'; import { useRouter } from 'next/router'; -import { MouseEvent, ReactElement } from 'react'; -import { MenuDropdown } from './MenuDropdown'; +import { Fragment, MouseEvent, ReactElement } from 'react'; import { ListType } from './types'; -import { isBrowser } from '@/utils/common/guards'; export const feedbackItems = { record: { @@ -32,26 +32,63 @@ interface IFeedbackDropdownProps { onFinished?: () => void; } +/** + * Build the href for a feedback item, including the encoded + * `from` query parameter (with any existing `from` stripped). + */ +const buildHref = (path: string, asPath: string): string => { + const cleaned = asPath.replace(/from=[^&]+(&|$)/, ''); + return `${path}?from=${encodeURIComponent(cleaned)}`; +}; + export const FeedbackDropdown = (props: IFeedbackDropdownProps): ReactElement => { const { type, onFinished } = props; - const items = Object.values(feedbackItems); - const router = useRouter(); - const handleSelect = (e: MouseEvent) => { - const id = (e.target as HTMLElement).dataset['id']; - if (isBrowser()) { - void router.push({ - pathname: items.find((item) => id === item.id).path, - query: { from: router.asPath.replace(/from=[^&]+(&|$)/, '') }, // remove existing from from query - }); + const handleAccordionClick = (e: MouseEvent) => { + if (typeof onFinished === 'function') { + // Allow the default navigation but also close the drawer. + // Use setTimeout so the drawer closes after the click propagates. + setTimeout(() => onFinished(), 0); + } - if (typeof onFinished === 'function') { - onFinished(); - } + // If Cmd/Ctrl+Click, let the browser handle natively (new tab) + if (e.metaKey || e.ctrlKey) { + return; } }; - return ; + return type === ListType.DROPDOWN ? ( + + + + <>Feedback + + + + {items.map((item) => ( + + {item.label} + + ))} + + + ) : ( + + {items.map((item) => ( + + + + {item.label} + + + + ))} + + ); }; diff --git a/src/components/NavBar/__tests__/FeedbackDropdown.test.tsx b/src/components/NavBar/__tests__/FeedbackDropdown.test.tsx new file mode 100644 index 000000000..7b3516e25 --- /dev/null +++ b/src/components/NavBar/__tests__/FeedbackDropdown.test.tsx @@ -0,0 +1,128 @@ +import { describe, expect, it, vi } from 'vitest'; +import { FeedbackDropdown, feedbackItems } from '../FeedbackDropdown'; +import { ListType } from '../types'; +import { render, screen } from '@/test-utils'; +import { NextRouter } from 'next/router'; + +const createMockRouter = (initial: Partial = {}): NextRouter => { + const router: Partial = { + basePath: '', + pathname: '/search', + route: '/search', + asPath: '/search?q=star', + query: { q: 'star' }, + isReady: true, + isLocaleDomain: false, + isPreview: false, + isFallback: false, + push: vi.fn().mockResolvedValue(true), + replace: vi.fn().mockResolvedValue(true), + reload: vi.fn(), + back: vi.fn(), + prefetch: vi.fn().mockResolvedValue(undefined), + beforePopState: vi.fn(), + events: { + on: vi.fn(), + off: vi.fn(), + emit: vi.fn(), + }, + ...initial, + }; + return router as NextRouter; +}; + +let mockRouter: NextRouter; + +vi.mock('next/router', () => ({ + useRouter: () => mockRouter, +})); + +const items = Object.values(feedbackItems); + +describe('FeedbackDropdown', () => { + describe('dropdown variant', () => { + it('renders menu items as elements with href', async () => { + mockRouter = createMockRouter(); + + const { user } = render(); + + const menuButton = screen.getByRole('button', { name: /feedback/i }); + await user.click(menuButton); + + const menuItems = screen.getAllByRole('menuitem'); + expect(menuItems).toHaveLength(items.length); + + for (const menuItem of menuItems) { + expect(menuItem.tagName).toBe('A'); + expect(menuItem).toHaveAttribute('href'); + } + }); + + it('includes correct path and from query param in href', async () => { + mockRouter = createMockRouter({ asPath: '/search?q=star' }); + + const { user } = render(); + + const menuButton = screen.getByRole('button', { name: /feedback/i }); + await user.click(menuButton); + + const menuItems = screen.getAllByRole('menuitem'); + + for (const item of items) { + const link = menuItems.find((el) => el.getAttribute('href')?.startsWith(item.path)); + expect(link).toBeDefined(); + const href = link.getAttribute('href'); + expect(href).toContain(`from=${encodeURIComponent('/search?q=star')}`); + } + }); + + it('strips existing from param before encoding', async () => { + mockRouter = createMockRouter({ + asPath: '/search?q=star&from=/abs/1234', + }); + + const { user } = render(); + + const menuButton = screen.getByRole('button', { name: /feedback/i }); + await user.click(menuButton); + + const menuItems = screen.getAllByRole('menuitem'); + const href = menuItems[0].getAttribute('href'); + + // The "from" value should not contain the original from param + expect(href).not.toContain('from=%2Fabs%2F1234'); + // It should contain the cleaned path + expect(href).toContain(`from=${encodeURIComponent('/search?q=star&')}`); + }); + }); + + describe('accordion variant', () => { + it('renders list items with elements containing href', () => { + mockRouter = createMockRouter(); + + render(); + + const links = screen.getAllByRole('menuitem'); + expect(links).toHaveLength(items.length); + + for (const link of links) { + const anchor = link.querySelector('a'); + expect(anchor).not.toBeNull(); + expect(anchor).toHaveAttribute('href'); + } + }); + + it('calls onFinished when a link is clicked', async () => { + mockRouter = createMockRouter(); + const onFinished = vi.fn(); + + const { user } = render(); + + const links = screen.getAllByRole('menuitem'); + const anchor = links[0].querySelector('a'); + await user.click(anchor); + + expect(onFinished).toHaveBeenCalledTimes(1); + }); + }); +}); From a4081f845585811e3e3f8b5214e9babf758ee52d Mon Sep 17 00:00:00 2001 From: Tim Hostetler <6970899+thostetler@users.noreply.github.com> Date: Tue, 10 Feb 2026 12:17:35 -0500 Subject: [PATCH 2/2] fix(nav): clean up trailing chars in from param and remove vestigial Fragment SCIX-826 --- src/components/NavBar/FeedbackDropdown.tsx | 24 +++++++++---------- .../__tests__/FeedbackDropdown.test.tsx | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/components/NavBar/FeedbackDropdown.tsx b/src/components/NavBar/FeedbackDropdown.tsx index 70cb47976..7e62b2aff 100644 --- a/src/components/NavBar/FeedbackDropdown.tsx +++ b/src/components/NavBar/FeedbackDropdown.tsx @@ -1,7 +1,7 @@ import { ChevronDownIcon } from '@chakra-ui/icons'; import { HStack, List, ListItem, Menu, MenuButton, MenuItem, MenuList } from '@chakra-ui/react'; import { useRouter } from 'next/router'; -import { Fragment, MouseEvent, ReactElement } from 'react'; +import { MouseEvent, ReactElement } from 'react'; import { ListType } from './types'; export const feedbackItems = { @@ -37,7 +37,7 @@ interface IFeedbackDropdownProps { * `from` query parameter (with any existing `from` stripped). */ const buildHref = (path: string, asPath: string): string => { - const cleaned = asPath.replace(/from=[^&]+(&|$)/, ''); + const cleaned = asPath.replace(/from=[^&]+(&|$)/, '').replace(/[?&]$/, ''); return `${path}?from=${encodeURIComponent(cleaned)}`; }; @@ -77,17 +77,15 @@ export const FeedbackDropdown = (props: IFeedbackDropdownProps): ReactElement => ) : ( {items.map((item) => ( - - - - {item.label} - - - + + + {item.label} + + ))} ); diff --git a/src/components/NavBar/__tests__/FeedbackDropdown.test.tsx b/src/components/NavBar/__tests__/FeedbackDropdown.test.tsx index 7b3516e25..ff2646fcf 100644 --- a/src/components/NavBar/__tests__/FeedbackDropdown.test.tsx +++ b/src/components/NavBar/__tests__/FeedbackDropdown.test.tsx @@ -91,8 +91,8 @@ describe('FeedbackDropdown', () => { // The "from" value should not contain the original from param expect(href).not.toContain('from=%2Fabs%2F1234'); - // It should contain the cleaned path - expect(href).toContain(`from=${encodeURIComponent('/search?q=star&')}`); + // It should contain the cleaned path (trailing & stripped) + expect(href).toContain(`from=${encodeURIComponent('/search?q=star')}`); }); });