Quick actions refactor + new features (#3131)

#3129 inspired me to add a couple more features to the quick actions
menu, which in turn inspired a big refactor to avoid problems with
possible duplicate entries. I also added a bunch of e2e tests that we
probably should have had before.

### New features 

- `ctrl+n`/`ctrl+p` keyboard navigation (in addition to arrow keys)
- "Go up" actions derived from breadcrumbs, so you can quickly navigate
to parent pages
- Links are real links, which means you can middle click them if you
insist


https://github.com/user-attachments/assets/d40d4d80-88b4-41e6-a41b-e7274520fa11

### Refactor

The old store held a flat list of `QuickActionItem`. 


https://github.com/oxidecomputer/console/blob/751a80e64b32bacbfc22379e70c24383685cb853/app/ui/lib/ActionMenu.tsx#L22-L28

`value` there is the text label displayed in the UI and
(problematically) is also meant to uniquely identify the item. When
calling pages (e.g., the project layout and the instance list page)
called `useQuickActions` with a list of items, `useQuickActions` called
`addActions`, which deduped the list of items based on `value`. The
problem is that items with the same `value` in different groups could
collide — removing by value would remove items you didn't meant to
remove. We had an `invariant` call making sure the values were unique,
but this is actually pretty bad in hindsight because it's probably
possible for a user to name something to create a duplicate. (In
practice we probably avoided this because all user-settable names are
lower-case and all our nav items have upper case letters.)

In any case, the new breadcrumbs feature added a new source of items
that could potentially be a duplicate with something else in the list.
That is what prompted me to get rid of the deduping mechanism
altogether. The new store is a `Map<string, QuickActionItem[]>` keyed by
an ID for each calling page, so registration is just `map.set(id,
items)` and cleanup is `map.delete(id)`. Deduping is unnecessary because
each source component owns its own slot. In theory each source is
responsible for deduping its items, but the way the item lists are
constructed, this is never really a problem. And if a duplicate sneaks
in, it doesn't actually matter.

Finally, since I was already messing everything up, I changed
`QuickActionItem` to take both link and callback items, like we do for
menu items. They're almost all links — only a few modal-opening actions
are not.

Author: david-crespo

app/hooks/use-quick-actions.tsx

@@ -5,45 +5,51 @@
  *
  * Copyright Oxide Computer Company
  */
-import { useEffect, useMemo } from 'react'
-import { useLocation, useNavigate } from 'react-router'
+import { useEffect, useId, useMemo } from 'react'
+import { useLocation } from 'react-router'
 import { create } from 'zustand'
 
+import { useCrumbs } from '~/hooks/use-crumbs'
 import { useCurrentUser } from '~/hooks/use-current-user'
 import { ActionMenu, type QuickActionItem } from '~/ui/lib/ActionMenu'
-import { invariant } from '~/util/invariant'
 import { pb } from '~/util/path-builder'
 
 import { useKey } from './use-key'
 
-type Items = QuickActionItem[]
+export type { QuickActionItem }
 
 type StoreState = {
-  items: Items
+  // Multiple useQuickActions() hooks are active simultaneously — e.g., a
+  // layout registers "Create project" while a nested page registers "Create
+  // instance." Each caller is called a source and gets its own slot keyed by
+  // React useId(), so when a page unmounts, its cleanup removes only its items,
+  // leaving the layout's items intact. The map is flattened into a single list
+  // at render time.
+  itemsBySource: Map<string, QuickActionItem[]>
   isOpen: boolean
 }
 
-// TODO: dedupe by group and value together so we can have, e.g., both silo and
-// system utilization at the same time
-
-// removeByValue dedupes items so they can be added as many times as we want
-// without appearing in the menu multiple times
-const removeByValue = (items: Items, toRemove: Items) => {
-  const valuesToRemove = new Set(toRemove.map((i) => i.value))
-  return items.filter((i) => !valuesToRemove.has(i.value))
-}
-
-const useStore = create<StoreState>(() => ({ items: [], isOpen: false }))
+const useStore = create<StoreState>(() => ({ itemsBySource: new Map(), isOpen: false }))
 
 // zustand docs say it's fine not to put your setters in the store
 // https://github.com/pmndrs/zustand/blob/0426978/docs/guides/practice-with-no-store-actions.md
 
-function addActions(toAdd: Items) {
-  useStore.setState(({ items }) => ({ items: removeByValue(items, toAdd).concat(toAdd) }))
+// These create a new Map each time because zustand uses reference equality to
+// detect changes — mutating in place wouldn't trigger a re-render.
+function setSourceItems(sourceId: string, items: QuickActionItem[]) {
+  useStore.setState(({ itemsBySource }) => {
+    const next = new Map(itemsBySource)
+    next.set(sourceId, items)
+    return { itemsBySource: next }
+  })
 }
 
-function removeActions(toRemove: Items) {
-  useStore.setState(({ items }) => ({ items: removeByValue(items, toRemove) }))
+function clearSource(sourceId: string) {
+  useStore.setState(({ itemsBySource }) => {
+    const next = new Map(itemsBySource)
+    next.delete(sourceId)
+    return { itemsBySource: next }
+  })
 }
 
 export function openQuickActions() {
@@ -54,61 +60,66 @@ function closeQuickActions() {
   useStore.setState({ isOpen: false })
 }
 
-function useGlobalActions() {
+function useGlobalActions(): QuickActionItem[] {
   const location = useLocation()
-  const navigate = useNavigate()
   const { me } = useCurrentUser()
 
   return useMemo(() => {
-    const actions = []
-    // only add settings link if we're not on a settings page
-    if (!location.pathname.startsWith('/settings/')) {
-      actions.push({
-        navGroup: 'User',
-        value: 'Settings',
-        onSelect: () => navigate(pb.profile()),
-      })
-    }
+    const actions: QuickActionItem[] = []
     if (me.fleetViewer && !location.pathname.startsWith('/system/')) {
       actions.push({
         navGroup: 'System',
         value: 'Manage system',
-        onSelect: () => navigate(pb.silos()),
+        action: pb.silos(),
+      })
+    }
+    if (!location.pathname.startsWith('/settings/')) {
+      actions.push({
+        navGroup: 'User',
+        value: 'Settings',
+        action: pb.profile(),
       })
     }
     return actions
-  }, [location.pathname, navigate, me.fleetViewer])
+  }, [location.pathname, me.fleetViewer])
+}
+
+/** Derive "Go up" actions from breadcrumb ancestors of the current page */
+function useParentActions(): QuickActionItem[] {
+  const crumbs = useCrumbs()
+
+  return useMemo(() => {
+    const navCrumbs = crumbs.filter((c) => !c.titleOnly)
+    // Everything except the last crumb (the current page)
+    const parentCrumbs = navCrumbs.slice(0, -1)
+    return parentCrumbs.map((c) => ({
+      value: c.label,
+      action: c.path,
+      navGroup: 'Go up',
+    }))
+  }, [crumbs])
 }
 
 /**
- * Register action items with the global quick actions menu. `itemsToAdd` must
+ * Register action items with the global quick actions menu. `items` must
  * be memoized by the caller, otherwise the effect will run too often.
  *
- * The idea here is that any component in the tree can register actions to go in
- * the menu and having them appear when the component is mounted and not appear
- * when the component is unmounted Just Works.
+ * Each component instance gets its own source slot (via useId), so items are
+ * cleaned up automatically when the component unmounts without affecting
+ * other sources' registrations.
  */
-export function useQuickActions(itemsToAdd: QuickActionItem[]) {
-  const location = useLocation()
-
-  // Add routes without declaring them in every `useQuickActions` call
-  const globalItems = useGlobalActions()
+export function useQuickActions(items: QuickActionItem[]) {
+  const sourceId = useId()
 
   useEffect(() => {
-    const allItems = [...itemsToAdd, ...globalItems]
-    invariant(
-      allItems.length === new Set(allItems.map((i) => i.value)).size,
-      'Items being added to the list of quick actions must have unique `value` values.'
-    )
-    addActions(allItems)
-    return () => removeActions(allItems)
-  }, [itemsToAdd, globalItems, location.pathname])
+    setSourceItems(sourceId, items)
+    return () => clearSource(sourceId)
+  }, [sourceId, items])
 }
 
 function toggleDialog(e: Mousetrap.ExtendedKeyboardEvent) {
-  const { items, isOpen } = useStore.getState()
-
-  if (items.length > 0 && !isOpen) {
+  const { itemsBySource, isOpen } = useStore.getState()
+  if (itemsBySource.size > 0 && !isOpen) {
     e.preventDefault()
     openQuickActions()
   } else {
@@ -117,9 +128,23 @@ function toggleDialog(e: Mousetrap.ExtendedKeyboardEvent) {
 }
 
 export function QuickActions() {
-  const items = useStore((state) => state.items)
+  const itemsBySource = useStore((state) => state.itemsBySource)
   const isOpen = useStore((state) => state.isOpen)
 
+  // Ambient items (global nav + breadcrumb ancestors) are computed inline
+  // rather than stored, so QuickActions never writes to the store it reads.
+  const globalItems = useGlobalActions()
+  const parentItems = useParentActions()
+
+  // Flatten: page items first, then layout items, then breadcrumb ancestors,
+  // then global nav. Pages register after their parent layouts (a fact about
+  // React Router, I think), so reversing itemsBySource puts page-level items
+  // before layout-level items.
+  const items = useMemo(
+    () => [...itemsBySource.values()].reverse().flat().concat(parentItems, globalItems),
+    [itemsBySource, globalItems, parentItems]
+  )
+
   useKey('mod+k', toggleDialog, { global: true })
 
   return (

app/layouts/ProjectLayoutBase.tsx

@@ -6,7 +6,7 @@
  * Copyright Oxide Computer Company
  */
 import { useMemo, type ReactElement } from 'react'
-import { useLocation, useNavigate, type LoaderFunctionArgs } from 'react-router'
+import { useLocation, type LoaderFunctionArgs } from 'react-router'
 
 import { api, q, queryClient, usePrefetchedQuery } from '@oxide/api'
 import {
@@ -53,7 +53,6 @@ export async function projectLayoutLoader({ params }: LoaderFunctionArgs) {
 }
 
 export function ProjectLayoutBase({ overrideContentPane }: ProjectLayoutProps) {
-  const navigate = useNavigate()
   // project will always be there, instance may not
   const projectSelector = useProjectSelector()
   const { data: project } = usePrefetchedQuery(projectView(projectSelector))
@@ -77,9 +76,9 @@ export function ProjectLayoutBase({ overrideContentPane }: ProjectLayoutProps) {
           .map((i) => ({
             navGroup: `Project '${project.name}'`,
             value: i.value,
-            onSelect: () => navigate(i.path),
+            action: i.path,
           })),
-      [pathname, navigate, project.name, projectSelector]
+      [pathname, project.name, projectSelector]
     )
   )
 

app/layouts/SettingsLayout.tsx

@@ -6,7 +6,7 @@
  * Copyright Oxide Computer Company
  */
 import { useMemo } from 'react'
-import { useLocation, useNavigate } from 'react-router'
+import { useLocation } from 'react-router'
 
 import {
   AccessToken16Icon,
@@ -27,7 +27,6 @@ import { ContentPane, PageContainer } from './helpers'
 export const handle = makeCrumb('Settings', pb.profile())
 
 export default function SettingsLayout() {
-  const navigate = useNavigate()
   const { pathname } = useLocation()
 
   useQuickActions(
@@ -43,9 +42,9 @@ export default function SettingsLayout() {
           .map((i) => ({
             navGroup: `Settings`,
             value: i.value,
-            onSelect: () => navigate(i.path),
+            action: i.path,
           })),
-      [pathname, navigate]
+      [pathname]
     )
   )
 

app/layouts/SiloLayout.tsx

@@ -6,7 +6,7 @@
  * Copyright Oxide Computer Company
  */
 import { useMemo } from 'react'
-import { useLocation, useNavigate } from 'react-router'
+import { useLocation } from 'react-router'
 
 import {
   Access16Icon,
@@ -25,7 +25,6 @@ import { pb } from '~/util/path-builder'
 import { ContentPane, PageContainer } from './helpers'
 
 export default function SiloLayout() {
-  const navigate = useNavigate()
   const { pathname } = useLocation()
   const { me } = useCurrentUser()
 
@@ -43,9 +42,9 @@ export default function SiloLayout() {
           .map((i) => ({
             navGroup: `Silo '${me.siloName}'`,
             value: i.value,
-            onSelect: () => navigate(i.path),
+            action: i.path,
           })),
-      [pathname, navigate, me.siloName]
+      [pathname, me.siloName]
     )
   )
 

app/layouts/SystemLayout.tsx

@@ -6,7 +6,7 @@
  * Copyright Oxide Computer Company
  */
 import { useMemo } from 'react'
-import { useLocation, useNavigate } from 'react-router'
+import { useLocation } from 'react-router'
 
 import { api, q, queryClient } from '@oxide/api'
 import {
@@ -22,7 +22,7 @@ import { trigger404 } from '~/components/ErrorBoundary'
 import { DocsLinkItem, NavLinkItem, Sidebar } from '~/components/Sidebar'
 import { TopBar } from '~/components/TopBar'
 import { useCurrentUser } from '~/hooks/use-current-user'
-import { useQuickActions } from '~/hooks/use-quick-actions'
+import { useQuickActions, type QuickActionItem } from '~/hooks/use-quick-actions'
 import { Divider } from '~/ui/lib/Divider'
 import { inventoryBase, pb } from '~/util/path-builder'
 
@@ -44,7 +44,6 @@ export default function SystemLayout() {
   // robust way of doing this would be to make a separate layout for the
   // silo-specific routes in the route config, but it's overkill considering
   // this is a one-liner. Switch to that approach at the first sign of trouble.
-  const navigate = useNavigate()
   const { pathname } = useLocation()
 
   const { me } = useCurrentUser()
@@ -63,16 +62,16 @@ export default function SystemLayout() {
       .map((i) => ({
         navGroup: 'System',
         value: i.value,
-        onSelect: () => navigate(i.path),
+        action: i.path,
       }))
 
-    const backToSilo = {
+    const backToSilo: QuickActionItem = {
       navGroup: `Back to silo '${me.siloName}'`,
       value: 'Projects',
-      onSelect: () => navigate(pb.projects()),
+      action: pb.projects(),
     }
     return [...systemLinks, backToSilo]
-  }, [pathname, navigate, me.siloName])
+  }, [pathname, me.siloName])
 
   useQuickActions(actions)
 

app/pages/ProjectsPage.tsx

@@ -108,15 +108,16 @@ export default function ProjectsPage() {
       () => [
         {
           value: 'New project',
-          onSelect: () => navigate(pb.projectsNew()),
+          navGroup: 'Actions',
+          action: pb.projectsNew(),
         },
         ...(allProjects?.items || []).map((p) => ({
           value: p.name,
-          onSelect: () => navigate(pb.project({ project: p.name })),
+          action: pb.project({ project: p.name }),
           navGroup: 'Go to project',
         })),
       ],
-      [navigate, allProjects]
+      [allProjects]
     )
   )
 

app/pages/SiloAccessPage.tsx

@@ -170,7 +170,8 @@ export default function SiloAccessPage() {
       () => [
         {
           value: 'Add user or group',
-          onSelect: () => setAddModalOpen(true),
+          navGroup: 'Actions',
+          action: () => setAddModalOpen(true),
         },
       ],
       []

app/pages/SiloImagesPage.tsx

@@ -9,7 +9,7 @@ import { useQuery } from '@tanstack/react-query'
 import { createColumnHelper } from '@tanstack/react-table'
 import { useCallback, useMemo, useState } from 'react'
 import { useForm } from 'react-hook-form'
-import { Outlet, useNavigate } from 'react-router'
+import { Outlet } from 'react-router'
 
 import { api, getListQFn, q, queryClient, useApiMutation, type Image } from '@oxide/api'
 import { Images16Icon, Images24Icon } from '@oxide/design-system/icons/react'
@@ -96,22 +96,22 @@ export default function SiloImagesPage() {
   const columns = useColsWithActions(staticCols, makeActions)
   const { table } = useQueryTable({ query: imageList, columns, emptyState: <EmptyState /> })
 
-  const navigate = useNavigate()
   const { data: allImages } = useQuery(q(api.imageList, { query: { limit: ALL_ISH } }))
   useQuickActions(
     useMemo(
       () => [
         {
           value: 'Promote image',
-          onSelect: () => setShowModal(true),
+          navGroup: 'Actions',
+          action: () => setShowModal(true),
         },
         ...(allImages?.items || []).map((i) => ({
           value: i.name,
-          onSelect: () => navigate(pb.siloImageEdit({ image: i.name })),
+          action: pb.siloImageEdit({ image: i.name }),
           navGroup: 'Go to silo image',
         })),
       ],
-      [navigate, allImages]
+      [allImages]
     )
   )