Skip to content

Commit

Permalink
[dagit] Move User Settings into a Dialog (#11706)
Browse files Browse the repository at this point in the history
### Summary & Motivation

Begin moving "User settings" into a dialog.

<img width="1223" alt="Screenshot 2023-01-12 at 4 14 38 PM" src="https://user-images.githubusercontent.com/2823852/212395674-e58604d6-d6ff-44b6-a3b8-9acdd38914ee.png">


- Create `UserSettingsButton` to replace the `SettingsLink` defined in the `app` index file. This is because we no longer have a global route to refer to -- all of the user settings behavior needs to move to `core`.
- Create a dialog component that contains the contentful pieces of User Settings configuration
- Create `UserSettingsDialogContent`, which retrieves and renders time zone, feature flag, and keyboard shortcut management UI
- Refactor `getFeatureFlagRows` to return key/value pairs for the flags that should be shown. This allows `UserSettingsDialogContent` to render checkboxes for us, instead of specifying them in this function.
  - In Cloud, we will be able to splat this array alongside any Cloud-specific flags, and pass that as `visibleFlags` to the `UserSettingsDialog` component 
- Remove the OSS `/settings` route.

Because toggling flags previously would reload the page, and now a fresh reload would leave us with no dialog, the user will be forced to click "Done" to continue from the dialog. If any changes have been made, the page reloads to incorporate the flag change. If no changes are made, "Done" just closes the dialog.

### How I Tested These Changes

View Dagit, click on gear button. Verify that the dialog appears.

- Make no changes, close the dialog with "Done". Verify that there is no page reload.
- Reopen, toggle a flag, click "Done". Verify that the page reloads.

View Cloud (master). Verify that the User Settings page renders and behaves properly using the new components, with no errors.
  • Loading branch information
hellendag committed Jan 17, 2023
1 parent 26f7e93 commit c0c4811
Show file tree
Hide file tree
Showing 6 changed files with 247 additions and 174 deletions.
32 changes: 2 additions & 30 deletions js_modules/dagit/packages/app/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ import {errorLink} from '@dagster-io/dagit-core/app/AppError';
import {AppProvider} from '@dagster-io/dagit-core/app/AppProvider';
import {AppTopNav} from '@dagster-io/dagit-core/app/AppTopNav';
import {ContentRoot} from '@dagster-io/dagit-core/app/ContentRoot';
import {UserSettingsButton} from '@dagster-io/dagit-core/app/UserSettingsButton';
import {logLink, timeStartLink} from '@dagster-io/dagit-core/app/apolloLinks';
import {DeploymentStatusType} from '@dagster-io/dagit-core/instance/DeploymentStatusProvider';
import {Colors, Icon, IconWrapper} from '@dagster-io/ui';
import * as React from 'react';
import ReactDOM from 'react-dom';
import {Link} from 'react-router-dom';
import styled from 'styled-components/macro';

import {extractInitializationData} from './extractInitializationData';
import {telemetryLink} from './telemetryLink';
Expand All @@ -37,36 +35,10 @@ const config = {

const appCache = createAppCache();

const SettingsLink = styled(Link)`
padding: 24px;
${IconWrapper} {
transition: background 50ms linear;
}
&:hover ${IconWrapper} {
background: ${Colors.White};
}
&:active ${IconWrapper} {
background: ${Colors.White};
}
&:focus {
outline: none;
${IconWrapper} {
background: ${Colors.White};
}
}
`;

ReactDOM.render(
<AppProvider appCache={appCache} config={config}>
<AppTopNav searchPlaceholder="Search…">
<SettingsLink to="/settings" title="User settings">
<Icon name="settings" color={Colors.Gray200} />
</SettingsLink>
<UserSettingsButton />
</AppTopNav>
<App>
<ContentRoot />
Expand Down
6 changes: 0 additions & 6 deletions js_modules/dagit/packages/core/src/app/ContentRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {MainContent} from '@dagster-io/ui';
import * as React from 'react';
import {Route, Switch, useLocation} from 'react-router-dom';

const UserSettingsRoot = React.lazy(() => import('./UserSettingsRoot'));
const WorkspaceRoot = React.lazy(() => import('../workspace/WorkspaceRoot'));
const OverviewRoot = React.lazy(() => import('../overview/OverviewRoot'));
const FallthroughRoot = React.lazy(() => import('./FallthroughRoot'));
Expand Down Expand Up @@ -79,11 +78,6 @@ export const ContentRoot = React.memo(() => {
<WorkspaceRoot />
</React.Suspense>
</Route>
<Route path="/settings">
<React.Suspense fallback={<div />}>
<UserSettingsRoot />
</React.Suspense>
</Route>
<Route path="/overview">
<React.Suspense fallback={<div />}>
<OverviewRoot />
Expand Down
49 changes: 49 additions & 0 deletions js_modules/dagit/packages/core/src/app/UserSettingsButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {IconWrapper, Colors, Icon} from '@dagster-io/ui';
import * as React from 'react';
import styled from 'styled-components/macro';

import {UserSettingsDialog} from './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.White};
}
&:active ${IconWrapper} {
background: ${Colors.White};
}
&:focus {
outline: none;
${IconWrapper} {
background: ${Colors.White};
}
}
`;

export const UserSettingsButton = () => {
const [isOpen, setIsOpen] = React.useState(false);
return (
<>
<SettingsButton onClick={() => setIsOpen(true)} title="User settings">
<Icon name="settings" color={Colors.Gray200} />
</SettingsButton>
<UserSettingsDialog
isOpen={isOpen}
onClose={() => setIsOpen(false)}
visibleFlags={getVisibleFeatureFlagRows()}
/>
</>
);
};
156 changes: 156 additions & 0 deletions js_modules/dagit/packages/core/src/app/UserSettingsDialog.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import {
Box,
Button,
ButtonLink,
Checkbox,
Colors,
Dialog,
DialogBody,
DialogFooter,
MetadataTable,
Subheading,
} from '@dagster-io/ui';
import * as React from 'react';

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

import {FeatureFlagType, getFeatureFlags, setFeatureFlags} from './Flags';
import {SHORTCUTS_STORAGE_KEY} from './ShortcutHandler';
import {TimezoneSelect} from './time/TimezoneSelect';
import {automaticLabel} from './time/browserTimezone';

type OnCloseFn = (event: React.SyntheticEvent<HTMLElement>) => void;
type VisibleFlag = {key: string; flagType: FeatureFlagType};

interface DialogProps {
isOpen: boolean;
onClose: OnCloseFn;
visibleFlags: VisibleFlag[];
}

export const UserSettingsDialog: React.FC<DialogProps> = ({isOpen, onClose, visibleFlags}) => {
return (
<Dialog
title="User settings"
isOpen={isOpen}
canOutsideClickClose={false}
canEscapeKeyClose={false}
>
<UserSettingsDialogContent onClose={onClose} visibleFlags={visibleFlags} />
</Dialog>
);
};

interface DialogContentProps {
onClose: OnCloseFn;
visibleFlags: {key: string; flagType: FeatureFlagType}[];
}

/**
* Separate the content from the `Dialog` so that we don't prepare its state before
* we want to render it.
*/
export const UserSettingsDialogContent: React.FC<DialogContentProps> = ({
onClose,
visibleFlags,
}) => {
const [flags, setFlags] = React.useState<FeatureFlagType[]>(() => getFeatureFlags());
const [reloading, setReloading] = React.useState(false);

const [shortcutsEnabled, setShortcutsEnabled] = useStateWithStorage(
SHORTCUTS_STORAGE_KEY,
(value: any) => (typeof value === 'boolean' ? value : true),
);

const initialFlagState = React.useRef(JSON.stringify([...getFeatureFlags().sort()]));
const initialShortcutsEnabled = React.useRef(shortcutsEnabled);

React.useEffect(() => {
setFeatureFlags(flags);
});

const toggleFlag = (flag: FeatureFlagType) => {
setFlags(flags.includes(flag) ? flags.filter((f) => f !== flag) : [...flags, flag]);
};

const trigger = React.useCallback(
(timezone: string) => (
<ButtonLink>{timezone === 'Automatic' ? automaticLabel() : timezone}</ButtonLink>
),
[],
);

const toggleKeyboardShortcuts = (e: React.ChangeEvent<HTMLInputElement>) => {
const {checked} = e.target;
setShortcutsEnabled(checked);
};

const anyChange =
initialFlagState.current !== JSON.stringify([...flags.sort()]) ||
initialShortcutsEnabled.current !== shortcutsEnabled;

const handleClose = (event: React.SyntheticEvent<HTMLElement>) => {
if (anyChange) {
setReloading(true);
window.location.reload();
} else {
onClose(event);
}
};

return (
<>
<DialogBody>
<Box padding={{bottom: 8}}>
<Box padding={{bottom: 8}}>
<Subheading>Preferences</Subheading>
</Box>
<MetadataTable
rows={[
{
key: 'Timezone',
value: (
<Box margin={{bottom: 4}}>
<TimezoneSelect trigger={trigger} />
</Box>
),
},
{
key: 'Enable keyboard shortcuts',
value: (
<Checkbox
checked={shortcutsEnabled}
format="switch"
onChange={toggleKeyboardShortcuts}
/>
),
},
]}
/>
</Box>
<Box padding={{top: 16}} border={{side: 'top', width: 1, color: Colors.KeylineGray}}>
<Box padding={{bottom: 8}}>
<Subheading>Experimental features</Subheading>
</Box>
<MetadataTable
rows={visibleFlags.map(({key, flagType}) => ({
key,
value: (
<Checkbox
format="switch"
checked={flags.includes(flagType)}
onChange={() => toggleFlag(flagType)}
/>
),
}))}
/>
</Box>
</DialogBody>
<DialogFooter topBorder>
<Button intent="primary" onClick={handleClose} disabled={reloading}>
Done
</Button>
</DialogFooter>
</>
);
};

0 comments on commit c0c4811

Please sign in to comment.