Skip to content

Commit

Permalink
[ui] Top nav: iterate to build shortcut handling (#20969)
Browse files Browse the repository at this point in the history 
## Summary & Motivation

Streamline rendering of shortcut wrappers for top nav items.

Thus far we've just hardcoded the shortcut digit, but as our nav grows
increasingly complex with gated features, Cloud differences, it gets
harder to keep the digits in order.

<img width="1212" alt="Screenshot 2024-04-02 at 12 24 42"
src="https://github.com/dagster-io/dagster/assets/2823852/fd04d021-0c34-4e45-9572-14c4fdbb2cad">

<img width="1214" alt="Screenshot 2024-04-02 at 12 24 07"
src="https://github.com/dagster-io/dagster/assets/2823852/79186b1d-fc02-4d37-a831-cd681762745c">


## How I Tested These Changes

View OSS and Cloud. Verify that the top nav items are in the correct
order, with incremental shortcut digits correctly applied.
  • Loading branch information
@hellendag
hellendag authored Apr 2, 2024
1 parent d726dd9 commit d03bc75
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 79 deletions.
136 changes: 60 additions & 76 deletions js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavLinks.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Box} from '@dagster-io/ui-components';
import React from 'react';
import {ReactNode} from 'react';
import {useHistory} from 'react-router-dom';

import {TopNavLink} from './AppTopNav';
Expand All @@ -9,113 +9,97 @@ import {FeatureFlag, featureEnabled} from '../Flags';
import {ShortcutHandler} from '../ShortcutHandler';

export type AppNavLinkType = {
title: string;
element: React.ReactNode;
key: string;
path: string;
element: ReactNode;
};

export const AppTopNavLinks = ({links}: {links: AppNavLinkType[]}) => {
const history = useHistory();
return (
<Box margin={{left: 8}} flex={{direction: 'row', alignItems: 'center', gap: 16}}>
{links.map((link) => link.element)}
{links.map((link, ii) => {
const {key, path, element} = link;
return (
<ShortcutHandler
key={key}
onShortcut={() => history.push(path)}
shortcutLabel={`⌥${ii + 1}`}
shortcutFilter={(e) => e.altKey && e.code === `Digit${ii + 1}`}
>
{element}
</ShortcutHandler>
);
})}
</Box>
);
};

export const navLinks = (history: ReturnType<typeof useHistory>) => {
export const navLinks = () => {
return [
{
title: 'overview' as const,
key: 'overview',
path: '/overview',
element: (
<ShortcutHandler
key="overview"
onShortcut={() => history.push('/overview')}
shortcutLabel="⌥1"
shortcutFilter={(e) => e.altKey && e.code === 'Digit1'}
>
<TopNavLink to="/overview" data-cy="AppTopNav_StatusLink">
Overview
</TopNavLink>
</ShortcutHandler>
<TopNavLink to="/overview" data-cy="AppTopNav_StatusLink">
Overview
</TopNavLink>
),
},
{
title: 'runs' as const,
key: 'runs',
path: '/runs',
element: (
<ShortcutHandler
key="runs"
onShortcut={() => history.push('/runs')}
shortcutLabel="⌥2"
shortcutFilter={(e) => e.altKey && e.code === 'Digit2'}
>
<TopNavLink to="/runs" data-cy="AppTopNav_RunsLink">
Runs
</TopNavLink>
</ShortcutHandler>
<TopNavLink to="/runs" data-cy="AppTopNav_RunsLink">
Runs
</TopNavLink>
),
},
{
title: 'assets' as const,
key: 'assets',
path: '/assets',
element: (
<ShortcutHandler
key="assets"
onShortcut={() => history.push('/assets')}
shortcutLabel="⌥3"
shortcutFilter={(e) => e.altKey && e.code === 'Digit3'}
<TopNavLink
to="/assets"
data-cy="AppTopNav_AssetsLink"
isActive={assetsPathMatcher}
exact={false}
>
<TopNavLink
to="/assets"
data-cy="AppTopNav_AssetsLink"
isActive={assetsPathMatcher}
exact={false}
>
Assets
</TopNavLink>
</ShortcutHandler>
Assets
</TopNavLink>
),
},
featureEnabled(FeatureFlag.flagSettingsPage)
? {
title: 'settings' as const,
key: 'settings',
path: '/settings',
element: (
<ShortcutHandler
key="settings"
onShortcut={() => history.push('/settings')}
shortcutLabel="⌥4"
shortcutFilter={(e) => e.altKey && e.code === 'Digit4'}
<TopNavLink
to="/settings"
data-cy="AppTopNav_SettingsLink"
isActive={settingsPathMatcher}
>
<TopNavLink
to="/settings"
data-cy="AppTopNav_SettingsLink"
isActive={settingsPathMatcher}
>
<Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
Settings
<DeploymentStatusIcon />
</Box>
</TopNavLink>
</ShortcutHandler>
<Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
Settings
<DeploymentStatusIcon />
</Box>
</TopNavLink>
),
}
: {
title: 'deployment' as const,
key: 'deployment',
path: '/locations',
element: (
<ShortcutHandler
key="deployment"
onShortcut={() => history.push('/locations')}
shortcutLabel="⌥4"
shortcutFilter={(e) => e.altKey && e.code === 'Digit4'}
<TopNavLink
to="/locations"
data-cy="AppTopNav_StatusLink"
isActive={locationPathMatcher}
>
<TopNavLink
to="/locations"
data-cy="AppTopNav_StatusLink"
isActive={locationPathMatcher}
>
<Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
Deployment
<DeploymentStatusIcon />
</Box>
</TopNavLink>
</ShortcutHandler>
<Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
Deployment
<DeploymentStatusIcon />
</Box>
</TopNavLink>
),
},
];
Expand Down
4 changes: 1 addition & 3 deletions js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavRightOfLogo.oss.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import {memo} from 'react';
import {useHistory} from 'react-router-dom';

import {AppTopNavLinks, navLinks} from './AppTopNavLinks';

export const AppTopNavRightOfLogo = memo(() => {
const history = useHistory();
return <AppTopNavLinks links={navLinks(history)} />;
return <AppTopNavLinks links={navLinks()} />;
});

1 comment on commit d03bc75

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-k68zk66w5-elementl.vercel.app

Built with commit d03bc75.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.