add toPrefix to NavLinks in Sidebar to better highlight isActive

app/components/RouteTabs.tsx

@@ -84,11 +84,11 @@ export interface TabProps {
    * able to link directly to the first sidebar tab, but we of course also want
    * this tab to appear active for all the sidebar tabs. See instance metrics.
    */
-  toPrefix?: string
+  activePrefix?: string
   children: ReactNode
 }
-export const Tab = ({ to, toPrefix, children }: TabProps) => {
-  const isActive = useIsActivePath({ to: toPrefix || to })
+export const Tab = ({ to, activePrefix, children }: TabProps) => {
+  const isActive = useIsActivePath({ to: activePrefix || to })
   return (
     <Link
       role="tab"

app/components/Sidebar.tsx

@@ -6,10 +6,11 @@
  * Copyright Oxide Computer Company
  */
 import cn from 'classnames'
-import { NavLink, useLocation } from 'react-router'
+import { Link, useLocation } from 'react-router'
 
 import { Action16Icon, Document16Icon } from '@oxide/design-system/icons/react'
 
+import { useIsActivePath } from '~/hooks/use-is-active-path'
 import { openQuickActions } from '~/hooks/use-quick-actions'
 import { Button } from '~/ui/lib/Button'
 import { Truncate } from '~/ui/lib/Truncate'
@@ -83,29 +84,40 @@ Sidebar.Nav = ({ children, heading }: SidebarNav) => (
   </div>
 )
 
-export const NavLinkItem = (props: {
+type NavLinkProps = {
   to: string
   children: React.ReactNode
   end?: boolean
   disabled?: boolean
-}) => {
+  // Only for cases where we want to spoof the path and pretend 'isActive'
+  activePrefix?: string
+}
+
+export const NavLinkItem = ({
+  to,
+  children,
+  end,
+  disabled,
+  activePrefix,
+}: NavLinkProps) => {
   // If the current page is the create form for this NavLinkItem's resource, highlight the NavLink in the sidebar
-  const currentPathIsCreateForm = useLocation().pathname.startsWith(`${props.to}-new`)
+  const currentPathIsCreateForm = useLocation().pathname.startsWith(`${to}-new`)
+  // We aren't using NavLink, as we need to occasionally use an activePrefix to create an active state for matching root paths
+  // so we also recreate the isActive logic here
+  const isActive = useIsActivePath({ to: activePrefix || to, end })
   return (
     <li>
-      <NavLink
-        to={props.to}
-        className={({ isActive }) =>
-          cn(linkStyles, {
-            'text-accent !bg-accent-secondary hover:!bg-accent-secondary-hover [&>svg]:!text-accent-tertiary':
-              isActive || currentPathIsCreateForm,
-            'pointer-events-none text-disabled': props.disabled,
-          })
-        }
-        end={props.end}
+      <Link
+        to={to}
+        className={cn(linkStyles, {
+          'text-accent !bg-accent-secondary hover:!bg-accent-secondary-hover [&>svg]:!text-accent-tertiary':
+            isActive || currentPathIsCreateForm,
+          'pointer-events-none text-disabled': disabled,
+        })}
+        aria-current={isActive ? 'page' : undefined}
       >
-        {props.children}
-      </NavLink>
+        {children}
+      </Link>
     </li>
   )
 }

app/layouts/SystemLayout.tsx

@@ -22,7 +22,7 @@ import { TopBar } from '~/components/TopBar'
 import { useCurrentUser } from '~/hooks/use-current-user'
 import { useQuickActions } from '~/hooks/use-quick-actions'
 import { Divider } from '~/ui/lib/Divider'
-import { pb } from '~/util/path-builder'
+import { inventoryBase, pb } from '~/util/path-builder'
 
 import { ContentPane, PageContainer } from './helpers'
 
@@ -102,7 +102,7 @@ export default function SystemLayout() {
           <NavLinkItem to={pb.systemUtilization()}>
             <Metrics16Icon /> Utilization
           </NavLinkItem>
-          <NavLinkItem to={pb.sledInventory()}>
+          <NavLinkItem to={pb.sledInventory()} activePrefix={inventoryBase()}>
             <Servers16Icon /> Inventory
           </NavLinkItem>
           <NavLinkItem to={pb.ipPools()}>

app/pages/project/instances/InstancePage.tsx

@@ -258,7 +258,7 @@ export default function InstancePage() {
         <Tab to={pb.instanceNetworking(instanceSelector)}>Networking</Tab>
         <Tab
           to={pb.instanceCpuMetrics(instanceSelector)}
-          toPrefix={instanceMetricsBase(instanceSelector)}
+          activePrefix={instanceMetricsBase(instanceSelector)}
         >
           Metrics
         </Tab>

app/pages/system/inventory/SledsTab.tsx

@@ -40,7 +40,7 @@ export const handle = { crumb: 'Sleds' }
 const colHelper = createColumnHelper<Sled>()
 const staticCols = [
   colHelper.accessor('id', {
-    cell: makeLinkCell((sledId) => pb.sled({ sledId })),
+    cell: makeLinkCell((sledId) => pb.sledInstances({ sledId })),
   }),
   // TODO: colHelper.accessor('baseboard.serviceAddress', { header: 'service address' }),
   colHelper.group({

app/pages/system/inventory/sled/SledPage.tsx

@@ -28,7 +28,7 @@ export async function clientLoader({ params }: LoaderFunctionArgs) {
 }
 export const handle = makeCrumb(
   (p) => truncate(p.sledId!, 12, 'middle'),
-  (p) => pb.sled({ sledId: p.sledId! })
+  (p) => pb.sledInstances({ sledId: p.sledId! })
 )
 
 export default function SledPage() {

app/util/__snapshots__/path-builder.spec.ts.snap

@@ -529,24 +529,6 @@ exports[`breadcrumbs 2`] = `
       "path": "/system/silos",
     },
   ],
-  "sled (/system/inventory/sleds/5c56b522-c9b8-49e4-9f9a-8d52a89ec3e0/instances)": [
-    {
-      "label": "Inventory",
-      "path": "/system/inventory",
-    },
-    {
-      "label": "Sleds",
-      "path": "/system/inventory/sleds",
-    },
-    {
-      "label": "5c56b…ec3e0",
-      "path": "/system/inventory/sleds/5c56b522-c9b8-49e4-9f9a-8d52a89ec3e0/instances",
-    },
-    {
-      "label": "Instances",
-      "path": "/system/inventory/sleds/5c56b522-c9b8-49e4-9f9a-8d52a89ec3e0/instances",
-    },
-  ],
   "sledInstances (/system/inventory/sleds/5c56b522-c9b8-49e4-9f9a-8d52a89ec3e0/instances)": [
     {
       "label": "Inventory",

app/util/path-builder.spec.ts

@@ -81,7 +81,6 @@ test('path builder', () => {
         "siloUtilization": "/utilization",
         "silos": "/system/silos",
         "silosNew": "/system/silos-new",
-        "sled": "/system/inventory/sleds/5c56b522-c9b8-49e4-9f9a-8d52a89ec3e0/instances",
         "sledInstances": "/system/inventory/sleds/5c56b522-c9b8-49e4-9f9a-8d52a89ec3e0/instances",
         "sledInventory": "/system/inventory/sleds",
         "snapshotImagesNew": "/projects/p/snapshots/sn/images-new",

app/util/path-builder.ts

@@ -15,9 +15,10 @@ const instanceBase = ({ project, instance }: PP.Instance) =>
   `${pb.instances({ project })}/${instance}`
 const vpcBase = ({ project, vpc }: PP.Vpc) => `${pb.vpcs({ project })}/${vpc}`
 
-/** Don't use this for links. only exported for use as toPrefix on metrics tab */
+/** Don't use these for links. only exported for use with activePrefix */
 export const instanceMetricsBase = ({ project, instance }: PP.Instance) =>
   `${instanceBase({ project, instance })}/metrics`
+export const inventoryBase = () => '/system/inventory'
 
 export const pb = {
   projects: () => `/projects`,
@@ -109,10 +110,9 @@ export const pb = {
   ipPoolEdit: (params: PP.IpPool) => `${pb.ipPool(params)}/edit`,
   ipPoolRangeAdd: (params: PP.IpPool) => `${pb.ipPool(params)}/ranges-add`,
 
-  sledInventory: () => '/system/inventory/sleds',
-  diskInventory: () => '/system/inventory/disks',
-  sled: ({ sledId }: PP.Sled) => `/system/inventory/sleds/${sledId}/instances`,
-  sledInstances: ({ sledId }: PP.Sled) => `/system/inventory/sleds/${sledId}/instances`,
+  sledInventory: () => `${inventoryBase()}/sleds`,
+  diskInventory: () => `${inventoryBase()}/disks`,
+  sledInstances: ({ sledId }: PP.Sled) => `${pb.sledInventory()}/${sledId}/instances`,
 
   silos: () => '/system/silos',
   silosNew: () => '/system/silos-new',