diff --git a/app/hooks/use-quick-actions.tsx b/app/hooks/use-quick-actions.tsx index eaf16463b..5848eab71 100644 --- a/app/hooks/use-quick-actions.tsx +++ b/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 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(() => ({ items: [], isOpen: false })) +const useStore = create(() => ({ 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 ( diff --git a/app/layouts/ProjectLayoutBase.tsx b/app/layouts/ProjectLayoutBase.tsx index 74e734a84..c021b08ae 100644 --- a/app/layouts/ProjectLayoutBase.tsx +++ b/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] ) ) diff --git a/app/layouts/SettingsLayout.tsx b/app/layouts/SettingsLayout.tsx index eb3238ca3..649e2d159 100644 --- a/app/layouts/SettingsLayout.tsx +++ b/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] ) ) diff --git a/app/layouts/SiloLayout.tsx b/app/layouts/SiloLayout.tsx index 361727119..d07076511 100644 --- a/app/layouts/SiloLayout.tsx +++ b/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] ) ) diff --git a/app/layouts/SystemLayout.tsx b/app/layouts/SystemLayout.tsx index 9faa528c9..05c0e7434 100644 --- a/app/layouts/SystemLayout.tsx +++ b/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) diff --git a/app/pages/ProjectsPage.tsx b/app/pages/ProjectsPage.tsx index cf6612329..9fdd256f4 100644 --- a/app/pages/ProjectsPage.tsx +++ b/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] ) ) diff --git a/app/pages/SiloAccessPage.tsx b/app/pages/SiloAccessPage.tsx index 87ae0a5c2..c93b19a02 100644 --- a/app/pages/SiloAccessPage.tsx +++ b/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), }, ], [] diff --git a/app/pages/SiloImagesPage.tsx b/app/pages/SiloImagesPage.tsx index 7cc6f5e53..e0209d130 100644 --- a/app/pages/SiloImagesPage.tsx +++ b/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: }) - 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] ) ) diff --git a/app/pages/project/access/ProjectAccessPage.tsx b/app/pages/project/access/ProjectAccessPage.tsx index 254144480..f99c926ff 100644 --- a/app/pages/project/access/ProjectAccessPage.tsx +++ b/app/pages/project/access/ProjectAccessPage.tsx @@ -211,7 +211,8 @@ export default function ProjectAccessPage() { () => [ { value: 'Add user or group', - onSelect: () => setAddModalOpen(true), + navGroup: 'Actions', + action: () => setAddModalOpen(true), }, ], [] diff --git a/app/pages/project/affinity/AffinityPage.tsx b/app/pages/project/affinity/AffinityPage.tsx index 1860bd0b7..d15568910 100644 --- a/app/pages/project/affinity/AffinityPage.tsx +++ b/app/pages/project/affinity/AffinityPage.tsx @@ -8,7 +8,7 @@ import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table' import { useCallback, useMemo } from 'react' -import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router' +import { Outlet, type LoaderFunctionArgs } from 'react-router' import { api, @@ -139,22 +139,21 @@ export default function AffinityPage() { getCoreRowModel: getCoreRowModel(), }) - const navigate = useNavigate() useQuickActions( useMemo( () => [ { value: 'New anti-affinity group', - onSelect: () => navigate(pb.affinityNew({ project })), + navGroup: 'Actions', + action: pb.affinityNew({ project }), }, ...antiAffinityGroups.map((g) => ({ value: g.name, - onSelect: () => - navigate(pb.antiAffinityGroup({ project, antiAffinityGroup: g.name })), + action: pb.antiAffinityGroup({ project, antiAffinityGroup: g.name }), navGroup: 'Go to anti-affinity group', })), ], - [navigate, project, antiAffinityGroups] + [project, antiAffinityGroups] ) ) diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index 247259c1d..b9acc3627 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -8,7 +8,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo } from 'react' -import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router' +import { Outlet, type LoaderFunctionArgs } from 'react-router' import { api, @@ -189,7 +189,6 @@ export default function DisksPage() { emptyState: , }) - const navigate = useNavigate() const { data: allDisks } = useQuery( q(api.diskList, { query: { project, limit: ALL_ISH } }) ) @@ -198,15 +197,16 @@ export default function DisksPage() { () => [ { value: 'New disk', - onSelect: () => navigate(pb.disksNew({ project })), + navGroup: 'Actions', + action: pb.disksNew({ project }), }, ...(allDisks?.items || []).map((d) => ({ value: d.name, - onSelect: () => navigate(pb.disk({ project, disk: d.name })), + action: pb.disk({ project, disk: d.name }), navGroup: 'Go to disk', })), ], - [navigate, project, allDisks] + [project, allDisks] ) ) diff --git a/app/pages/project/floating-ips/FloatingIpsPage.tsx b/app/pages/project/floating-ips/FloatingIpsPage.tsx index 120a73a28..6ffea50fe 100644 --- a/app/pages/project/floating-ips/FloatingIpsPage.tsx +++ b/app/pages/project/floating-ips/FloatingIpsPage.tsx @@ -221,15 +221,16 @@ export default function FloatingIpsPage() { () => [ { value: 'New floating IP', - onSelect: () => navigate(pb.floatingIpsNew({ project })), + navGroup: 'Actions', + action: pb.floatingIpsNew({ project }), }, ...(allFips?.items || []).map((f) => ({ value: f.name, - onSelect: () => navigate(pb.floatingIpEdit({ project, floatingIp: f.name })), + action: pb.floatingIpEdit({ project, floatingIp: f.name }), navGroup: 'Go to floating IP', })), ], - [project, navigate, allFips] + [project, allFips] ) ) diff --git a/app/pages/project/images/ImagesPage.tsx b/app/pages/project/images/ImagesPage.tsx index 852fa60a0..b464a48c2 100644 --- a/app/pages/project/images/ImagesPage.tsx +++ b/app/pages/project/images/ImagesPage.tsx @@ -8,7 +8,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' -import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router' +import { Outlet, type LoaderFunctionArgs } from 'react-router' import { api, getListQFn, q, queryClient, useApiMutation, type Image } from '@oxide/api' import { Images16Icon, Images24Icon } from '@oxide/design-system/icons/react' @@ -109,7 +109,6 @@ export default function ImagesPage() { emptyState: , }) - const navigate = useNavigate() const { data: allImages } = useQuery( q(api.imageList, { query: { project, limit: ALL_ISH } }) ) @@ -118,15 +117,16 @@ export default function ImagesPage() { () => [ { value: 'Upload image', - onSelect: () => navigate(pb.projectImagesNew({ project })), + navGroup: 'Actions', + action: pb.projectImagesNew({ project }), }, ...(allImages?.items || []).map((i) => ({ value: i.name, - onSelect: () => navigate(pb.projectImageEdit({ project, image: i.name })), + action: pb.projectImageEdit({ project, image: i.name }), navGroup: 'Go to project image', })), ], - [project, navigate, allImages] + [project, allImages] ) ) diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index 9e5b22aa5..8e9080eb5 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -9,7 +9,7 @@ import { useQuery, type UseQueryOptions } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { filesize } from 'filesize' import { useMemo, useRef, useState } from 'react' -import { useNavigate, type LoaderFunctionArgs } from 'react-router' +import { type LoaderFunctionArgs } from 'react-router' import { api, @@ -198,21 +198,21 @@ export default function InstancesPage() { const { data: allInstances } = useQuery( q(api.instanceList, { query: { project, limit: ALL_ISH } }) ) - const navigate = useNavigate() useQuickActions( useMemo( () => [ { value: 'New instance', - onSelect: () => navigate(pb.instancesNew({ project })), + navGroup: 'Actions', + action: pb.instancesNew({ project }), }, ...(allInstances?.items || []).map((i) => ({ value: i.name, - onSelect: () => navigate(pb.instance({ project, instance: i.name })), + action: pb.instance({ project, instance: i.name }), navGroup: 'Go to instance', })), ], - [project, allInstances, navigate] + [project, allInstances] ) ) diff --git a/app/pages/project/snapshots/SnapshotsPage.tsx b/app/pages/project/snapshots/SnapshotsPage.tsx index a9f30404a..ceeefe16d 100644 --- a/app/pages/project/snapshots/SnapshotsPage.tsx +++ b/app/pages/project/snapshots/SnapshotsPage.tsx @@ -171,10 +171,11 @@ export default function SnapshotsPage() { () => [ { value: 'New snapshot', - onSelect: () => navigate(pb.snapshotsNew({ project })), + navGroup: 'Actions', + action: pb.snapshotsNew({ project }), }, ], - [navigate, project] + [project] ) ) diff --git a/app/pages/project/vpcs/VpcsPage.tsx b/app/pages/project/vpcs/VpcsPage.tsx index 99a3af503..90ab42fd9 100644 --- a/app/pages/project/vpcs/VpcsPage.tsx +++ b/app/pages/project/vpcs/VpcsPage.tsx @@ -141,15 +141,16 @@ export default function VpcsPage() { () => [ { value: 'New VPC', - onSelect: () => navigate(pb.vpcsNew({ project })), + navGroup: 'Actions', + action: pb.vpcsNew({ project }), }, ...(allVpcs?.items || []).map((v) => ({ value: v.name, - onSelect: () => navigate(pb.vpc({ project, vpc: v.name })), + action: pb.vpc({ project, vpc: v.name }), navGroup: 'Go to VPC', })), ], - [project, allVpcs, navigate] + [project, allVpcs] ) ) diff --git a/app/pages/system/networking/IpPoolsPage.tsx b/app/pages/system/networking/IpPoolsPage.tsx index a908ccdd2..ec4c5432e 100644 --- a/app/pages/system/networking/IpPoolsPage.tsx +++ b/app/pages/system/networking/IpPoolsPage.tsx @@ -137,15 +137,16 @@ export default function IpPoolsPage() { () => [ { value: 'New IP pool', - onSelect: () => navigate(pb.ipPoolsNew()), + navGroup: 'Actions', + action: pb.ipPoolsNew(), }, ...(allPools?.items || []).map((p) => ({ value: p.name, - onSelect: () => navigate(pb.ipPool({ pool: p.name })), + action: pb.ipPool({ pool: p.name }), navGroup: 'Go to IP pool', })), ], - [navigate, allPools] + [allPools] ) ) diff --git a/app/pages/system/silos/SilosPage.tsx b/app/pages/system/silos/SilosPage.tsx index e23c40838..a560e5d81 100644 --- a/app/pages/system/silos/SilosPage.tsx +++ b/app/pages/system/silos/SilosPage.tsx @@ -8,7 +8,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo } from 'react' -import { Outlet, useNavigate } from 'react-router' +import { Outlet } from 'react-router' import { api, getListQFn, q, queryClient, useApiMutation, type Silo } from '@oxide/api' import { Cloud16Icon, Cloud24Icon } from '@oxide/design-system/icons/react' @@ -69,8 +69,6 @@ export async function clientLoader() { export const handle = makeCrumb('Silos', pb.silos()) export default function SilosPage() { - const navigate = useNavigate() - const { mutateAsync: deleteSilo } = useApiMutation(api.siloDelete, { onSuccess(_silo, { path }) { queryClient.invalidateEndpoint('siloList') @@ -103,14 +101,14 @@ export default function SilosPage() { useQuickActions( useMemo( () => [ - { value: 'New silo', onSelect: () => navigate(pb.silosNew()) }, + { value: 'New silo', navGroup: 'Actions', action: pb.silosNew() }, ...(allSilos?.items || []).map((o) => ({ value: o.name, - onSelect: () => navigate(pb.silo({ silo: o.name })), + action: pb.silo({ silo: o.name }), navGroup: 'Go to silo', })), ], - [navigate, allSilos] + [allSilos] ) ) diff --git a/app/ui/lib/ActionMenu.tsx b/app/ui/lib/ActionMenu.tsx index 29ac84828..90b2d9d8d 100644 --- a/app/ui/lib/ActionMenu.tsx +++ b/app/ui/lib/ActionMenu.tsx @@ -9,22 +9,22 @@ import { Dialog as BaseDialog } from '@base-ui/react/dialog' import cn from 'classnames' import { matchSorter } from 'match-sorter' import { useRef, useState } from 'react' +import { Link, useNavigate } from 'react-router' +import * as R from 'remeda' import { Close12Icon } from '@oxide/design-system/icons/react' import { KEYS } from '~/ui/util/keys' -import { groupBy } from '~/util/array' import { classed } from '~/util/classed' import { DialogOverlay } from './DialogOverlay' import { useSteppedScroll } from './use-stepped-scroll' -export interface QuickActionItem { +export type QuickActionItem = { value: string - // strings are paths to navigate() to - // onSelect: string | (() => void) - onSelect: () => void - navGroup?: string + navGroup: string + /** Path string to navigate to or callback to invoke */ + action: string | (() => void) } export interface ActionMenuProps { @@ -40,7 +40,13 @@ const LIST_HEIGHT = 384 const Outline = classed.div`absolute z-10 h-full w-full border border-accent pointer-events-none` +const liBase = + 'text-sans-md border-secondary box-border block h-full w-full cursor-pointer overflow-visible border select-none' +const liSelected = 'text-accent bg-accent hover:bg-accent-hover' +const liDefault = 'text-default bg-raise hover:bg-hover' + export function ActionMenu(props: ActionMenuProps) { + const navigate = useNavigate() const [input, setInput] = useState('') // TODO: filter by both nav group and item value const items = matchSorter(props.items, input, { @@ -49,26 +55,21 @@ export function ActionMenu(props: ActionMenuProps) { baseSort: (a, b) => (a.index < b.index ? -1 : 1), }) - // items without a navGroup label are considered actions and rendered first - const actions = items.filter((i) => !i.navGroup) - - // TODO: repent. this is horrible - const groupedItems = groupBy( - items.filter((i) => i.navGroup), - (i) => i.navGroup! - ) - - const allGroups: [string, QuickActionItem[]][] = - actions.length > 0 - ? [['Actions', items.filter((i) => !i.navGroup)], ...groupedItems] - : groupedItems - - const itemsInOrder = ([] as QuickActionItem[]).concat( - ...allGroups.map(([_, items]) => items) + const allGroups = R.pipe( + items, + R.groupBy((i) => i.navGroup), + R.entries(), + // "Actions" first so page-level create/add actions are always at the top. + // Other sorting by group that we've already done in the calling code is + // preserved. + R.sortBy(([key]) => key !== 'Actions') ) + const itemsInOrder = allGroups.flatMap(([_, items]) => items) + // Map each item to its global index for O(1) lookup during render + const itemIndex = new Map(itemsInOrder.map((item, i) => [item, i])) const [selectedIdx, setSelectedIdx] = useState(0) - const selectedItem = itemsInOrder[selectedIdx] as QuickActionItem | undefined + const selectedItem = itemsInOrder.at(selectedIdx) const divRef = useRef(null) const ulRef = useRef(null) @@ -100,14 +101,20 @@ export function ActionMenu(props: ActionMenuProps) { const lastIdx = itemsInOrder.length - 1 if (e.key === KEYS.enter) { if (selectedItem) { - selectedItem.onSelect() + if (typeof selectedItem.action === 'function') { + selectedItem.action() + } else { + navigate(selectedItem.action) + } e.preventDefault() onDismiss() } - } else if (e.key === KEYS.down) { + } else if (e.key === KEYS.down || (e.ctrlKey && e.key === 'n')) { + e.preventDefault() const newIdx = selectedIdx === lastIdx ? 0 : selectedIdx + 1 setSelectedIdx(newIdx) - } else if (e.key === KEYS.up) { + } else if (e.key === KEYS.up || (e.ctrlKey && e.key === 'p')) { + e.preventDefault() const newIdx = selectedIdx === 0 ? lastIdx : selectedIdx - 1 setSelectedIdx(newIdx) } @@ -123,6 +130,12 @@ export function ActionMenu(props: ActionMenuProps) { )} >
    - {allGroups.map(([label, items]) => ( + {allGroups.map(([label, groupItems]) => (
    -

    +

    {label}

    - {items.map((item) => ( -
    - {item.value === selectedItem?.value && } - - {/* - TODO: there is probably a more correct way of fixing this reasonable lint error. - Putting a button inside the
  • is not a great solution because it becomes - focusable separate from the item selection - */} - - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events */} -
  • { - item.onSelect() - onDismiss() - }} + {groupItems.map((item) => { + const idx = itemIndex.get(item)! + const isSelected = idx === selectedIdx + const { action } = item + return ( +
    - {item.value} -
    • -
    - ))} + {isSelected && } + {typeof action === 'string' ? ( +
  • + { + if (!e.metaKey && !e.ctrlKey && !e.shiftKey) { + onDismiss() + } + }} + > + {item.value} + +
    • + ) : ( + // Keyboard events handled by combobox div above + // eslint-disable-next-line jsx-a11y/click-events-have-key-events +
  • { + action() + onDismiss() + }} + > + {item.value} +
    • + )} +
    + ) + })} ))}
