mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(frontend): use links for desktop navigation
This commit is contained in:
pulse-triage[bot]
@@ -1,7 +1,7 @@
|
||||
import { For, Show, Suspense, createEffect, createMemo, createSignal, onCleanup } from 'solid-js';
|
||||
import { Portal } from 'solid-js/web';
|
||||
import type { JSX } from 'solid-js';
|
||||
import { useLocation, useNavigate } from '@solidjs/router';
|
||||
import { A, useLocation, useNavigate } from '@solidjs/router';
|
||||
import BellIcon from 'lucide-solid/icons/bell';
|
||||
import ListChecksIcon from 'lucide-solid/icons/list-checks';
|
||||
import SettingsIcon from 'lucide-solid/icons/settings';
|
||||
@@ -256,6 +256,7 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
|
||||
const [headerVisible, setHeaderVisible] = createSignal(true);
|
||||
const [skipLinkFocused, setSkipLinkFocused] = createSignal(false);
|
||||
const [primaryRouteMemoryVersion, setPrimaryRouteMemoryVersion] = createSignal(0);
|
||||
let headerEl: HTMLDivElement | undefined;
|
||||
let assistantLauncherEl: HTMLButtonElement | undefined;
|
||||
let restoreAssistantLauncherFocus = false;
|
||||
@@ -358,6 +359,7 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
if (!routeBelongsToPrimaryTab(route, activeTab)) return;
|
||||
if (primaryRouteMemory[activeTab] === route) return;
|
||||
primaryRouteMemory = { ...primaryRouteMemory, [activeTab]: route };
|
||||
setPrimaryRouteMemoryVersion((version) => version + 1);
|
||||
});
|
||||
|
||||
createEffect(() => {
|
||||
@@ -665,12 +667,17 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
};
|
||||
|
||||
const getPrimaryTargetRoute = (tab: PrimaryTab) => {
|
||||
// Route memory lives across AppLayout remounts. Track local writes with a
|
||||
// signal so link hrefs update as users move within a platform, including
|
||||
// for native middle-click and context-menu navigation.
|
||||
void primaryRouteMemoryVersion();
|
||||
return resolvePrimaryNavigationRoute(tab, primaryRouteMemory);
|
||||
};
|
||||
|
||||
const renderPrimaryNavigationTab = (tab: PrimaryTab) => {
|
||||
const isActive = () => getActiveTabDesktop() === tab.id;
|
||||
const disabled = () => !tab.enabled;
|
||||
const needsSetup = () => !tab.enabled;
|
||||
const targetRoute = () => getPrimaryTargetRoute(tab);
|
||||
const Icon = tab.icon;
|
||||
const baseClasses =
|
||||
'tab relative px-1.5 xl:px-2 2xl:px-3 py-1.5 text-xs xl:text-sm font-medium flex items-center gap-1 2xl:gap-1.5 rounded-t border border-transparent transition-colors whitespace-nowrap cursor-pointer select-none';
|
||||
@@ -679,30 +686,23 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
if (isActive()) {
|
||||
return `${baseClasses} bg-surface text-blue-600 dark:text-blue-400 border-border border-b border-b-surface shadow-sm font-semibold`;
|
||||
}
|
||||
if (disabled()) {
|
||||
return `${baseClasses} cursor-not-allowed text-muted opacity-70 bg-base`;
|
||||
if (needsSetup()) {
|
||||
return `${baseClasses} text-muted opacity-70 bg-base hover:bg-surface-hover`;
|
||||
}
|
||||
return `${baseClasses} text-muted hover:text-base-content hover:bg-surface-hover`;
|
||||
};
|
||||
|
||||
const title = () =>
|
||||
disabled() ? `${tab.label} is not configured yet. Click to open settings.` : tab.tooltip;
|
||||
needsSetup() ? `${tab.label} is not configured yet. Click to open settings.` : tab.tooltip;
|
||||
|
||||
return (
|
||||
<div
|
||||
<A
|
||||
href={targetRoute()}
|
||||
class={className()}
|
||||
role="tab"
|
||||
tabIndex={0}
|
||||
aria-label={tab.label}
|
||||
aria-disabled={disabled()}
|
||||
onMouseEnter={() => warmNavigationTarget(getPrimaryTargetRoute(tab))}
|
||||
onClick={() => handlePrimaryClick(tab)}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === 'Enter' || event.key === ' ') {
|
||||
event.preventDefault();
|
||||
handlePrimaryClick(tab);
|
||||
}
|
||||
}}
|
||||
aria-current={isActive() ? 'page' : undefined}
|
||||
onMouseEnter={() => warmNavigationTarget(targetRoute())}
|
||||
onFocus={() => warmNavigationTarget(targetRoute())}
|
||||
title={title()}
|
||||
>
|
||||
<span aria-hidden="true" class="inline-flex items-center justify-center">
|
||||
@@ -717,7 +717,7 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
</Show>
|
||||
</span>
|
||||
<span class="xs:hidden">{tab.label.charAt(0)}</span>
|
||||
</div>
|
||||
</A>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -889,9 +889,8 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
</div>
|
||||
|
||||
<Show when={!kioskMode()}>
|
||||
<div
|
||||
<nav
|
||||
class="tabs mb-2 hidden xl:flex items-end gap-2 overflow-x-auto overflow-y-hidden whitespace-nowrap border-b border-border scrollbar-hide"
|
||||
role="tablist"
|
||||
aria-label="Primary navigation"
|
||||
>
|
||||
<div class="flex items-end gap-1" role="group" aria-label="Infrastructure">
|
||||
@@ -914,20 +913,13 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
};
|
||||
|
||||
return (
|
||||
<div
|
||||
<A
|
||||
href={tab.route}
|
||||
class={className()}
|
||||
role="tab"
|
||||
tabIndex={0}
|
||||
aria-label={getDesktopUtilityTabAriaLabel(tab)}
|
||||
aria-disabled={false}
|
||||
aria-current={isActive() ? 'page' : undefined}
|
||||
onMouseEnter={() => warmNavigationTarget(tab.route)}
|
||||
onClick={() => handleUtilityClick(tab)}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === 'Enter' || event.key === ' ') {
|
||||
event.preventDefault();
|
||||
handleUtilityClick(tab);
|
||||
}
|
||||
}}
|
||||
onFocus={() => warmNavigationTarget(tab.route)}
|
||||
title={tab.tooltip}
|
||||
>
|
||||
<span aria-hidden="true" class="inline-flex items-center justify-center">
|
||||
@@ -978,13 +970,13 @@ export function AppLayout(props: AppLayoutProps) {
|
||||
Pro
|
||||
</span>
|
||||
</Show>
|
||||
</div>
|
||||
</A>
|
||||
);
|
||||
}}
|
||||
</For>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</nav>
|
||||
</Show>
|
||||
|
||||
<main
|
||||
|
||||
@@ -156,11 +156,11 @@ describe('AppLayout navigation icons', () => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
const getInfrastructureTab = (name: string) => {
|
||||
const desktopNav = screen.getByRole('tablist', { name: 'Primary navigation' });
|
||||
const getInfrastructureLink = (name: string) => {
|
||||
const desktopNav = screen.getByRole('navigation', { name: 'Primary navigation' });
|
||||
const infrastructureGroup = desktopNav.querySelector('[aria-label="Infrastructure"]');
|
||||
expect(infrastructureGroup).toBeTruthy();
|
||||
return within(infrastructureGroup as HTMLElement).getByRole('tab', { name });
|
||||
return within(infrastructureGroup as HTMLElement).getByRole('link', { name });
|
||||
};
|
||||
|
||||
const platformResources = () => [
|
||||
@@ -177,26 +177,27 @@ describe('AppLayout navigation icons', () => {
|
||||
expect(container.querySelector('main')).toHaveClass('mb-1', 'sm:mb-2');
|
||||
expect(container.querySelector('footer')).toHaveClass('pulse-footer', 'px-2', 'sm:px-4');
|
||||
|
||||
const desktopNav = screen.getByRole('tablist', { name: 'Primary navigation' });
|
||||
const desktopNav = screen.getByRole('navigation', { name: 'Primary navigation' });
|
||||
const systemGroup = desktopNav.querySelector('[aria-label="System"]');
|
||||
expect(systemGroup).toBeTruthy();
|
||||
|
||||
const desktopTabs = within(systemGroup as HTMLElement).getAllByRole('tab');
|
||||
const desktopTabs = within(systemGroup as HTMLElement).getAllByRole('link');
|
||||
expect(desktopTabs).toHaveLength(4);
|
||||
desktopTabs.forEach((tab) => {
|
||||
expect(tab.tagName).toBe('A');
|
||||
expect(tab.querySelector('svg')).toBeTruthy();
|
||||
});
|
||||
const desktopPatrolTab = within(systemGroup as HTMLElement).getByRole('tab', {
|
||||
const desktopPatrolTab = within(systemGroup as HTMLElement).getByRole('link', {
|
||||
name: 'Patrol',
|
||||
});
|
||||
expect(desktopPatrolTab.querySelector('svg')).toBeTruthy();
|
||||
expect(within(systemGroup as HTMLElement).getByRole('tab', { name: '1 Alerts' })).toBeTruthy();
|
||||
expect(within(systemGroup as HTMLElement).getByRole('link', { name: '1 Alerts' })).toBeTruthy();
|
||||
expect(
|
||||
within(systemGroup as HTMLElement).queryByRole('tab', {
|
||||
within(systemGroup as HTMLElement).queryByRole('link', {
|
||||
name: 'Pulse Patrol Patrol',
|
||||
}),
|
||||
).toBeNull();
|
||||
expect(within(systemGroup as HTMLElement).queryByRole('tab', { name: 'Patrol P' })).toBeNull();
|
||||
expect(within(systemGroup as HTMLElement).queryByRole('link', { name: 'Patrol P' })).toBeNull();
|
||||
|
||||
const mobileNav = screen.getByRole('navigation', { name: 'Mobile navigation' });
|
||||
['alerts', 'ai', 'actions'].forEach((tabId) => {
|
||||
@@ -221,11 +222,11 @@ describe('AppLayout navigation icons', () => {
|
||||
patrolAttentionMockState.activeCount = 2;
|
||||
renderLayout();
|
||||
|
||||
const desktopNav = screen.getByRole('tablist', { name: 'Primary navigation' });
|
||||
const desktopNav = screen.getByRole('navigation', { name: 'Primary navigation' });
|
||||
const systemGroup = desktopNav.querySelector('[aria-label="System"]');
|
||||
expect(systemGroup).toBeTruthy();
|
||||
|
||||
const desktopPatrolTab = within(systemGroup as HTMLElement).getByRole('tab', {
|
||||
const desktopPatrolTab = within(systemGroup as HTMLElement).getByRole('link', {
|
||||
name: 'Patrol: 2 active attention items',
|
||||
});
|
||||
expect(desktopPatrolTab).toHaveTextContent('Patrol');
|
||||
@@ -245,14 +246,16 @@ describe('AppLayout navigation icons', () => {
|
||||
actionInboxMockState.pendingActionCount = 3;
|
||||
renderLayout([], '/actions');
|
||||
|
||||
const desktopNav = screen.getByRole('tablist', { name: 'Primary navigation' });
|
||||
const desktopNav = screen.getByRole('navigation', { name: 'Primary navigation' });
|
||||
const systemGroup = desktopNav.querySelector('[aria-label="System"]');
|
||||
expect(systemGroup).toBeTruthy();
|
||||
const actionsTab = within(systemGroup as HTMLElement).getByRole('tab', {
|
||||
const actionsTab = within(systemGroup as HTMLElement).getByRole('link', {
|
||||
name: 'Actions: 3 actions await approval',
|
||||
});
|
||||
expect(actionsTab.className).toContain('text-blue-600');
|
||||
expect(within(systemGroup as HTMLElement).getByRole('tab', { name: 'Patrol' })).toBeTruthy();
|
||||
expect(actionsTab).toHaveAttribute('href', '/actions');
|
||||
expect(actionsTab).toHaveAttribute('aria-current', 'page');
|
||||
expect(within(systemGroup as HTMLElement).getByRole('link', { name: 'Patrol' })).toBeTruthy();
|
||||
|
||||
const mobileNav = screen.getByRole('navigation', { name: 'Mobile navigation' });
|
||||
expect(
|
||||
@@ -280,32 +283,32 @@ describe('AppLayout navigation icons', () => {
|
||||
makeResource({ id: 'vcenter-1', type: 'vm', platformType: 'vmware-vsphere' }),
|
||||
]);
|
||||
|
||||
const desktopNav = screen.getByRole('tablist', { name: 'Primary navigation' });
|
||||
const desktopNav = screen.getByRole('navigation', { name: 'Primary navigation' });
|
||||
const infrastructureGroup = desktopNav.querySelector('[aria-label="Infrastructure"]');
|
||||
expect(infrastructureGroup).toBeTruthy();
|
||||
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement)
|
||||
.getAllByRole('tab')
|
||||
.getAllByRole('link')
|
||||
.map((tab) => tab.getAttribute('aria-label')),
|
||||
).toEqual(['Proxmox', 'Docker', 'vSphere', 'Machines']);
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement).getByRole('tab', { name: 'Machines' }),
|
||||
within(infrastructureGroup as HTMLElement).getByRole('link', { name: 'Machines' }),
|
||||
).toBeTruthy();
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement).getByRole('tab', { name: 'Proxmox' }),
|
||||
within(infrastructureGroup as HTMLElement).getByRole('link', { name: 'Proxmox' }),
|
||||
).toHaveAttribute('href', '/proxmox/overview');
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement).getByRole('link', { name: 'Docker' }),
|
||||
).toBeTruthy();
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement).getByRole('tab', { name: 'Docker' }),
|
||||
).toBeTruthy();
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement).queryByRole('tab', { name: 'Kubernetes' }),
|
||||
within(infrastructureGroup as HTMLElement).queryByRole('link', { name: 'Kubernetes' }),
|
||||
).toBeNull();
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement).queryByRole('tab', { name: 'TrueNAS' }),
|
||||
within(infrastructureGroup as HTMLElement).queryByRole('link', { name: 'TrueNAS' }),
|
||||
).toBeNull();
|
||||
expect(
|
||||
within(infrastructureGroup as HTMLElement).getByRole('tab', { name: 'vSphere' }),
|
||||
within(infrastructureGroup as HTMLElement).getByRole('link', { name: 'vSphere' }),
|
||||
).toBeTruthy();
|
||||
|
||||
const mobileNav = screen.getByRole('navigation', { name: 'Mobile navigation' });
|
||||
@@ -334,22 +337,22 @@ describe('AppLayout navigation icons', () => {
|
||||
},
|
||||
);
|
||||
|
||||
const desktopNav = screen.getByRole('tablist', { name: 'Primary navigation' });
|
||||
const desktopNav = screen.getByRole('navigation', { name: 'Primary navigation' });
|
||||
const infrastructureGroup = desktopNav.querySelector('[aria-label="Infrastructure"]');
|
||||
expect(infrastructureGroup).toBeTruthy();
|
||||
expect(within(infrastructureGroup as HTMLElement).queryByRole('tab')).toBeNull();
|
||||
expect(within(infrastructureGroup as HTMLElement).queryByRole('link')).toBeNull();
|
||||
});
|
||||
|
||||
it('restores the previous Proxmox route state when returning from another platform tab', async () => {
|
||||
renderLayout(platformResources(), '/proxmox/overview?status=running');
|
||||
|
||||
await fireEvent.click(getInfrastructureTab('Docker'));
|
||||
await fireEvent.click(getInfrastructureLink('Docker'));
|
||||
await waitFor(() => {
|
||||
expect(window.location.pathname).toBe('/docker/overview');
|
||||
expect(window.location.search).toBe('');
|
||||
});
|
||||
|
||||
await fireEvent.click(getInfrastructureTab('Proxmox'));
|
||||
await fireEvent.click(getInfrastructureLink('Proxmox'));
|
||||
await waitFor(() => {
|
||||
expect(window.location.pathname).toBe('/proxmox/overview');
|
||||
expect(window.location.search).toBe('?status=running');
|
||||
@@ -365,13 +368,13 @@ describe('AppLayout navigation icons', () => {
|
||||
expect(window.location.search).toBe('?status=running');
|
||||
});
|
||||
|
||||
await fireEvent.click(getInfrastructureTab('Docker'));
|
||||
await fireEvent.click(getInfrastructureLink('Docker'));
|
||||
await waitFor(() => {
|
||||
expect(window.location.pathname).toBe('/docker/overview');
|
||||
expect(window.location.search).toBe('');
|
||||
});
|
||||
|
||||
await fireEvent.click(getInfrastructureTab('Proxmox'));
|
||||
await fireEvent.click(getInfrastructureLink('Proxmox'));
|
||||
await waitFor(() => {
|
||||
expect(window.location.pathname).toBe('/proxmox/overview');
|
||||
expect(window.location.search).toBe('?status=running');
|
||||
@@ -381,13 +384,13 @@ describe('AppLayout navigation icons', () => {
|
||||
it('keeps remembered route state scoped to the platform tab that owns it', async () => {
|
||||
renderLayout(platformResources(), '/docker/overview?host=docker-1');
|
||||
|
||||
await fireEvent.click(getInfrastructureTab('Proxmox'));
|
||||
await fireEvent.click(getInfrastructureLink('Proxmox'));
|
||||
await waitFor(() => {
|
||||
expect(window.location.pathname).toBe('/proxmox/overview');
|
||||
expect(window.location.search).toBe('');
|
||||
});
|
||||
|
||||
await fireEvent.click(getInfrastructureTab('Docker'));
|
||||
await fireEvent.click(getInfrastructureLink('Docker'));
|
||||
await waitFor(() => {
|
||||
expect(window.location.pathname).toBe('/docker/overview');
|
||||
expect(window.location.search).toBe('?host=docker-1');
|
||||
@@ -397,7 +400,7 @@ describe('AppLayout navigation icons', () => {
|
||||
it('uses the canonical platform root route when there is no remembered route state', async () => {
|
||||
renderLayout(platformResources(), '/settings/infrastructure');
|
||||
|
||||
await fireEvent.click(getInfrastructureTab('Proxmox'));
|
||||
await fireEvent.click(getInfrastructureLink('Proxmox'));
|
||||
await waitFor(() => {
|
||||
expect(window.location.pathname).toBe('/proxmox/overview');
|
||||
expect(window.location.search).toBe('');
|
||||
@@ -569,7 +572,7 @@ describe('AppLayout Issue1650 kiosk scope containment', () => {
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
|
||||
|
||||
expect(isKioskMode()).toBe(true);
|
||||
expect(screen.queryByRole('tablist', { name: 'Primary navigation' })).toBeNull();
|
||||
expect(screen.queryByRole('navigation', { name: 'Primary navigation' })).toBeNull();
|
||||
expect(headerIsRevealed(container)).toBe(true);
|
||||
|
||||
// The peek is temporary, exactly like the hover and touch affordances.
|
||||
@@ -585,7 +588,7 @@ describe('AppLayout Issue1650 kiosk scope containment', () => {
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
|
||||
|
||||
expect(isKioskMode()).toBe(false);
|
||||
expect(screen.getByRole('tablist', { name: 'Primary navigation' })).toBeTruthy();
|
||||
expect(screen.getByRole('navigation', { name: 'Primary navigation' })).toBeTruthy();
|
||||
});
|
||||
|
||||
it('redirects a scope-limited session away from settings even when kiosk is off', async () => {
|
||||
@@ -597,9 +600,9 @@ describe('AppLayout Issue1650 kiosk scope containment', () => {
|
||||
expect(window.location.pathname).toBe('/docker/overview');
|
||||
});
|
||||
const systemGroup = screen
|
||||
.getByRole('tablist', { name: 'Primary navigation' })
|
||||
.getByRole('navigation', { name: 'Primary navigation' })
|
||||
.querySelector('[aria-label="System"]');
|
||||
expect(within(systemGroup as HTMLElement).queryByRole('tab', { name: 'Settings' })).toBeNull();
|
||||
expect(within(systemGroup as HTMLElement).queryByRole('link', { name: 'Settings' })).toBeNull();
|
||||
});
|
||||
|
||||
it('leaves settings reachable with kiosk off for a session that has settings access', async () => {
|
||||
@@ -607,7 +610,7 @@ describe('AppLayout Issue1650 kiosk scope containment', () => {
|
||||
renderLayout([], '/settings/infrastructure', DOCKER_ONLY_VISIBILITY, ['settings:read']);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('tablist', { name: 'Primary navigation' })).toBeTruthy();
|
||||
expect(screen.getByRole('navigation', { name: 'Primary navigation' })).toBeTruthy();
|
||||
});
|
||||
expect(window.location.pathname).toBe('/settings/infrastructure');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user