gsadmin/pulse
1
0
Fork 0
mirror of https://github.com/rcourtman/Pulse.git synced 2026-09-10 02:25:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(frontend): use links for mobile navigation

Browse Source
This commit is contained in:
pulse-triage[bot]
2026-08-29 20:47:23 +01:00
parent 8c65d716f4
commit 5cfa0f26b6
7 changed files with 169 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend-modern/src/AppLayout.tsx
+1
View File
@@ -995,6 +995,7 @@ export function AppLayout(props: AppLayoutProps) {
activeTab={getActiveTabMobile}
primaryTabs={primaryTabs}
utilityTabs={utilityTabs}
getPrimaryHref={getPrimaryTargetRoute}
onPrimaryClick={handlePrimaryClick}
onUtilityClick={handleUtilityClick}
/>
frontend-modern/src/__tests__/AppLayout.test.tsx
+14 -4
View File
@@ -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');
frontend-modern/src/components/shared/MobileNavBar.tsx
+69 -30
View File
@@ -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 (
<button
type="button"
<a
href={destinationHref(destination)}
role="menuitem"
tabIndex={-1}
data-tab-id={destination.tab.id}
@@ -168,7 +187,7 @@ export function MobileNavBar(props: MobileNavBarProps) {
? getMobileNavTabAriaLabel(destination.tab)
: undefined
}
onClick={() => mobileNav.handleDestinationClick(destination)}
onClick={(event) => handleDestinationLinkClick(event, destination)}
title={destination.tab.tooltip}
class={`flex min-h-11 w-full items-center gap-3 rounded-md px-3 py-2 text-left text-sm transition-colors ${
active()
@@ -177,7 +196,7 @@ export function MobileNavBar(props: MobileNavBarProps) {
} ${enabled() === false ? 'opacity-70' : ''}`.trim()}
>
<MobileNavDestinationContent destination={destination} iconClass={tabIconClass} />
</button>
</a>
);
}}
</For>
@@ -229,35 +248,55 @@ export function MobileNavBar(props: MobileNavBarProps) {
const canSwitch = () => mobileNav.platformDestinations().length > 1;
const currentLabel = () => destination().tab.label;
const content = () => (
<MobileNavDestinationContent destination={destination()} iconClass={tabIconClass} />
);
const className = () =>
getMobileNavTabButtonClass({
active: mobileNav.platformIsActive(),
enabled: destination().tab.enabled,
});
return (
<button
ref={mobileNav.setPlatformTriggerRef}
type="button"
data-tab-id="platform-switcher"
data-mobile-nav-destination="platform-switcher"
aria-label={
canSwitch() ? `Switch platform, current ${currentLabel()}` : currentLabel()
<Show
when={canSwitch()}
fallback={
<a
href={destinationHref(destination())}
data-tab-id="platform-switcher"
data-mobile-nav-destination="platform-switcher"
aria-label={currentLabel()}
aria-current={mobileNav.platformIsActive() ? 'page' : undefined}
onClick={(event) => handleDestinationLinkClick(event, destination())}
title={destination().tab.tooltip}
class={className()}
>
{content()}
</a>
}
aria-haspopup={canSwitch() ? 'menu' : undefined}
aria-expanded={canSwitch() ? mobileNav.isPlatformMenuOpen() : undefined}
aria-controls={canSwitch() ? MOBILE_NAV_PLATFORM_SWITCHER_ID : undefined}
aria-current={mobileNav.platformIsActive() ? 'page' : undefined}
onClick={mobileNav.handlePlatformTriggerClick}
onKeyDown={mobileNav.handlePlatformTriggerKeyDown}
title={canSwitch() ? 'Switch platform' : destination().tab.tooltip}
class={getMobileNavTabButtonClass({
active: mobileNav.platformIsActive(),
enabled: destination().tab.enabled,
})}
>
<MobileNavDestinationContent destination={destination()} iconClass={tabIconClass} />
<Show when={canSwitch()}>
<button
ref={mobileNav.setPlatformTriggerRef}
type="button"
data-tab-id="platform-switcher"
data-mobile-nav-destination="platform-switcher"
aria-label={`Switch platform, current ${currentLabel()}`}
aria-haspopup="menu"
aria-expanded={mobileNav.isPlatformMenuOpen()}
aria-controls={MOBILE_NAV_PLATFORM_SWITCHER_ID}
aria-current={mobileNav.platformIsActive() ? 'page' : undefined}
onClick={mobileNav.handlePlatformTriggerClick}
onKeyDown={mobileNav.handlePlatformTriggerKeyDown}
title="Switch platform"
class={className()}
>
{content()}
<ChevronsUpDownIcon
aria-hidden="true"
class="absolute right-1 top-1 h-2.5 w-2.5 text-muted"
/>
</Show>
</button>
</button>
</Show>
);
}}
</Show>
@@ -269,8 +308,8 @@ export function MobileNavBar(props: MobileNavBarProps) {
destination.kind === 'primary' ? destination.tab.enabled : undefined;
return (
<button
type="button"
<a
href={destinationHref(destination)}
data-tab-id={destination.tab.id}
data-mobile-nav-destination={getMobileNavDestinationKey(destination)}
aria-current={active() ? 'page' : undefined}
@@ -279,12 +318,12 @@ export function MobileNavBar(props: MobileNavBarProps) {
? getMobileNavTabAriaLabel(destination.tab)
: undefined
}
onClick={() => mobileNav.handleDestinationClick(destination)}
onClick={(event) => handleDestinationLinkClick(event, destination)}
title={destination.tab.tooltip}
class={getMobileNavTabButtonClass({ active: active(), enabled: enabled() })}
>
<MobileNavDestinationContent destination={destination} iconClass={tabIconClass} />
</button>
</a>
);
}}
</For>
frontend-modern/src/components/shared/__tests__/MobileNavBar.test.tsx
+39 -22
View File
@@ -61,6 +61,7 @@ describe('MobileNavBar', () => {
expect(mobileNavBarSource).toContain('useMobileNavBarState');
expect(mobileNavBarSource).toContain('getMobileNavTabButtonClass');
expect(mobileNavBarSource).toContain('role="menu"');
expect(mobileNavBarSource).toContain('<a');
expect(mobileNavBarSource).toContain('pb-safe xl:hidden');
expect(mobileNavBarSource).not.toContain('createSignal');
expect(mobileNavBarSource).not.toContain('requestAnimationFrame');
@@ -110,8 +111,8 @@ describe('MobileNavBar', () => {
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<HTMLButtonElement>('[data-mobile-nav-destination]'),
const destinationsBefore = [
...container.querySelectorAll<HTMLElement>('[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<HTMLButtonElement>('[data-mobile-nav-destination]'),
const destinationsAfter = [
...container.querySelectorAll<HTMLElement>('[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);
});
});
});
frontend-modern/src/components/shared/__tests__/mobileNavBarModel.branchcov0712.test.ts
+34
View File
@@ -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';
frontend-modern/src/components/shared/mobileNavBarModel.ts
+10
View File
@@ -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,
frontend-modern/src/components/shared/useMobileNavBarState.ts
+2 -2
View File
@@ -57,7 +57,7 @@ export function useMobileNavBarState(props: MobileNavBarProps) {
const activeTriggerRef = () =>
openMenu() === 'platform' ? platformTriggerRef() : overflowTriggerRef();
const activeMenuItems = () =>
Array.from(activeMenuRef()?.querySelectorAll<HTMLButtonElement>('[role="menuitem"]') ?? []);
Array.from(activeMenuRef()?.querySelectorAll<HTMLElement>('[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;