diff --git a/test/e2e/action-menu.e2e.ts b/test/e2e/action-menu.e2e.ts index 6aadc1760..6f1aa9f46 100644 --- a/test/e2e/action-menu.e2e.ts +++ b/test/e2e/action-menu.e2e.ts @@ -16,14 +16,118 @@ const openActionMenu = async (page: Page) => { await expect(page.getByText('Enterto submit')).toBeVisible() } -test('Ensure that the Enter key in the ActionMenu doesn’t prematurely submit a linked form', async ({ - page, -}) => { +const getSelectedItem = (page: Page) => page.getByRole('option', { selected: true }) + +test('Enter key does not prematurely submit a linked form', async ({ page }) => { await page.goto('/system/silos') await openActionMenu(page) - // "New silo" is the first item in the list, so we can just hit enter to open the modal + // wait for page-level quick actions to register before pressing Enter + await expect(page.getByRole('option', { name: 'New silo' })).toBeVisible() await page.keyboard.press('Enter') await expect(page.getByRole('dialog', { name: 'Create silo' })).toBeVisible() // make sure error text is not visible await expectNotVisible(page, [page.getByText('Name is required')]) }) + +test('Ctrl+N/P navigate up and down', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await openActionMenu(page) + + // wait for instance list to load so the items don't shift mid-test + await expect(page.getByRole('option', { name: 'db1' })).toBeVisible() + + const selected = getSelectedItem(page) + // first item is selected by default + await expect(selected).toHaveText('New instance') + + // Ctrl+N moves down + await page.keyboard.press('Control+n') + await expect(selected).toHaveText('db1') + + // Ctrl+P moves back up + await page.keyboard.press('Control+p') + await expect(selected).toHaveText('New instance') + + // Ctrl+N again gets the same second item + await page.keyboard.press('Control+n') + await expect(selected).toHaveText('db1') +}) + +test('Arrow keys navigate up and down', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await openActionMenu(page) + + await expect(page.getByRole('option', { name: 'db1' })).toBeVisible() + + const selected = getSelectedItem(page) + await expect(selected).toHaveText('New instance') + + await page.keyboard.press('ArrowDown') + await expect(selected).toHaveText('db1') + + await page.keyboard.press('ArrowUp') + await expect(selected).toHaveText('New instance') +}) + +test('search filters items', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await openActionMenu(page) + + // wait for instance list items to load before searching + await expect(page.getByRole('option', { name: 'db1' })).toBeVisible() + + // type a search query that matches a specific instance + await page.keyboard.type('db1') + // the matching item should be visible + await expect(page.getByRole('option', { name: 'db1' })).toBeVisible() + // an unrelated item should be filtered out + await expect(page.getByRole('option', { name: 'New instance' })).toBeHidden() +}) + +test('"Go up" actions derived from breadcrumbs', async ({ page }) => { + await page.goto('/projects/mock-project/disks') + await openActionMenu(page) + + // breadcrumb ancestors should appear in a "Go up" group + await expect(page.getByText('Go up')).toBeVisible() + await expect(page.getByRole('option', { name: 'Projects' })).toBeVisible() + await expect(page.getByRole('option', { name: 'mock-project' })).toBeVisible() + + // selecting "Projects" navigates to the projects page + await page.keyboard.type('Projects') + await page.keyboard.press('Enter') + await expect(page).toHaveURL('/projects') +}) + +test('Enter navigates to selected item', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await openActionMenu(page) + + // search for a specific instance and hit Enter + await page.keyboard.type('db1') + await expect(getSelectedItem(page)).toHaveText('db1') + await page.keyboard.press('Enter') + await expect(page).toHaveURL('/projects/mock-project/instances/db1/storage') +}) + +test('items from page and layout sources coexist', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await openActionMenu(page) + + // page-level actions (from InstancesPage) + await expect(page.getByRole('option', { name: 'New instance' })).toBeVisible() + // page-level "Go to" group (from InstancesPage) + await expect(page.getByText('Go to instance')).toBeVisible() + await expect(page.getByRole('option', { name: 'db1' })).toBeVisible() + // layout-level group (from ProjectLayoutBase) + await expect(page.getByText("Project 'mock-project'")).toBeVisible() + await expect(page.getByRole('option', { name: 'Disks' })).toBeVisible() +}) + +test('dismiss with Escape', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await openActionMenu(page) + + await page.keyboard.press('Escape') + await expect(page.getByText('Enterto submit')).toBeHidden() +})