From 9d6f360705c9d6c193c72c5229cb8dafc32981cb Mon Sep 17 00:00:00 2001 From: Isaac Hellendag <2823852+hellendag@users.noreply.github.com> Date: Wed, 3 Apr 2024 09:51:25 -0500 Subject: [PATCH] [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: Screenshot 2024-04-02 at 13 12 32 With settings flag: Screenshot 2024-04-02 at 13 12 18 ## How I Tested These Changes Load app, verify top nav rendering and behavior. Enable new Settings flag, verify same. --- .../dagster-ui/packages/app-oss/src/App.tsx | 2 +- .../ui-components/src/components/Icon.tsx | 2 + .../src/icon-svgs/help_circle.svg | 3 + .../packages/ui-core/src/app/App.tsx | 4 +- .../ui-core/src/app/AppTopNav/AppTopNav.tsx | 11 +- .../src/app/AppTopNav/AppTopNavLinks.tsx | 153 ++++++++++-------- .../packages/ui-core/src/app/HelpMenu.tsx | 32 +--- .../packages/ui-core/src/app/TopNavButton.tsx | 30 ++++ .../ui-core/src/app/UserSettingsButton.tsx | 42 ++--- .../src/app/__tests__/AppTopNav.test.tsx | 2 +- .../ui-core/src/search/SearchDialog.tsx | 58 +------ .../__stories__/SearchDialog.stories.tsx | 6 +- 12 files changed, 157 insertions(+), 188 deletions(-) create mode 100644 js_modules/dagster-ui/packages/ui-components/src/icon-svgs/help_circle.svg create mode 100644 js_modules/dagster-ui/packages/ui-core/src/app/TopNavButton.tsx diff --git a/js_modules/dagster-ui/packages/app-oss/src/App.tsx b/js_modules/dagster-ui/packages/app-oss/src/App.tsx index bbc400656401b..857ee4852bc53 100644 --- 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() { - + 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 index 2a5cf05377200..3108c020abe92 100644 --- 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 new file mode 100644 index 0000000000000..1728929a680e0 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-components/src/icon-svgs/help_circle.svg @@ -0,0 +1,3 @@ + + + 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 index 6f5f6ed8a48c2..6e936a52f5fb3 100644 --- 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 ( - + {flagSettingsPage ? null : }
{banner}
{children} 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 index 61fe74d4b4d5d..193b2ded6a33f 100644 --- 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 - + {allowGlobalReload ? ( { @@ -48,7 +48,7 @@ export const AppTopNav = ({children, searchPlaceholder, allowGlobalReload = fals
) : null} - + {children} @@ -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); const onToggle = React.useCallback(() => { @@ -75,7 +76,7 @@ export const AppTopNavLogo = () => { return ( - {nav.canOpen ? ( + {!flagSettingsPage && nav.canOpen ? ( 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 index a3eab87214df0..c11ae872b969d 100644 --- 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: ( - - Overview - - ), - }, - { - key: 'runs', - path: '/runs', - element: ( - - Runs - - ), - }, - { - key: 'assets', - path: '/assets', - element: ( - - Assets - - ), - }, - featureEnabled(FeatureFlag.flagSettingsPage) - ? { - key: 'settings', - path: '/settings', - element: ( - - - Settings - - - - ), - } - : { - key: 'deployment', - path: '/locations', - element: ( - - - Deployment - - - - ), - }, - ]; + const overview = { + key: 'overview', + path: '/overview', + element: ( + + Overview + + ), + }; + + const runs = { + key: 'runs', + path: '/runs', + element: ( + + Runs + + ), + }; + + const jobs = { + key: 'jobs', + path: '/jobs', + element: ( + + Jobs + + ), + }; + + const assets = { + key: 'assets', + path: '/assets', + element: ( + + Assets + + ), + }; + + const automation = { + key: 'automation', + path: '/automation', + element: ( + + Automation + + ), + }; + + const settings = { + key: 'settings', + path: '/settings', + element: ( + + + Settings + + + + ), + }; + + const deployment = { + key: 'deployment', + path: '/locations', + element: ( + + + Deployment + + + + ), + }; + + 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 index 9705d7053f644..0b97e07e8e793 100644 --- 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} } > - - - - + + + + ); }; - -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 new file mode 100644 index 0000000000000..604efb416d02c --- /dev/null +++ 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()}; + } +`; diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/UserSettingsButton.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/UserSettingsButton.tsx index 71680ba2372c8..9981aa1477477 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/app/UserSettingsButton.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/app/UserSettingsButton.tsx @@ -1,44 +1,24 @@ -import {Colors, Icon, IconWrapper} from '@dagster-io/ui-components'; +import {Icon} from '@dagster-io/ui-components'; import {useState} from 'react'; -import styled from 'styled-components'; +import {useFeatureFlags} from './Flags'; +import {TopNavButton} from './TopNavButton'; import {UserSettingsDialog} from './UserSettingsDialog/UserSettingsDialog'; import {getVisibleFeatureFlagRows} from './getVisibleFeatureFlagRows'; -const SettingsButton = styled.button` - background: transparent; - border: 0; - cursor: pointer; - padding: 24px; - - ${IconWrapper} { - transition: background 50ms linear; - } - - &:hover ${IconWrapper} { - background: ${Colors.navTextHover()}; - } - - &:active ${IconWrapper} { - background: ${Colors.navTextHover()}; - } - - &:focus { - outline: none; +export const UserSettingsButton = () => { + const {flagSettingsPage} = useFeatureFlags(); + const [isOpen, setIsOpen] = useState(false); - ${IconWrapper} { - background: ${Colors.navTextHover()}; - } + if (flagSettingsPage) { + return null; } -`; -export const UserSettingsButton = () => { - const [isOpen, setIsOpen] = useState(false); return ( <> - setIsOpen(true)} title="User settings"> - - + setIsOpen(true)} title="User settings"> + + setIsOpen(false)} diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/AppTopNav.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/AppTopNav.test.tsx index a4412faca6e7d..4082d54388fd0 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/AppTopNav.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/AppTopNav.test.tsx @@ -20,7 +20,7 @@ describe('AppTopNav', () => { > - + , diff --git a/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx b/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx index cc019efd902bf..75bad57bb0921 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx @@ -1,6 +1,6 @@ // eslint-disable-next-line no-restricted-imports import {Overlay} from '@blueprintjs/core'; -import {Box, Colors, FontFamily, Icon, Spinner} from '@dagster-io/ui-components'; +import {Colors, FontFamily, Icon, Spinner} from '@dagster-io/ui-components'; import Fuse from 'fuse.js'; import debounce from 'lodash/debounce'; import * as React from 'react'; @@ -12,6 +12,7 @@ import {SearchResult} from './types'; import {useGlobalSearch} from './useGlobalSearch'; import {__updateSearchVisibility} from './useSearchVisibility'; import {ShortcutHandler} from '../app/ShortcutHandler'; +import {TopNavButton} from '../app/TopNavButton'; import {useTrackEvent} from '../app/analytics'; import {Trace, createTrace} from '../performance'; @@ -72,7 +73,7 @@ const initialState: State = { const DEBOUNCE_MSEC = 100; -export const SearchDialog = ({searchPlaceholder}: {searchPlaceholder: string}) => { +export const SearchDialog = () => { const history = useHistory(); const {initialize, loading, searchPrimary, searchSecondary} = useGlobalSearch({ includeAssetFilters: false, @@ -211,25 +212,9 @@ export const SearchDialog = ({searchPlaceholder}: {searchPlaceholder: string}) = return ( <> - - - -
- -
-
{searchPlaceholder}
- - / - - + + + ( - + ); @@ -27,7 +27,7 @@ export const BasicSearch = () => ( export const SlowSecondaryQuerySearch = () => ( - + ); @@ -35,7 +35,7 @@ export const SlowSecondaryQuerySearch = () => ( export const LotsOfAssetsSearch = () => ( - + );