[ui] IA flagged changes to navigation (#20971)

## Summary & Motivation Apply changes to the top nav for our info architecture exploration, along with some stylistic changes to the top right side of the nav to make it more consistent. Non-flagged: <img width="1136" alt="Screenshot 2024-04-02 at 13 12 32" src="https://github.com/dagster-io/dagster/assets/2823852/e9f72c37-d212-4f3a-8a94-260f5adaf3c2"> With settings flag: <img width="1136" alt="Screenshot 2024-04-02 at 13 12 18" src="https://github.com/dagster-io/dagster/assets/2823852/59ab39bf-2b5d-4f14-ad6e-5e5f3aacf6d2"> ## How I Tested These Changes Load app, verify top nav rendering and behavior. Enable new Settings flag, verify same.
dagster-io · Apr 3, 2024 · 9d6f360 · 9d6f360 · github-actions · Apr 3, 2024
1 parent 87dfd5e
commit 9d6f360
Show file tree

Hide file tree

Showing 12 changed files with 157 additions and 188 deletions.
diff --git a/js_modules/dagster-ui/packages/app-oss/src/App.tsx b/js_modules/dagster-ui/packages/app-oss/src/App.tsx
@@ -42,7 +42,7 @@ export default function AppPage() {
     <InjectedComponents>
       <LiveDataPollRateContext.Provider value={liveDataPollRate ?? 2000}>
         <AppProvider appCache={appCache} config={config}>
-          <AppTopNav searchPlaceholder="Search…" allowGlobalReload>
+          <AppTopNav allowGlobalReload>
             <HelpMenu showContactSales={false} />
             <UserSettingsButton />
           </AppTopNav>

diff --git a/js_modules/dagster-ui/packages/ui-components/src/components/Icon.tsx b/js_modules/dagster-ui/packages/ui-components/src/components/Icon.tsx
@@ -90,6 +90,7 @@ import graph_horizontal from '../icon-svgs/graph_horizontal.svg';
 import graph_neighbors from '../icon-svgs/graph_neighbors.svg';
 import graph_upstream from '../icon-svgs/graph_upstream.svg';
 import graph_vertical from '../icon-svgs/graph_vertical.svg';
+import help_circle from '../icon-svgs/help_circle.svg';
 import history from '../icon-svgs/history.svg';
 import history_toggle_off from '../icon-svgs/history_toggle_off.svg';
 import hourglass from '../icon-svgs/hourglass.svg';
@@ -225,6 +226,7 @@ export const Icons = {
 
   // Other custom icons
   toggle_whitespace,
+  help_circle,
   panel_show_top,
   panel_show_left,
   panel_show_right,

diff --git a/js_modules/dagster-ui/packages/ui-components/src/icon-svgs/help_circle.svg b/js_modules/dagster-ui/packages/ui-components/src/icon-svgs/help_circle.svg
diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/App.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/App.tsx
@@ -1,6 +1,7 @@
 import * as React from 'react';
 import styled from 'styled-components';
 
+import {useFeatureFlags} from './Flags';
 import {LayoutContext} from './LayoutProvider';
 import {LEFT_NAV_WIDTH, LeftNav} from '../nav/LeftNav';
 
@@ -11,6 +12,7 @@ interface Props {
 
 export const App = ({banner, children}: Props) => {
   const {nav} = React.useContext(LayoutContext);
+  const {flagSettingsPage} = useFeatureFlags();
 
   const onClickMain = React.useCallback(() => {
     if (nav.isSmallScreen) {
@@ -20,7 +22,7 @@ export const App = ({banner, children}: Props) => {
 
   return (
     <Container>
-      <LeftNav />
+      {flagSettingsPage ? null : <LeftNav />}
       <Main $smallScreen={nav.isSmallScreen} $navOpen={nav.isOpen} onClick={onClickMain}>
         <div>{banner}</div>
         <ChildContainer>{children}</ChildContainer>

diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNav.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNav.tsx
@@ -10,18 +10,18 @@ import {
   useRepositoryLocationReload,
 } from '../../nav/useRepositoryLocationReload';
 import {SearchDialog} from '../../search/SearchDialog';
+import {useFeatureFlags} from '../Flags';
 import {LayoutContext} from '../LayoutProvider';
 import {ShortcutHandler} from '../ShortcutHandler';
 import {WebSocketStatus} from '../WebSocketProvider';
 
 interface Props {
   children?: React.ReactNode;
-  searchPlaceholder: string;
   showStatusWarningIcon?: boolean;
   allowGlobalReload?: boolean;
 }
 
-export const AppTopNav = ({children, searchPlaceholder, allowGlobalReload = false}: Props) => {
+export const AppTopNav = ({children, allowGlobalReload = false}: Props) => {
   const {reloading, tryReload} = useRepositoryLocationReload({
     scope: 'workspace',
     reloadFn: reloadFnForWorkspace,
@@ -33,7 +33,7 @@ export const AppTopNav = ({children, searchPlaceholder, allowGlobalReload = fals
         <AppTopNavLogo />
         <AppTopNavRightOfLogo />
       </Box>
-      <Box flex={{direction: 'row', alignItems: 'center'}}>
+      <Box flex={{direction: 'row', alignItems: 'center', gap: 12}} margin={{right: 20}}>
         {allowGlobalReload ? (
           <ShortcutHandler
             onShortcut={() => {
@@ -48,7 +48,7 @@ export const AppTopNav = ({children, searchPlaceholder, allowGlobalReload = fals
             <div style={{width: '0px', height: '30px'}} />
           </ShortcutHandler>
         ) : null}
-        <SearchDialog searchPlaceholder={searchPlaceholder} />
+        <SearchDialog />
         {children}
       </Box>
     </AppTopNavContainer>
@@ -57,6 +57,7 @@ export const AppTopNav = ({children, searchPlaceholder, allowGlobalReload = fals
 
 export const AppTopNavLogo = () => {
   const {nav} = React.useContext(LayoutContext);
+  const {flagSettingsPage} = useFeatureFlags();
   const navButton = React.useRef<null | HTMLButtonElement>(null);
 
   const onToggle = React.useCallback(() => {
@@ -75,7 +76,7 @@ export const AppTopNavLogo = () => {
 
   return (
     <LogoContainer>
-      {nav.canOpen ? (
+      {!flagSettingsPage && nav.canOpen ? (
         <ShortcutHandler
           onShortcut={() => onToggle()}
           shortcutLabel="."

diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavLinks.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavLinks.tsx
@@ -36,71 +36,90 @@ export const AppTopNavLinks = ({links}: {links: AppNavLinkType[]}) => {
 };
 
 export const navLinks = () => {
-  return [
-    {
-      key: 'overview',
-      path: '/overview',
-      element: (
-        <TopNavLink to="/overview" data-cy="AppTopNav_StatusLink">
-          Overview
-        </TopNavLink>
-      ),
-    },
-    {
-      key: 'runs',
-      path: '/runs',
-      element: (
-        <TopNavLink to="/runs" data-cy="AppTopNav_RunsLink">
-          Runs
-        </TopNavLink>
-      ),
-    },
-    {
-      key: 'assets',
-      path: '/assets',
-      element: (
-        <TopNavLink
-          to="/assets"
-          data-cy="AppTopNav_AssetsLink"
-          isActive={assetsPathMatcher}
-          exact={false}
-        >
-          Assets
-        </TopNavLink>
-      ),
-    },
-    featureEnabled(FeatureFlag.flagSettingsPage)
-      ? {
-          key: 'settings',
-          path: '/settings',
-          element: (
-            <TopNavLink
-              to="/settings"
-              data-cy="AppTopNav_SettingsLink"
-              isActive={settingsPathMatcher}
-            >
-              <Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
-                Settings
-                <DeploymentStatusIcon />
-              </Box>
-            </TopNavLink>
-          ),
-        }
-      : {
-          key: 'deployment',
-          path: '/locations',
-          element: (
-            <TopNavLink
-              to="/locations"
-              data-cy="AppTopNav_StatusLink"
-              isActive={locationPathMatcher}
-            >
-              <Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
-                Deployment
-                <DeploymentStatusIcon />
-              </Box>
-            </TopNavLink>
-          ),
-        },
-  ];
+  const overview = {
+    key: 'overview',
+    path: '/overview',
+    element: (
+      <TopNavLink to="/overview" data-cy="AppTopNav_StatusLink">
+        Overview
+      </TopNavLink>
+    ),
+  };
+
+  const runs = {
+    key: 'runs',
+    path: '/runs',
+    element: (
+      <TopNavLink to="/runs" data-cy="AppTopNav_RunsLink">
+        Runs
+      </TopNavLink>
+    ),
+  };
+
+  const jobs = {
+    key: 'jobs',
+    path: '/jobs',
+    element: (
+      <TopNavLink to="/jobs" data-cy="AppTopNav_JobsLink">
+        Jobs
+      </TopNavLink>
+    ),
+  };
+
+  const assets = {
+    key: 'assets',
+    path: '/assets',
+    element: (
+      <TopNavLink
+        to="/assets"
+        data-cy="AppTopNav_AssetsLink"
+        isActive={assetsPathMatcher}
+        exact={false}
+      >
+        Assets
+      </TopNavLink>
+    ),
+  };
+
+  const automation = {
+    key: 'automation',
+    path: '/automation',
+    element: (
+      <TopNavLink to="/automation" data-cy="AppTopNav_AutomationLink">
+        Automation
+      </TopNavLink>
+    ),
+  };
+
+  const settings = {
+    key: 'settings',
+    path: '/settings',
+    element: (
+      <TopNavLink to="/settings" data-cy="AppTopNav_SettingsLink" isActive={settingsPathMatcher}>
+        <Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
+          Settings
+          <DeploymentStatusIcon />
+        </Box>
+      </TopNavLink>
+    ),
+  };
+
+  const deployment = {
+    key: 'deployment',
+    path: '/locations',
+    element: (
+      <TopNavLink to="/locations" data-cy="AppTopNav_StatusLink" isActive={locationPathMatcher}>
+        <Box flex={{direction: 'row', alignItems: 'center', gap: 6}}>
+          Deployment
+          <DeploymentStatusIcon />
+        </Box>
+      </TopNavLink>
+    ),
+  };
+
+  if (featureEnabled(FeatureFlag.flagSettingsPage)) {
+    return [overview, assets, jobs, automation, runs, settings];
+  }
+
+  return [overview, runs, assets, deployment];
 };
diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/HelpMenu.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/HelpMenu.tsx
@@ -1,8 +1,6 @@
 import {
-  Colors,
   ExternalAnchorButton,
   Icon,
-  IconWrapper,
   Menu,
   MenuDivider,
   MenuExternalLink,
@@ -12,9 +10,9 @@ import {
   Tooltip,
 } from '@dagster-io/ui-components';
 import {useCallback, useState} from 'react';
-import styled from 'styled-components';
 
 import {ShortcutHandler} from './ShortcutHandler';
+import {TopNavButton} from './TopNavButton';
 import DagsterUniversityImage from './dagster_university.svg';
 import {useStateWithStorage} from '../hooks/useStateWithStorage';
 
@@ -102,33 +100,13 @@ export const HelpMenu = ({showContactSales = true}: {showContactSales?: boolean}
             </Menu>
           }
         >
-          <Tooltip content="Help">
-            <HelpButton>
-              <Icon name="chat_support" size={20} />
-            </HelpButton>
+          <Tooltip content="Help" placement="bottom" canShow={!isOpen}>
+            <TopNavButton>
+              <Icon name="help_circle" size={20} />
+            </TopNavButton>
           </Tooltip>
         </Popover>
       </ProductTour>
     </ShortcutHandler>
   );
 };
-
-const HelpButton = styled.button`
-  background: transparent;
-  border: none;
-  cursor: pointer;
-  padding: 8px;
-
-  :focus {
-    outline: none;
-  }
-
-  ${IconWrapper} {
-    background-color: ${Colors.navTextSelected()};
-    transition: background-color 100ms linear;
-  }
-
-  :focus ${IconWrapper}, :hover ${IconWrapper} {
-    background-color: ${Colors.navTextHover()};
-  }
-`;
diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/TopNavButton.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/TopNavButton.tsx
@@ -0,0 +1,30 @@
+import {Colors, IconWrapper, UnstyledButton} from '@dagster-io/ui-components';
+import styled from 'styled-components';
+
+export const TopNavButton = styled(UnstyledButton)`
+  background-color: ${Colors.navButton()};
+  height: 32px;
+  width: 32px;
+  border-radius: 50%;
+  display: inline-flex;
+  align-items: center;
+  justify-content: center;
+
+  :hover,
+  :focus {
+    background-color: ${Colors.navButtonHover()};
+  }
+
+  :focus:not(:active) {
+    outline: ${Colors.focusRing()} auto 1px;
+  }
+
+  ${IconWrapper} {
+    background-color: ${Colors.accentWhite()};
+    transition: background-color 100ms linear;
+  }
+
+  :focus ${IconWrapper}, :hover ${IconWrapper} {
+    background-color: ${Colors.navTextHover()};
+  }
+`;