Skip to content

Commit

Permalink
[dagit] Make the left nav stay open/closed (#8173)
Browse files Browse the repository at this point in the history
### Summary & Motivation

Modify the logic and behavior for rendering the left nav as open or closed.

- Remove the flag and any route-based logic for hiding the nav.
- If the user is using a small viewport:
  - The hamburger button will open and close the nav. Same as before.
  - When navigating, the nav will close.
- If the user is using a large viewport:
  - The hamburger button is now present, and shows/hides the nav. The main contents are rendered next to the nav, not beneath it.
  - There is no animation for opening/closing the nav.
  - The nav will not hide when navigating.
  - Open/closed state is tracked in localStorage. This way, the user's preference is tracked across sessions.
  - If the user switches to a small viewport, the large viewport preference is saved, but behavior will go back to small viewport.
  - If the user switches back to a large viewport, the large viewport preference is used.

### How I Tested These Changes

Test the situations described above. Verify that the nav behaves as expected, and writes to localStorage properly.
  • Loading branch information
hellendag committed Jun 3, 2022
1 parent cca4561 commit c8e5fa4
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 48 deletions.
30 changes: 18 additions & 12 deletions js_modules/dagit/packages/core/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,38 @@ export const App: React.FC = (props) => {
const {nav} = React.useContext(LayoutContext);

const onClickMain = React.useCallback(() => {
nav.close();
if (nav.isSmallScreen) {
nav.close();
}
}, [nav]);

return (
<Container>
<LeftNav />
<Main $navCollapsible={nav.isCollapsible} onClick={onClickMain}>
<Main $smallScreen={nav.isSmallScreen} $navOpen={nav.isOpen} onClick={onClickMain}>
{props.children}
</Main>
</Container>
);
};

const Main = styled.div<{$navCollapsible: boolean}>`
const Main = styled.div<{$smallScreen: boolean; $navOpen: boolean}>`
height: 100%;
z-index: 1;
${(p) =>
p.$navCollapsible
? `
margin-left: 0;
width: 100%;`
: `
margin-left: ${LEFT_NAV_WIDTH}px;
width: calc(100% - ${LEFT_NAV_WIDTH}px);
`}
${({$navOpen, $smallScreen}) => {
if ($smallScreen || !$navOpen) {
return `
margin-left: 0;
width: 100%;
`;
}
return `
margin-left: ${LEFT_NAV_WIDTH}px;
width: calc(100% - ${LEFT_NAV_WIDTH}px);
`;
}}
`;

const Container = styled.div`
Expand Down
11 changes: 3 additions & 8 deletions js_modules/dagit/packages/core/src/app/AppTopNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,7 @@ export const AppTopNavLogo: React.FC = () => {
shortcutLabel="."
shortcutFilter={(e) => e.key === '.'}
>
<NavButton
onClick={onToggle}
onKeyDown={onKeyDown}
ref={navButton}
$visible={nav.isCollapsible}
>
<NavButton onClick={onToggle} onKeyDown={onKeyDown} ref={navButton}>
<Icon name="menu" color={Colors.White} size={24} />
</NavButton>
</ShortcutHandler>
Expand Down Expand Up @@ -214,15 +209,15 @@ const LogoContainer = styled.div`
}
`;

const NavButton = styled.button<{$visible: boolean}>`
const NavButton = styled.button`
border-radius: 20px;
cursor: pointer;
margin-left: 4px;
outline: none;
padding: 6px;
border: none;
background: transparent;
display: ${(p) => (p.$visible ? `block` : 'none')};
display: block;
${IconWrapper} {
transition: background 100ms linear;
Expand Down
1 change: 0 additions & 1 deletion js_modules/dagit/packages/core/src/app/Flags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export const DAGIT_FLAGS_KEY = 'DAGIT_FLAGS';

export enum FeatureFlag {
flagDebugConsoleLogging = 'flagDebugConsoleLogging',
flagAlwaysCollapseNavigation = 'flagAlwaysCollapseNavigation',
flagDisableWebsockets = 'flagDisableWebsockets',
flagNewPartitionsView = 'flagNewPartitionsView',
flagFlatLeftNav = 'flagFlatLeftNav',
Expand Down
51 changes: 38 additions & 13 deletions js_modules/dagit/packages/core/src/app/LayoutProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import {useLocation} from 'react-router-dom';

import {useFeatureFlags} from './Flags';
import {useStateWithStorage} from '../hooks/useStateWithStorage';

function useMatchMedia(query: string) {
const match = React.useRef(matchMedia(query));
Expand All @@ -22,7 +22,7 @@ function useMatchMedia(query: string) {
type LayoutContextValue = {
nav: {
isOpen: boolean;
isCollapsible: boolean;
isSmallScreen: boolean;
open: () => void;
close: () => void;
};
Expand All @@ -31,34 +31,59 @@ type LayoutContextValue = {
export const LayoutContext = React.createContext<LayoutContextValue>({
nav: {
isOpen: false,
isCollapsible: false,
isSmallScreen: false,
open: () => {},
close: () => {},
},
});

const STORAGE_KEY = 'large-screen-nav-open';

export const LayoutProvider: React.FC = (props) => {
const [navOpen, setNavOpen] = React.useState(false);
const flags = useFeatureFlags();
const [navOpenIfLargeScreen, setNavOpenIfLargeScreen] = useStateWithStorage(
STORAGE_KEY,
(json: any) => {
if (typeof json !== 'boolean') {
return false;
}
return json;
},
);

const [navOpenIfSmallScreen, setNavOpenIfSmallScreen] = React.useState(false);
const location = useLocation();
const isSmallScreen = useMatchMedia('(max-width: 1440px)');
const isInstancePage = location.pathname.startsWith('/instance');
const isCollapsible = flags.flagAlwaysCollapseNavigation || isInstancePage || isSmallScreen;

const open = React.useCallback(() => {
setNavOpenIfSmallScreen(true);
if (!isSmallScreen) {
setNavOpenIfLargeScreen(true);
}
}, [isSmallScreen, setNavOpenIfLargeScreen]);

const close = React.useCallback(() => {
setNavOpenIfSmallScreen(false);
if (!isSmallScreen) {
setNavOpenIfLargeScreen(false);
}
}, [isSmallScreen, setNavOpenIfLargeScreen]);

React.useEffect(() => {
setNavOpen(false);
setNavOpenIfSmallScreen(false);
}, [location]);

const isOpen = isSmallScreen ? navOpenIfSmallScreen : navOpenIfLargeScreen;

const value = React.useMemo(
() => ({
nav: {
isOpen: navOpen,
isCollapsible,
open: () => setNavOpen(true),
close: () => setNavOpen(false),
isOpen,
isSmallScreen,
open,
close,
},
}),
[navOpen, isCollapsible],
[isOpen, isSmallScreen, open, close],
);

return <LayoutContext.Provider value={value}>{props.children}</LayoutContext.Provider>;
Expand Down
10 changes: 0 additions & 10 deletions js_modules/dagit/packages/core/src/app/UserSettingsRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,6 @@ const UserSettingsRoot: React.FC<SettingsRootProps> = ({tabs}) => {
/>
),
},
{
key: 'Always collapse left navigation',
value: (
<Checkbox
format="switch"
checked={flags.includes(FeatureFlag.flagAlwaysCollapseNavigation)}
onChange={() => toggleFlag(FeatureFlag.flagAlwaysCollapseNavigation)}
/>
),
},
{
key: 'Disable WebSockets',
value: (
Expand Down
8 changes: 4 additions & 4 deletions js_modules/dagit/packages/core/src/nav/LeftNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,31 @@ export const LeftNav = () => {
const {nav} = React.useContext(LayoutContext);

return (
<LeftNavContainer $open={nav.isOpen} $collapsible={nav.isCollapsible}>
<LeftNavContainer $open={nav.isOpen} $smallScreen={nav.isSmallScreen}>
<LeftNavRepositorySection />
</LeftNavContainer>
);
};

export const LEFT_NAV_WIDTH = 332;

const LeftNavContainer = styled.div<{$open: boolean; $collapsible: boolean}>`
const LeftNavContainer = styled.div<{$open: boolean; $smallScreen: boolean}>`
position: fixed;
z-index: 2;
top: 64px;
bottom: 0;
left: 0;
padding-top: 16px;
width: ${LEFT_NAV_WIDTH}px;
display: flex;
display: ${({$open, $smallScreen}) => ($open || $smallScreen ? 'flex' : 'none')};
flex-shrink: 0;
flex-direction: column;
justify-content: start;
background: ${Colors.Gray100};
box-shadow: 1px 0px 0px ${Colors.KeylineGray};
${(p) =>
p.$collapsible
p.$smallScreen
? `
transform: translateX(${p.$open ? '0' : `-${LEFT_NAV_WIDTH}px`});
transition: transform 150ms ease-in-out;
Expand Down

0 comments on commit c8e5fa4

Please sign in to comment.