diff --git a/frontend-modern/src/AppLayout.tsx b/frontend-modern/src/AppLayout.tsx index a51723a3f..2fae9e46d 100644 --- a/frontend-modern/src/AppLayout.tsx +++ b/frontend-modern/src/AppLayout.tsx @@ -995,6 +995,7 @@ export function AppLayout(props: AppLayoutProps) { activeTab={getActiveTabMobile} primaryTabs={primaryTabs} utilityTabs={utilityTabs} + getPrimaryHref={getPrimaryTargetRoute} onPrimaryClick={handlePrimaryClick} onUtilityClick={handleUtilityClick} /> diff --git a/frontend-modern/src/__tests__/AppLayout.test.tsx b/frontend-modern/src/__tests__/AppLayout.test.tsx index e2129cd8f..821a963de 100644 --- a/frontend-modern/src/__tests__/AppLayout.test.tsx +++ b/frontend-modern/src/__tests__/AppLayout.test.tsx @@ -205,7 +205,7 @@ describe('AppLayout navigation icons', () => { expect(button).toBeTruthy(); expect(button?.querySelector('svg')).toBeTruthy(); }); - const mobilePatrolTab = within(mobileNav).getByRole('button', { + const mobilePatrolTab = within(mobileNav).getByRole('link', { name: 'Patrol', }); expect(mobilePatrolTab.querySelector('svg')).toBeTruthy(); @@ -234,7 +234,7 @@ describe('AppLayout navigation icons', () => { expect(within(systemGroup as HTMLElement).queryByText('Needs Attention')).toBeNull(); const mobileNav = screen.getByRole('navigation', { name: 'Mobile navigation' }); - const mobilePatrolTab = within(mobileNav).getByRole('button', { + const mobilePatrolTab = within(mobileNav).getByRole('link', { name: 'Patrol: 2 active attention items', }); expect(mobilePatrolTab).toHaveTextContent('Patrol'); @@ -259,7 +259,7 @@ describe('AppLayout navigation icons', () => { const mobileNav = screen.getByRole('navigation', { name: 'Mobile navigation' }); expect( - within(mobileNav).getByRole('button', { name: 'Actions: 3 actions await approval' }), + within(mobileNav).getByRole('link', { name: 'Actions: 3 actions await approval' }), ).toHaveAttribute('aria-current', 'page'); expect(document.title).toContain('Actions'); }); @@ -270,7 +270,7 @@ describe('AppLayout navigation icons', () => { renderLayout([], '/alerts'); const mobileNav = screen.getByRole('navigation', { name: 'Mobile navigation' }); - fireEvent.click(within(mobileNav).getByRole('button', { name: 'Actions' })); + fireEvent.click(within(mobileNav).getByRole('link', { name: 'Actions' })); await waitFor(() => expect(window.location.pathname).toBe('/actions')); }); @@ -346,6 +346,16 @@ describe('AppLayout navigation icons', () => { it('restores the previous Proxmox route state when returning from another platform tab', async () => { renderLayout(platformResources(), '/proxmox/overview?status=running'); + const mobileNav = screen.getByRole('navigation', { name: 'Mobile navigation' }); + fireEvent.click( + within(mobileNav).getByRole('button', { name: 'Switch platform, current Proxmox' }), + ); + const platformMenu = await screen.findByRole('menu', { name: 'Switch platform' }); + const rememberedMobileLink = within(platformMenu).getByRole('menuitem', { name: 'Proxmox' }); + expect(rememberedMobileLink.tagName).toBe('A'); + expect(rememberedMobileLink).toHaveAttribute('href', '/proxmox/overview?status=running'); + fireEvent.keyDown(rememberedMobileLink, { key: 'Escape' }); + await fireEvent.click(getInfrastructureLink('Docker')); await waitFor(() => { expect(window.location.pathname).toBe('/docker/overview'); diff --git a/frontend-modern/src/components/shared/MobileNavBar.tsx b/frontend-modern/src/components/shared/MobileNavBar.tsx index 976d7e1c4..923afc8a2 100644 --- a/frontend-modern/src/components/shared/MobileNavBar.tsx +++ b/frontend-modern/src/components/shared/MobileNavBar.tsx @@ -5,6 +5,7 @@ import { type MobileNavBarProps, useMobileNavBarState } from './useMobileNavBarS import { getMobileNavAlertBadgeCounts, getMobileNavDestinationKey, + getMobileNavDestinationHref, getMobileNavTabAriaLabel, getMobileNavTabButtonClass, isMobileNavDestinationActive, @@ -139,6 +140,24 @@ export function MobileNavBar(props: MobileNavBarProps) { }); const overflowHasBadge = () => mobileNav.overflowDestinations().some(mobileNavDestinationHasBadge); + const destinationHref = (destination: MobileNavBarDestination) => + getMobileNavDestinationHref(destination, props.getPrimaryHref); + const handleDestinationLinkClick = (event: MouseEvent, destination: MobileNavBarDestination) => { + // Preserve native link behavior for new tabs/windows and context-menu + // actions. Plain activation stays in the SPA and retains route memory. + if ( + event.defaultPrevented || + event.button !== 0 || + event.metaKey || + event.ctrlKey || + event.shiftKey || + event.altKey + ) { + return; + } + event.preventDefault(); + mobileNav.handleDestinationClick(destination); + }; const renderOverflowGroup = (options: { label: string; @@ -156,8 +175,8 @@ export function MobileNavBar(props: MobileNavBarProps) { destination.kind === 'primary' ? destination.tab.enabled : undefined; return ( - + ); }} @@ -229,35 +248,55 @@ export function MobileNavBar(props: MobileNavBarProps) { const canSwitch = () => mobileNav.platformDestinations().length > 1; const currentLabel = () => destination().tab.label; + const content = () => ( + + ); + const className = () => + getMobileNavTabButtonClass({ + active: mobileNav.platformIsActive(), + enabled: destination().tab.enabled, + }); + return ( - + + ); }} @@ -269,8 +308,8 @@ export function MobileNavBar(props: MobileNavBarProps) { destination.kind === 'primary' ? destination.tab.enabled : undefined; return ( - + ); }} diff --git a/frontend-modern/src/components/shared/__tests__/MobileNavBar.test.tsx b/frontend-modern/src/components/shared/__tests__/MobileNavBar.test.tsx index e34057e20..aa2b93e20 100644 --- a/frontend-modern/src/components/shared/__tests__/MobileNavBar.test.tsx +++ b/frontend-modern/src/components/shared/__tests__/MobileNavBar.test.tsx @@ -61,6 +61,7 @@ describe('MobileNavBar', () => { expect(mobileNavBarSource).toContain('useMobileNavBarState'); expect(mobileNavBarSource).toContain('getMobileNavTabButtonClass'); expect(mobileNavBarSource).toContain('role="menu"'); + expect(mobileNavBarSource).toContain(' { expect(fixedRail).toHaveClass('px-0.5', 'py-0.5'); expect(fixedRail).not.toHaveClass('py-1'); expect( - Array.from(fixedRail?.querySelectorAll('button[data-tab-id]') ?? []).map((button) => - button.getAttribute('data-tab-id'), + Array.from(fixedRail?.querySelectorAll('[data-tab-id]') ?? []).map((item) => + item.getAttribute('data-tab-id'), ), ).toEqual(['platform-switcher', 'alerts', 'ai', 'actions', 'more']); @@ -121,6 +122,9 @@ describe('MobileNavBar', () => { expect(platformSwitcher).toHaveAttribute('aria-haspopup', 'menu'); expect(platformSwitcher).toHaveAttribute('aria-expanded', 'false'); expect(platformSwitcher).toHaveAttribute('aria-current', 'page'); + expect(screen.getByRole('link', { name: 'Alerts' })).toHaveAttribute('href', '/alerts'); + expect(screen.getByRole('link', { name: 'Patrol' })).toHaveAttribute('href', '/patrol'); + expect(screen.getByRole('link', { name: 'Actions' })).toHaveAttribute('href', '/actions'); fireEvent.click(platformSwitcher); const platformMenu = await screen.findByRole('menu', { name: 'Switch platform' }); @@ -134,6 +138,11 @@ describe('MobileNavBar', () => { 'aria-current', 'page', ); + expect(within(platformMenu).getByRole('menuitem', { name: 'Machines' }).tagName).toBe('A'); + expect(within(platformMenu).getByRole('menuitem', { name: 'Machines' })).toHaveAttribute( + 'href', + '/standalone/overview', + ); const more = screen.getByRole('button', { name: 'More navigation' }); expect(more).toHaveAttribute('aria-haspopup', 'menu'); @@ -149,6 +158,10 @@ describe('MobileNavBar', () => { .getAllByRole('menuitem') .map((item) => item.getAttribute('data-tab-id')), ).toEqual(['settings']); + expect(within(menu).getByRole('menuitem', { name: 'Settings' })).toHaveAttribute( + 'href', + '/settings', + ); expect(platformSwitcher).toHaveClass('min-h-10', 'gap-0', 'py-0.5', 'text-[9px]'); }); @@ -169,7 +182,7 @@ describe('MobileNavBar', () => { /> )); - fireEvent.click(screen.getByRole('button', { name: 'Alerts' })); + fireEvent.click(screen.getByRole('link', { name: 'Alerts' })); expect(onUtilityClick).toHaveBeenCalledWith( expect.objectContaining({ id: 'alerts', route: '/alerts' }), ); @@ -216,12 +229,12 @@ describe('MobileNavBar', () => { const nav = screen.getByRole('navigation', { name: 'Mobile navigation' }); expect( - within(nav).getByRole('button', { name: 'Alerts: 2 critical, 3 warning' }), + within(nav).getByRole('link', { name: 'Alerts: 2 critical, 3 warning' }), ).toHaveTextContent('23'); - expect( - within(nav).getByRole('button', { name: 'Patrol: 2 open work items' }), - ).toHaveTextContent('Patrol2'); - expect(within(nav).queryByRole('button', { name: 'Pulse Patrol Patrol' })).toBeNull(); + expect(within(nav).getByRole('link', { name: 'Patrol: 2 open work items' })).toHaveTextContent( + 'Patrol2', + ); + expect(within(nav).queryByRole('link', { name: 'Pulse Patrol Patrol' })).toBeNull(); fireEvent.click(within(nav).getByRole('button', { name: 'Switch platform, current Proxmox' })); const menu = await screen.findByRole('menu', { name: 'Switch platform' }); @@ -287,12 +300,16 @@ describe('MobileNavBar', () => { )); expect(screen.queryByRole('button', { name: 'More navigation' })).toBeNull(); - screen.getAllByRole('button').forEach((button) => { - expect(button).not.toHaveAttribute('aria-current'); + expect(screen.getByRole('link', { name: 'Proxmox' })).toHaveAttribute( + 'href', + '/proxmox/overview', + ); + screen.getAllByRole('link').forEach((link) => { + expect(link).not.toHaveAttribute('aria-current'); }); }); - it('keeps destination button DOM identity when tab arrays are rebuilt with equal content', async () => { + it('keeps destination DOM identity when tab arrays are rebuilt with equal content', async () => { const buildTabs = () => [ makeUtility('alerts', 'Alerts', { count: 3, breakdown: { warning: 2, critical: 1 } }), makeUtility('ai', 'Patrol'), @@ -310,24 +327,24 @@ describe('MobileNavBar', () => { /> )); - const buttonsBefore = [ - ...container.querySelectorAll('[data-mobile-nav-destination]'), + const destinationsBefore = [ + ...container.querySelectorAll('[data-mobile-nav-destination]'), ]; - expect(buttonsBefore.length).toBeGreaterThan(0); + expect(destinationsBefore.length).toBeGreaterThan(0); // A state frame carrying an unchanged alerts array rebuilds the tab - // objects; identical content must not recreate the rendered buttons. + // objects; identical content must not recreate the rendered destinations. setRawTabs(buildTabs()); await waitFor(() => { - const buttonsAfter = [ - ...container.querySelectorAll('[data-mobile-nav-destination]'), + const destinationsAfter = [ + ...container.querySelectorAll('[data-mobile-nav-destination]'), ]; - expect(buttonsAfter).toHaveLength(buttonsBefore.length); - buttonsAfter.forEach((button, index) => { - expect(button).toBe(buttonsBefore[index]); + expect(destinationsAfter).toHaveLength(destinationsBefore.length); + destinationsAfter.forEach((destination, index) => { + expect(destination).toBe(destinationsBefore[index]); }); - buttonsBefore.forEach((button) => { - expect(button.isConnected).toBe(true); + destinationsBefore.forEach((destination) => { + expect(destination.isConnected).toBe(true); }); }); }); diff --git a/frontend-modern/src/components/shared/__tests__/mobileNavBarModel.branchcov0712.test.ts b/frontend-modern/src/components/shared/__tests__/mobileNavBarModel.branchcov0712.test.ts index 4bab29c59..4b1132bd7 100644 --- a/frontend-modern/src/components/shared/__tests__/mobileNavBarModel.branchcov0712.test.ts +++ b/frontend-modern/src/components/shared/__tests__/mobileNavBarModel.branchcov0712.test.ts @@ -3,6 +3,7 @@ import { buildMobileNavBarLayout, buildOrderedMobileNavTabs, getMobileNavAlertBadgeCounts, + getMobileNavDestinationHref, getMobileNavTabAriaLabel, getMobileNavTabButtonClass, } from '@/components/shared/mobileNavBarModel'; @@ -265,6 +266,39 @@ describe('mobileNavBarModel.branchcov2', () => { }); }); + describe('getMobileNavDestinationHref', () => { + it('uses the utility route directly', () => { + expect( + getMobileNavDestinationHref({ kind: 'utility', tab: makeUtilityTab({ route: '/alerts' }) }), + ).toBe('/alerts'); + }); + + it('uses the canonical route for an enabled primary destination by default', () => { + expect(getMobileNavDestinationHref({ kind: 'primary', tab: makePrimaryTab('proxmox') })).toBe( + '/proxmox', + ); + }); + + it('sends an unconfigured primary destination to infrastructure settings by default', () => { + expect( + getMobileNavDestinationHref({ + kind: 'primary', + tab: makePrimaryTab('docker', { enabled: false }), + }), + ).toBe('/settings/infrastructure'); + }); + + it('prefers the shell route resolver so remembered state is exposed in the href', () => { + const tab = makePrimaryTab('proxmox'); + expect( + getMobileNavDestinationHref( + { kind: 'primary', tab }, + (primary) => `${primary.route}?status=running`, + ), + ).toBe('/proxmox?status=running'); + }); + }); + describe('getMobileNavTabButtonClass', () => { const BASE = 'relative flex min-h-10 min-w-0 flex-1 select-none flex-col items-center justify-center gap-0 rounded-md px-1 py-0.5 text-[9px] font-medium transition-colors'; diff --git a/frontend-modern/src/components/shared/mobileNavBarModel.ts b/frontend-modern/src/components/shared/mobileNavBarModel.ts index ec00dcba0..1045a34a5 100644 --- a/frontend-modern/src/components/shared/mobileNavBarModel.ts +++ b/frontend-modern/src/components/shared/mobileNavBarModel.ts @@ -31,6 +31,7 @@ export type MobileNavBarProps = { activeTab: () => string | null; primaryTabs: () => MobileNavBarPrimaryTab[]; utilityTabs: () => MobileNavBarUtilityTab[]; + getPrimaryHref?: (tab: MobileNavBarPrimaryTab) => string; onPrimaryClick: (tab: MobileNavBarPrimaryTab) => void; onUtilityClick: (tab: MobileNavBarUtilityTab) => void; }; @@ -129,6 +130,15 @@ export function getMobileNavDestinationKey(destination: MobileNavBarDestination) return `${destination.kind}:${destination.tab.id}`; } +export function getMobileNavDestinationHref( + destination: MobileNavBarDestination, + getPrimaryHref?: (tab: MobileNavBarPrimaryTab) => string, +): string { + if (destination.kind === 'utility') return destination.tab.route; + if (getPrimaryHref) return getPrimaryHref(destination.tab); + return destination.tab.enabled ? destination.tab.route : destination.tab.settingsRoute; +} + export function isMobileNavDestinationActive( destination: MobileNavBarDestination, activeTab: string | null, diff --git a/frontend-modern/src/components/shared/useMobileNavBarState.ts b/frontend-modern/src/components/shared/useMobileNavBarState.ts index 0fe5929a3..769eb284f 100644 --- a/frontend-modern/src/components/shared/useMobileNavBarState.ts +++ b/frontend-modern/src/components/shared/useMobileNavBarState.ts @@ -57,7 +57,7 @@ export function useMobileNavBarState(props: MobileNavBarProps) { const activeTriggerRef = () => openMenu() === 'platform' ? platformTriggerRef() : overflowTriggerRef(); const activeMenuItems = () => - Array.from(activeMenuRef()?.querySelectorAll('[role="menuitem"]') ?? []); + Array.from(activeMenuRef()?.querySelectorAll('[role="menuitem"]') ?? []); const focusMenuItem = (target: 'active' | 'first' | 'last') => { queueMicrotask(() => { @@ -177,7 +177,7 @@ export function useMobileNavBarState(props: MobileNavBarProps) { const items = activeMenuItems(); if (items.length === 0) return; event.preventDefault(); - const currentIndex = items.indexOf(document.activeElement as HTMLButtonElement); + const currentIndex = items.indexOf(document.activeElement as HTMLElement); let nextIndex = 0; if (event.key === 'End') { nextIndex = items.length - 1;