From aaa586c86d62061800f37c406eacf48c63861adc Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 27 Oct 2022 22:48:26 -0500 Subject: [PATCH 01/19] Move instance tabs to routes --- app/components/RouteTabs.tsx | 29 +++++++++++++ .../instances/instance/InstancePage.tsx | 41 ++++-------------- .../instances/instance/tabs/MetricsTab.tsx | 2 +- app/routes.tsx | 38 +++++++++++++--- app/util/path-builder.ts | 13 ++++++ libs/ui/lib/tabs/Tabs.css | 43 +++++++++++++------ libs/ui/lib/tabs/Tabs.tsx | 26 +++-------- 7 files changed, 119 insertions(+), 73 deletions(-) create mode 100644 app/components/RouteTabs.tsx diff --git a/app/components/RouteTabs.tsx b/app/components/RouteTabs.tsx new file mode 100644 index 0000000000..0360602623 --- /dev/null +++ b/app/components/RouteTabs.tsx @@ -0,0 +1,29 @@ +import cn from 'classnames' +import type { LinkProps } from 'react-router-dom' +import { NavLink, Outlet } from 'react-router-dom' + +import type { ChildrenProp } from '@oxide/util' + +type RouteTabsProps = ChildrenProp & { fullWidth?: boolean } +export function RouteTabs({ children, fullWidth }: RouteTabsProps) { + return ( +
+
{children}
+
+ +
+
+ ) +} + +type TabProps = Pick & ChildrenProp +export const Tab = ({ to, children }: TabProps) => { + return ( + cn('ox-tab', { 'is-selected': isActive })} + > +
{children}
+ + ) +} diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index d948b909b8..94c3cd144e 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -1,47 +1,19 @@ import filesize from 'filesize' -import React, { Suspense, memo, useMemo } from 'react' +import { useMemo } from 'react' import type { LoaderFunctionArgs } from 'react-router-dom' import { useNavigate } from 'react-router-dom' import { apiQueryClient, useApiQuery, useApiQueryClient } from '@oxide/api' -import { Instances24Icon, PageHeader, PageTitle, PropertiesTable, Tab } from '@oxide/ui' +import { Instances24Icon, PageHeader, PageTitle, PropertiesTable } from '@oxide/ui' import { pick } from '@oxide/util' import { MoreActionsMenu } from 'app/components/MoreActionsMenu' +import { RouteTabs, Tab } from 'app/components/RouteTabs' import { InstanceStatusBadge } from 'app/components/StatusBadge' -import { Tabs } from 'app/components/Tabs' import { requireInstanceParams, useQuickActions, useRequiredParams } from 'app/hooks' import { pb } from 'app/util/path-builder' import { useMakeInstanceActions } from '../actions' -import { NetworkingTab } from './tabs/NetworkingTab' -import { SerialConsoleTab } from './tabs/SerialConsoleTab' -import { StorageTab } from './tabs/StorageTab' - -const MetricsTab = React.lazy(() => import('./tabs/MetricsTab')) - -const InstanceTabs = memo(() => ( - - Storage - - - - Metrics - - - - - - Networking - - - - Serial Console - - - - -)) InstancePage.loader = async ({ params }: LoaderFunctionArgs) => { await apiQueryClient.prefetchQuery('instanceView', { @@ -111,7 +83,12 @@ export function InstancePage() { - + + Storage + Metrics + Network Interfaces + Serial Console + ) } diff --git a/app/pages/project/instances/instance/tabs/MetricsTab.tsx b/app/pages/project/instances/instance/tabs/MetricsTab.tsx index e067c3cd3b..6f87ff35cb 100644 --- a/app/pages/project/instances/instance/tabs/MetricsTab.tsx +++ b/app/pages/project/instances/instance/tabs/MetricsTab.tsx @@ -134,7 +134,7 @@ const Loading = () => ( ) -export default function MetricsTab() { +export function MetricsTab() { const instanceParams = useRequiredParams('orgName', 'projectName', 'instanceName') const { data: disks } = useApiQuery('instanceDiskList', { path: instanceParams }) diff --git a/app/routes.tsx b/app/routes.tsx index 770a39d565..1225e8994b 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -38,7 +38,10 @@ import { VpcsPage, } from './pages/project' import { InstanceCreatePage } from './pages/project/instances/InstanceCreatePage' -import { SerialConsolePage } from './pages/project/instances/instance/SerialConsolePage' +import { MetricsTab } from './pages/project/instances/instance/tabs/MetricsTab' +import { NetworkingTab } from './pages/project/instances/instance/tabs/NetworkingTab' +import { SerialConsoleTab } from './pages/project/instances/instance/tabs/SerialConsoleTab' +import { StorageTab } from './pages/project/instances/instance/tabs/StorageTab' import { AppearancePage } from './pages/settings/AppearancePage' import { HotkeysPage } from './pages/settings/HotkeysPage' import { ProfilePage } from './pages/settings/ProfilePage' @@ -165,15 +168,36 @@ export const routes = createRoutesFromElements( element={} handle={{ crumb: 'New instance' }} /> + } + /> } loader={InstancesPage.loader} /> - } loader={InstancePage.loader} /> - } - handle={{ crumb: 'serial-console' }} - /> + } loader={InstancePage.loader}> + } + handle={{ crumb: 'storage' }} + /> + } + handle={{ crumb: 'network-interfaces' }} + /> + } + handle={{ crumb: 'metrics' }} + /> + } + handle={{ crumb: 'serial-console' }} + /> + diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index 185eb1c1db..59be88661f 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -19,6 +19,19 @@ export const pb = { instances: (params: PP.Project) => `${pb.project(params)}/instances`, instanceNew: (params: PP.Project) => `${pb.project(params)}/instances-new`, instance: (params: PP.Instance) => `${pb.instances(params)}/${params.instanceName}`, + + instanceMetrics: (params: PP.Instance) => + `${pb.instances(params)}/${params.instanceName}/metrics`, + instanceStorage: (params: PP.Instance) => + `${pb.instances(params)}/${params.instanceName}/storage`, + + nics: (params: PP.Instance) => + `${pb.instances(params)}/${params.instanceName}/network-interfaces`, + nicNew: (params: PP.Instance) => + `${pb.instances(params)}/${params.instanceName}/network-interfaces-new`, + nicEdit: (params: PP.Instance) => + `${pb.instances(params)}/${params.instanceName}/network-interfaces-edit`, + serialConsole: (params: PP.Instance) => `${pb.instance(params)}/serial-console`, diskNew: (params: PP.Project) => `${pb.project(params)}/disks-new`, diff --git a/libs/ui/lib/tabs/Tabs.css b/libs/ui/lib/tabs/Tabs.css index 3c52902520..379174d4f3 100644 --- a/libs/ui/lib/tabs/Tabs.css +++ b/libs/ui/lib/tabs/Tabs.css @@ -1,34 +1,51 @@ -[data-reach-tabs] { +.ox-tabs.full-width { + @apply !mx-0 !w-full; } -[data-reach-tab-list] { - @apply mb-8 bg-transparent; +.ox-tabs.full-width .ox-tabs-panel > * { + @apply mx-[var(--content-gutter)] w-[calc(100%-var(--content-gutter)*2)]; } -[data-reach-tab-panels] { +.ox-tabs-list { + @apply mb-8 bg-transparent; } -[data-reach-tab] { - @apply h-10 space-x-2 whitespace-nowrap px-[6px] - uppercase text-mono-sm text-tertiary border-secondary; +.ox-tabs-list:after { + @apply block w-full border-b border-secondary; + content: ' '; +} +.ox-tabs.full-width .ox-tabs-list:before { + @apply block w-10 min-w-max flex-shrink-0 border-b border-secondary; + content: ' '; } -[data-reach-tab][data-selected] > div { - @apply hover:bg-secondary; +.ox-tab { + @apply h-10 space-x-2 whitespace-nowrap border-b px-[6px] + uppercase !no-underline text-mono-sm text-tertiary border-secondary; } -[data-reach-tab][data-selected] { +.ox-tab[data-selected], +.ox-tab.is-selected { @apply text-accent border-accent; } -[data-reach-tab] > .ox-badge { +.ox-tab > * { + @apply rounded bg-transparent px-[6px] py-[4px]; +} +.ox-tab:hover > * { + @apply bg-secondary; +} + +.ox-tab > .ox-badge { @apply -mt-1 select-none text-current; } -[data-reach-tab]:not([data-selected]) > .ox-badge { +.ox-tab:not([data-selected]) > .ox-badge, +.ox-tab:not(.is-selected) > .ox-badge { @apply bg-disabled; } -[data-reach-tab][data-selected] > .ox-badge { +.ox-tab[data-selected] > .ox-badge, +.ox-tab.is-selected > .ox-badge { @apply bg-accent-secondary; } diff --git a/libs/ui/lib/tabs/Tabs.tsx b/libs/ui/lib/tabs/Tabs.tsx index cde3f80662..1eff005db7 100644 --- a/libs/ui/lib/tabs/Tabs.tsx +++ b/libs/ui/lib/tabs/Tabs.tsx @@ -41,37 +41,25 @@ export function Tabs({ addProps((i, panelProps) => ({ key: `${id}-panel-${i}`, index: i, - className: cn( - fullWidth && - 'children:mx-[var(--content-gutter)] children:w-[calc(100%-var(--content-gutter)*2)]', - panelProps.className - ), + className: cn('ox-tabs-panel', panelProps.className), })) ) return [tabs, panels] - }, [children, fullWidth, id]) + }, [children, id]) invariant( tabs.length === panels.length, 'Expected there to be exactly one Tab for every Tab.Panel' ) - const after = 'after:block after:w-full after:border-b after:border-secondary' - const before = - 'before:block before:min-w-max before:w-10 before:border-b before:flex-shrink-0 before:border-secondary' - return ( - + {tabs} {panels} @@ -85,10 +73,8 @@ export type TabProps = Assign & { } export function Tab({ className, ...props }: TabProps) { return ( - -
- {props.children} -
+ +
{props.children}
 ) } From be722f8b1e553d046545acf262980df1034267d0 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 27 Oct 2022 23:14:39 -0500 Subject: [PATCH 02/19] Remove unused paths from pb --- app/util/path-builder.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index 59be88661f..cd634f9cf9 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -27,10 +27,6 @@ export const pb = { nics: (params: PP.Instance) => `${pb.instances(params)}/${params.instanceName}/network-interfaces`, - nicNew: (params: PP.Instance) => - `${pb.instances(params)}/${params.instanceName}/network-interfaces-new`, - nicEdit: (params: PP.Instance) => - `${pb.instances(params)}/${params.instanceName}/network-interfaces-edit`, serialConsole: (params: PP.Instance) => `${pb.instance(params)}/serial-console`, From a212debcda00b66181ffb5b48d5c3f4758581ef2 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 27 Oct 2022 23:25:00 -0500 Subject: [PATCH 03/19] Update path builder snapshots --- app/util/path-builder.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index ed38bd4232..98ed57b81c 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -19,8 +19,11 @@ test('path builder', () => { "disks": "/orgs/a/projects/b/disks", "hotkeys": "/settings/hotkeys", "instance": "/orgs/a/projects/b/instances/c", + "instanceMetrics": "/orgs/a/projects/b/instances/c/metrics", "instanceNew": "/orgs/a/projects/b/instances-new", + "instanceStorage": "/orgs/a/projects/b/instances/c/storage", "instances": "/orgs/a/projects/b/instances", + "nics": "/orgs/a/projects/b/instances/c/network-interfaces", "org": "/orgs/a", "orgAccess": "/orgs/a/access", "orgEdit": "/orgs/a/edit", From 35cde4a00a6399ce76726d28c521bfacb1f7f13a Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 27 Oct 2022 23:54:07 -0500 Subject: [PATCH 04/19] Add roles to route tabs; fix tab names --- app/components/RouteTabs.tsx | 5 ++++- app/pages/__tests__/click-everything.e2e.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/components/RouteTabs.tsx b/app/components/RouteTabs.tsx index 0360602623..7116a4bbcc 100644 --- a/app/components/RouteTabs.tsx +++ b/app/components/RouteTabs.tsx @@ -8,7 +8,9 @@ type RouteTabsProps = ChildrenProp & { fullWidth?: boolean } export function RouteTabs({ children, fullWidth }: RouteTabsProps) { return (
-
{children}
+
+ {children} +
@@ -20,6 +22,7 @@ type TabProps = Pick & ChildrenProp export const Tab = ({ to, children }: TabProps) => { return ( cn('ox-tab', { 'is-selected': isActive })} > diff --git a/app/pages/__tests__/click-everything.e2e.ts b/app/pages/__tests__/click-everything.e2e.ts index ac3217b874..2710dbe5d2 100644 --- a/app/pages/__tests__/click-everything.e2e.ts +++ b/app/pages/__tests__/click-everything.e2e.ts @@ -17,7 +17,7 @@ test("Click through everything and make it's all there", async ({ page }) => { 'role=heading[name*=db1]', 'role=tab[name="Storage"]', 'role=tab[name="Metrics"]', - 'role=tab[name="Networking"]', + 'role=tab[name="Network Interfaces"]', 'role=table[name="Boot disk"] >> role=cell[name="disk-1"]', 'role=table[name="Attached disks"] >> role=cell[name="disk-2"]', // buttons disabled while instance is running From 59dc7f4981c5838417372da876301775b6799f6c Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 28 Oct 2022 12:57:09 -0500 Subject: [PATCH 05/19] Fix navigation issues in tests --- app/pages/__tests__/instance/networking.e2e.ts | 2 +- app/routes.tsx | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/pages/__tests__/instance/networking.e2e.ts b/app/pages/__tests__/instance/networking.e2e.ts index b8a77d34f9..3c19433b34 100644 --- a/app/pages/__tests__/instance/networking.e2e.ts +++ b/app/pages/__tests__/instance/networking.e2e.ts @@ -8,7 +8,7 @@ test('Instance networking tab', async ({ page }) => { await page.goto('/orgs/maze-war/projects/mock-project/instances/db1') // Instance networking tab - await page.click('role=tab[name="Networking"]') + await page.click('role=tab[name="Network Interfaces"]') const table = page.locator('table') await expectRowVisible(table, { name: 'my-nic', primary: 'primary' }) diff --git a/app/routes.tsx b/app/routes.tsx index 066c3a2f30..20c8505718 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -171,16 +171,12 @@ export const routes = createRoutesFromElements( element={} handle={{ crumb: 'New instance' }} /> - } - /> + } /> } loader={InstancesPage.loader} /> } loader={InstancePage.loader}> } handle={{ crumb: 'storage' }} From f500bc6a20752a7d404eb761c9cefe20a7d190cb Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 28 Oct 2022 13:09:38 -0500 Subject: [PATCH 06/19] Remove serial console page for now --- .../instances/instance/SerialConsolePage.tsx | 40 ------------------- 1 file changed, 40 deletions(-) delete mode 100644 app/pages/project/instances/instance/SerialConsolePage.tsx diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx deleted file mode 100644 index c392017baf..0000000000 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ /dev/null @@ -1,40 +0,0 @@ -import { Suspense } from 'react' -import React from 'react' - -import { useApiQuery } from '@oxide/api' -import { Button, Divider, PageHeader, PageTitle, Terminal24Icon } from '@oxide/ui' -import { MiB } from '@oxide/util' - -import { PageActions } from 'app/components/PageActions' -import { useRequiredParams } from 'app/hooks' - -const Terminal = React.lazy(() => import('app/components/Terminal')) - -export function SerialConsolePage() { - const instanceParams = useRequiredParams('orgName', 'projectName', 'instanceName') - - const { data, refetch } = useApiQuery( - 'instanceSerialConsole', - { path: instanceParams, query: { maxBytes: 10 * MiB, fromStart: 0 } }, - { refetchOnWindowFocus: false } - ) - - return ( - <> - - }>Serial Console - - - Loading}> - - - -
- -
- - - ) -} From 0220001be5917f9cfd421b758a301c07aaba74ba Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 31 Oct 2022 14:07:32 -0500 Subject: [PATCH 07/19] Lazy load metrics tab --- .../project/instances/instance/tabs/MetricsTab.tsx | 2 +- app/routes.tsx | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/pages/project/instances/instance/tabs/MetricsTab.tsx b/app/pages/project/instances/instance/tabs/MetricsTab.tsx index 6f87ff35cb..e067c3cd3b 100644 --- a/app/pages/project/instances/instance/tabs/MetricsTab.tsx +++ b/app/pages/project/instances/instance/tabs/MetricsTab.tsx @@ -134,7 +134,7 @@ const Loading = () => (
) -export function MetricsTab() { +export default function MetricsTab() { const instanceParams = useRequiredParams('orgName', 'projectName', 'instanceName') const { data: disks } = useApiQuery('instanceDiskList', { path: instanceParams }) diff --git a/app/routes.tsx b/app/routes.tsx index bc3a34474d..18307b8e0d 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, { Suspense } from 'react' import { Navigate, Route, createRoutesFromElements } from 'react-router-dom' import { RouterDataErrorBoundary } from './components/ErrorBoundary' @@ -40,7 +40,6 @@ import { VpcsPage, } from './pages/project' import { InstanceCreatePage } from './pages/project/instances/InstanceCreatePage' -import { MetricsTab } from './pages/project/instances/instance/tabs/MetricsTab' import { NetworkingTab } from './pages/project/instances/instance/tabs/NetworkingTab' import { SerialConsoleTab } from './pages/project/instances/instance/tabs/SerialConsoleTab' import { StorageTab } from './pages/project/instances/instance/tabs/StorageTab' @@ -51,6 +50,10 @@ import { SSHKeysPage } from './pages/settings/SSHKeysPage' import SilosPage from './pages/system/SilosPage' import { pb } from './util/path-builder' +const MetricsTab = React.lazy( + () => import('./pages/project/instances/instance/tabs/MetricsTab') +) + const orgCrumb: CrumbFunc = (m) => m.params.orgName! const projectCrumb: CrumbFunc = (m) => m.params.projectName! const instanceCrumb: CrumbFunc = (m) => m.params.instanceName! @@ -189,7 +192,11 @@ export const routes = createRoutesFromElements( /> } + element={ + + + + } handle={{ crumb: 'metrics' }} /> Date: Mon, 7 Nov 2022 14:02:32 -0500 Subject: [PATCH 08/19] Update props to be more explicit --- app/components/RouteTabs.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/app/components/RouteTabs.tsx b/app/components/RouteTabs.tsx index 7116a4bbcc..15fb4e58ac 100644 --- a/app/components/RouteTabs.tsx +++ b/app/components/RouteTabs.tsx @@ -1,10 +1,11 @@ import cn from 'classnames' -import type { LinkProps } from 'react-router-dom' +import type { ReactNode } from 'react' import { NavLink, Outlet } from 'react-router-dom' -import type { ChildrenProp } from '@oxide/util' - -type RouteTabsProps = ChildrenProp & { fullWidth?: boolean } +export interface RouteTabsProps { + children: ReactNode + fullWidth?: boolean +} export function RouteTabs({ children, fullWidth }: RouteTabsProps) { return (
@@ -18,7 +19,10 @@ export function RouteTabs({ children, fullWidth }: RouteTabsProps) { ) } -type TabProps = Pick & ChildrenProp +export interface TabProps { + to: string + children: ReactNode +} export const Tab = ({ to, children }: TabProps) => { return ( Date: Mon, 7 Nov 2022 14:05:59 -0500 Subject: [PATCH 09/19] Correct instance tabs paths --- app/util/path-builder.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index a4080f0ed4..76f504300e 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -20,13 +20,10 @@ export const pb = { instanceNew: (params: PP.Project) => `${pb.project(params)}/instances-new`, instance: (params: PP.Instance) => `${pb.instances(params)}/${params.instanceName}`, - instanceMetrics: (params: PP.Instance) => - `${pb.instances(params)}/${params.instanceName}/metrics`, - instanceStorage: (params: PP.Instance) => - `${pb.instances(params)}/${params.instanceName}/storage`, + instanceMetrics: (params: PP.Instance) => `${pb.instance(params)}/metrics`, + instanceStorage: (params: PP.Instance) => `${pb.instance(params)}/storage`, - nics: (params: PP.Instance) => - `${pb.instances(params)}/${params.instanceName}/network-interfaces`, + nics: (params: PP.Instance) => `${pb.instance(params)}/network-interfaces`, serialConsole: (params: PP.Instance) => `${pb.instance(params)}/serial-console`, From 8f7c4f56666c44b7ab5c94109d8e0c4fc2e164be Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 7 Nov 2022 17:24:25 -0500 Subject: [PATCH 10/19] Update routetabs to be closer to full aria spec --- app/components/RouteTabs.tsx | 39 ++++++++++++++++++++++++++++----- app/hooks/use-is-active-path.ts | 29 ++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 app/hooks/use-is-active-path.ts diff --git a/app/components/RouteTabs.tsx b/app/components/RouteTabs.tsx index 15fb4e58ac..a8d677f3e2 100644 --- a/app/components/RouteTabs.tsx +++ b/app/components/RouteTabs.tsx @@ -1,6 +1,29 @@ import cn from 'classnames' import type { ReactNode } from 'react' -import { NavLink, Outlet } from 'react-router-dom' +import { Link, Outlet } from 'react-router-dom' + +import { useIsActivePath } from 'app/hooks/use-is-active-path' + +const selectTab = (e: React.KeyboardEvent) => { + if (e.key === 'ArrowLeft') { + e.stopPropagation() + e.preventDefault() + const sibling = (e.target as HTMLDivElement).previousSibling as HTMLDivElement | null + if (sibling) { + sibling.focus() + sibling.click() + } + } + if (e.key === 'ArrowRight') { + e.stopPropagation() + e.preventDefault() + const sibling = (e.target as HTMLDivElement).nextSibling as HTMLDivElement | null + if (sibling) { + sibling.focus() + sibling.click() + } + } +} export interface RouteTabsProps { children: ReactNode @@ -9,10 +32,11 @@ export interface RouteTabsProps { export function RouteTabs({ children, fullWidth }: RouteTabsProps) { return (
-
+ {/* eslint-disable-next-line jsx-a11y/interactive-supports-focus */} +
{children}
-
+
@@ -24,13 +48,16 @@ export interface TabProps { children: ReactNode } export const Tab = ({ to, children }: TabProps) => { + const isActive = useIsActivePath(to) return ( - cn('ox-tab', { 'is-selected': isActive })} + className={cn('ox-tab', { 'is-selected': isActive })} + tabIndex={isActive ? 0 : -1} + aria-selected={isActive} >
{children}
- + ) } diff --git a/app/hooks/use-is-active-path.ts b/app/hooks/use-is-active-path.ts new file mode 100644 index 0000000000..a1463e0259 --- /dev/null +++ b/app/hooks/use-is-active-path.ts @@ -0,0 +1,29 @@ +import { useLocation, useResolvedPath } from 'react-router-dom' + +interface ActivePathOptions { + end?: boolean +} +/** + * Returns true if the provided path is currently active. + * + * This implementation is based on logic from React Router's NavLink component. + * + * @see https://github.com/remix-run/react-router/blob/67f16e73603765158c63a27afb70d3a4b3e823d3/packages/react-router-dom/index.tsx#L448-L467 + * + * @param to The path to check + * @param options.end Ensure this path isn't matched as "active" when its descendant paths are matched. + */ +export const useIsActivePath = (to: string, { end }: ActivePathOptions = {}) => { + const path = useResolvedPath(to) + const location = useLocation() + + const toPathname = path.pathname + const locationPathname = location.pathname + + return ( + locationPathname === toPathname || + (!end && + locationPathname.startsWith(toPathname) && + locationPathname.charAt(toPathname.length) === '/') + ) +} From 4a14b29f2692a9e7c67dc524103dc40c7e37b64c Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 7 Nov 2022 17:29:55 -0500 Subject: [PATCH 11/19] Add comment about describedby --- app/components/RouteTabs.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/app/components/RouteTabs.tsx b/app/components/RouteTabs.tsx index a8d677f3e2..a3743f1fb7 100644 --- a/app/components/RouteTabs.tsx +++ b/app/components/RouteTabs.tsx @@ -36,6 +36,7 @@ export function RouteTabs({ children, fullWidth }: RouteTabsProps) {
{children}
+ {/* TODO: Add aria-describedby for active tab */}
From bd8ae69c064f693c3cc767dddcdb653b66bce720 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 7 Nov 2022 22:10:37 -0500 Subject: [PATCH 12/19] Implement wrap around behavior for arrows --- app/components/RouteTabs.tsx | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/app/components/RouteTabs.tsx b/app/components/RouteTabs.tsx index a3743f1fb7..94d76dad3f 100644 --- a/app/components/RouteTabs.tsx +++ b/app/components/RouteTabs.tsx @@ -5,23 +5,25 @@ import { Link, Outlet } from 'react-router-dom' import { useIsActivePath } from 'app/hooks/use-is-active-path' const selectTab = (e: React.KeyboardEvent) => { + const target = e.target as HTMLDivElement if (e.key === 'ArrowLeft') { e.stopPropagation() e.preventDefault() - const sibling = (e.target as HTMLDivElement).previousSibling as HTMLDivElement | null - if (sibling) { - sibling.focus() - sibling.click() - } - } - if (e.key === 'ArrowRight') { + + const sibling = (target.previousSibling ?? + target.parentElement!.lastChild!) as HTMLDivElement + + sibling.focus() + sibling.click() + } else if (e.key === 'ArrowRight') { e.stopPropagation() e.preventDefault() - const sibling = (e.target as HTMLDivElement).nextSibling as HTMLDivElement | null - if (sibling) { - sibling.focus() - sibling.click() - } + + const sibling = (target.nextSibling ?? + target.parentElement!.firstChild!) as HTMLDivElement + + sibling.focus() + sibling.click() } } From 57854aa3c91cc5da1df721b4702a7c9967f15d69 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 7 Nov 2022 22:45:30 -0500 Subject: [PATCH 13/19] Experimentally solve routing issue --- app/forms/instance-create.tsx | 2 +- app/pages/project/disks/DisksPage.tsx | 2 +- app/pages/project/instances/InstancesPage.tsx | 5 +++-- app/routes.tsx | 5 ++++- app/util/path-builder.ts | 3 +++ 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index c532ea200c..7ea48200e7 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -93,7 +93,7 @@ export function CreateInstanceForm() { title: 'Success!', content: 'Your instance has been created.', }) - navigate(pb.instance({ ...pageParams, instanceName: instance.name })) + navigate(pb.instancePage({ ...pageParams, instanceName: instance.name })) }, }) diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index ea65a1b7a3..f7aa37379a 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -43,7 +43,7 @@ function AttachedInstance({ return instance ? ( {instance.name} diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index c1bcd6f01c..35afd2e23b 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -65,7 +65,8 @@ export function InstancesPage() { { value: 'New instance', onSelect: () => navigate(pb.instanceNew(projectParams)) }, ...(instances?.items || []).map((i) => ({ value: i.name, - onSelect: () => navigate(pb.instance({ ...projectParams, instanceName: i.name })), + onSelect: () => + navigate(pb.instancePage({ ...projectParams, instanceName: i.name })), navGroup: 'Go to instance', })), ], @@ -101,7 +102,7 @@ export function InstancesPage() { - pb.instance({ orgName, projectName, instanceName }) + pb.instancePage({ orgName, projectName, instanceName }) )} /> - } /> + } + /> } loader={InstancesPage.loader} /> diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index 76f504300e..bcc8fef213 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -20,6 +20,9 @@ export const pb = { instanceNew: (params: PP.Project) => `${pb.project(params)}/instances-new`, instance: (params: PP.Instance) => `${pb.instances(params)}/${params.instanceName}`, + /** The landing page for the instance, this path should be used instead of `instance` */ + instancePage: (params: PP.Instance) => pb.instanceStorage(params), + instanceMetrics: (params: PP.Instance) => `${pb.instance(params)}/metrics`, instanceStorage: (params: PP.Instance) => `${pb.instance(params)}/storage`, From c8fd325a5c24c433ba8453067159870ae636f6d2 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 7 Nov 2022 22:54:22 -0500 Subject: [PATCH 14/19] Update route tests --- app/util/path-builder.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index e2b7475ee9..c4735aa77d 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -19,6 +19,7 @@ test('path builder', () => { "instance": "/orgs/a/projects/b/instances/c", "instanceMetrics": "/orgs/a/projects/b/instances/c/metrics", "instanceNew": "/orgs/a/projects/b/instances-new", + "instancePage": "/orgs/a/projects/b/instances/c/storage", "instanceStorage": "/orgs/a/projects/b/instances/c/storage", "instances": "/orgs/a/projects/b/instances", "nics": "/orgs/a/projects/b/instances/c/network-interfaces", From e0c2026943b95e35b4b33e0040563ac9879f85e4 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 7 Nov 2022 22:57:41 -0500 Subject: [PATCH 15/19] fix e2e paths --- app/test/instance-create.e2e.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/test/instance-create.e2e.ts b/app/test/instance-create.e2e.ts index 8c90364dfc..7c87d9ca6f 100644 --- a/app/test/instance-create.e2e.ts +++ b/app/test/instance-create.e2e.ts @@ -1,6 +1,7 @@ import { globalImages } from '@oxide/api-mocks' import { expectVisible, test } from 'app/test/e2e' +import { pb } from 'app/util/path-builder' test.beforeEach(async ({ createProject, orgName, projectName }) => { await createProject(orgName, projectName) @@ -12,7 +13,7 @@ test('can invoke instance create form from instances page', async ({ projectName, genName, }) => { - await page.goto(`/orgs/${orgName}/projects/${projectName}/instances`) + await page.goto(pb.instances({ orgName, projectName })) await page.locator('text="New Instance"').click() await expectVisible(page, [ @@ -41,9 +42,7 @@ test('can invoke instance create form from instances page', async ({ await page.locator('button:has-text("Create instance")').click() - await page.waitForURL( - `/orgs/${orgName}/projects/${projectName}/instances/${instanceName}` - ) + await page.waitForURL(pb.instancePage({ orgName, projectName, instanceName })) await expectVisible(page, [ `h1:has-text("${instanceName}")`, From ac1cb45c6c836b40d6db0018cbb65a36e4d51ec4 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 9 Nov 2022 16:29:22 -0500 Subject: [PATCH 16/19] Move argument into options of useIsActivePath --- app/components/RouteTabs.tsx | 2 +- app/hooks/use-is-active-path.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/components/RouteTabs.tsx b/app/components/RouteTabs.tsx index 94d76dad3f..2fe4760879 100644 --- a/app/components/RouteTabs.tsx +++ b/app/components/RouteTabs.tsx @@ -51,7 +51,7 @@ export interface TabProps { children: ReactNode } export const Tab = ({ to, children }: TabProps) => { - const isActive = useIsActivePath(to) + const isActive = useIsActivePath({ to }) return ( { +export const useIsActivePath = ({ to, end }: ActivePathOptions) => { const path = useResolvedPath(to) const location = useLocation() From 88bd3e90fcf88dde482af1b11f9cbb33d7ca7bde Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 10 Nov 2022 13:10:28 -0500 Subject: [PATCH 17/19] Fix tab layout styles --- libs/ui/lib/tabs/Tabs.css | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/ui/lib/tabs/Tabs.css b/libs/ui/lib/tabs/Tabs.css index 379174d4f3..4f65d3836e 100644 --- a/libs/ui/lib/tabs/Tabs.css +++ b/libs/ui/lib/tabs/Tabs.css @@ -20,7 +20,7 @@ } .ox-tab { - @apply h-10 space-x-2 whitespace-nowrap border-b px-[6px] + @apply h-10 space-x-2 whitespace-nowrap border-b px-1.5 pb-1 pt-2 uppercase !no-underline text-mono-sm text-tertiary border-secondary; } @@ -30,7 +30,7 @@ } .ox-tab > * { - @apply rounded bg-transparent px-[6px] py-[4px]; + @apply rounded bg-transparent px-1.5 py-1; } .ox-tab:hover > * { @apply bg-secondary; From a0743a3f2f2b252318c6e32a0d9fa42048ed0060 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 10 Nov 2022 14:30:27 -0500 Subject: [PATCH 18/19] Fix tab panel focus ring --- app/pages/project/instances/instance/tabs/MetricsTab.tsx | 2 +- app/pages/project/instances/instance/tabs/StorageTab.tsx | 4 ++-- libs/ui/lib/tabs/Tabs.css | 9 ++++++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/pages/project/instances/instance/tabs/MetricsTab.tsx b/app/pages/project/instances/instance/tabs/MetricsTab.tsx index e71adb8b2b..feb3753858 100644 --- a/app/pages/project/instances/instance/tabs/MetricsTab.tsx +++ b/app/pages/project/instances/instance/tabs/MetricsTab.tsx @@ -80,7 +80,7 @@ function DiskMetrics({ disks }: { disks: Disk[] }) { return ( <> -
+
+ <>

Boot disk

@@ -175,6 +175,6 @@ export function StorageTab() { {showDiskAttach && ( setShowDiskAttach(false)} /> )} -
+ ) } diff --git a/libs/ui/lib/tabs/Tabs.css b/libs/ui/lib/tabs/Tabs.css index 4f65d3836e..b4a33e5f75 100644 --- a/libs/ui/lib/tabs/Tabs.css +++ b/libs/ui/lib/tabs/Tabs.css @@ -2,8 +2,11 @@ @apply !mx-0 !w-full; } +.ox-tabs.full-width .ox-tabs-panel { + @apply mx-[var(--content-gutter)]; +} .ox-tabs.full-width .ox-tabs-panel > * { - @apply mx-[var(--content-gutter)] w-[calc(100%-var(--content-gutter)*2)]; + @apply w-[calc(100%-var(--content-gutter)*2)]; } .ox-tabs-list { @@ -19,6 +22,10 @@ content: ' '; } +.ox-tabs-panel:focus-visible { + @apply outline outline-2 outline-offset-[1rem] outline-accent-secondary; +} + .ox-tab { @apply h-10 space-x-2 whitespace-nowrap border-b px-1.5 pb-1 pt-2 uppercase !no-underline text-mono-sm text-tertiary border-secondary; From 782deade043409b8850425c26ef32df9fe68b65b Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 10 Nov 2022 14:50:57 -0500 Subject: [PATCH 19/19] Improve instancePage comment --- app/util/path-builder.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index bcc8fef213..1c49939b91 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -20,7 +20,13 @@ export const pb = { instanceNew: (params: PP.Project) => `${pb.project(params)}/instances-new`, instance: (params: PP.Instance) => `${pb.instances(params)}/${params.instanceName}`, - /** The landing page for the instance, this path should be used instead of `instance` */ + /** + * This route exists as a direct link to the default tab of the instance page. Unfortunately + * we don't currently have a good mechanism at the moment to handle a redirect to the default + * tab in a seemless way so we need all in-app links to go directly to the default tab. + * + * @see https://github.com/oxidecomputer/console/pull/1267#discussion_r1016766205 + */ instancePage: (params: PP.Instance) => pb.instanceStorage(params), instanceMetrics: (params: PP.Instance) => `${pb.instance(params)}/metrics`,