Skip to content

Commit 0836994

Browse files
SpencerWhitehead7SpencerWhitehead7
committed
refac: defer queries until the AssetActionMenu is actually opened
1 parent 5c56bf2 commit 0836994

5 files changed

Lines changed: 148 additions & 110 deletions

File tree

‎js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
/* eslint-disable @typescript-eslint/no-var-requires */
21
// eslint-disable-next-line no-restricted-imports
3-
import {Popover2, Popover2Props} from '@blueprintjs/popover2';
2+
import {
3+
Popover as BlueprintPopover,
4+
PopoverProps as BlueprintPopoverProps,
5+
} from '@blueprintjs/core';
46
import deepmerge from 'deepmerge';
5-
import * as React from 'react';
67
import {createGlobalStyle, css} from 'styled-components';
78

89
import {Colors} from './Color';
@@ -105,13 +106,9 @@ export const GlobalPopoverStyle = createGlobalStyle`
105106
// Overwrite arrays instead of concatting them.
106107
const overwriteMerge = (destination: any[], source: any[]) => source;
107108

108-
interface Props extends Popover2Props {
109-
children: React.ReactNode;
110-
}
111-
112-
export const Popover = (props: Props) => {
109+
export const Popover = (props: BlueprintPopoverProps) => {
113110
return (
114-
<Popover2
111+
<BlueprintPopover
115112
minimal
116113
autoFocus={false}
117114
enforceFocus={false}

‎js_modules/dagster-ui/packages/ui-core/src/assets/AssetActionMenu.tsx‎

Lines changed: 64 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@ import {
33
HoverButton,
44
Icon,
55
Menu,
6+
MenuDivider,
67
MenuItem,
78
Popover,
89
Spinner,
910
Tooltip,
1011
} from '@dagster-io/ui-components';
11-
import {memo, useContext, useMemo} from 'react';
12+
import {memo, useContext, useMemo, useState} from 'react';
1213
import {AddToFavoritesMenuItem} from 'shared/assets/AddToFavoritesMenuItem.oss';
1314

1415
import {optionsForExecuteButton, useMaterializationAction} from './LaunchAssetExecutionButton';
@@ -24,6 +25,7 @@ import {showSharedToaster} from '../app/DomUtils';
2425
import {AssetKeyInput} from '../graphql/types';
2526
import {MenuLink} from '../ui/MenuLink';
2627
import {RepoAddress} from '../workspace/types';
28+
2729
interface Props {
2830
path: string[];
2931
definition: AssetTableDefinitionFragment | null;
@@ -38,6 +40,10 @@ export const AssetActionMenu = memo((props: Props) => {
3840
featureContext: {canSeeMaterializeAction},
3941
} = useContext(CloudOSSContext);
4042

43+
// we need this to make the Popover a controlled component and choose what content to render because the
44+
// content contains several expensive queries we want to defer until the menu is actually opened and rendered
45+
const [isOpen, setIsOpen] = useState(false);
46+
4147
// Because the asset catalog loads a list of keys, and then definitions for SDAs,
4248
// we don't re-fetch the key inside the definition of each asset. Re-attach the
4349
// assetKey to the definition for the hook below.
@@ -77,6 +83,8 @@ export const AssetActionMenu = memo((props: Props) => {
7783
{reportEvents.element}
7884
{deletePartitions.element}
7985
<Popover
86+
isOpen={isOpen}
87+
onInteraction={setIsOpen}
8088
position="bottom-right"
8189
targetTagName="div"
8290
targetProps={{
@@ -86,57 +94,61 @@ export const AssetActionMenu = memo((props: Props) => {
8694
},
8795
}}
8896
content={
89-
<Menu>
90-
{executeItem}
91-
<AddToFavoritesMenuItem assetKey={{path}} />
92-
93-
<MenuLink
94-
text="Show in group"
95-
to={
96-
definition
97-
? globalAssetGraphPathForGroup(definition.groupName, definition.assetKey)
98-
: '/asset-groups'
99-
}
100-
disabled={!definition}
101-
icon="asset_group"
102-
/>
103-
<MenuLink
104-
text="View checks"
105-
to={assetDetailsPathForKey({path}, {view: 'checks'})}
106-
disabled={!definition}
107-
icon="asset_check"
108-
/>
109-
<MenuLink
110-
text="View lineage"
111-
to={assetDetailsPathForKey({path}, {view: 'lineage', lineageScope: 'neighbors'})}
112-
disabled={!definition}
113-
icon="graph_neighbors"
114-
/>
115-
<MenuLink
116-
text="View upstream assets"
117-
to={assetDetailsPathForKey({path}, {view: 'lineage', lineageScope: 'upstream'})}
118-
disabled={!definition}
119-
icon="graph_upstream"
120-
/>
121-
<MenuLink
122-
text="View downstream assets"
123-
to={assetDetailsPathForKey({path}, {view: 'lineage', lineageScope: 'downstream'})}
124-
disabled={!definition}
125-
icon="graph_downstream"
126-
/>
127-
{canSeeMaterializeAction && definition?.hasMaterializePermission
128-
? reportEvents.dropdownOptions.map((option) => (
129-
<MenuItem
130-
key={option.label}
131-
text={option.label}
132-
icon={option.icon}
133-
onClick={option.onClick}
134-
/>
135-
))
136-
: undefined}
137-
{wipe.dropdownOptions}
138-
{deletePartitions.dropdownOptions}
139-
</Menu>
97+
isOpen ? (
98+
<Menu>
99+
{executeItem}
100+
<AddToFavoritesMenuItem assetKey={{path}} />
101+
<MenuLink
102+
text="Show in group"
103+
to={
104+
definition
105+
? globalAssetGraphPathForGroup(definition.groupName, definition.assetKey)
106+
: '/asset-groups'
107+
}
108+
disabled={!definition}
109+
icon="asset_group"
110+
/>
111+
<MenuLink
112+
text="View checks"
113+
to={assetDetailsPathForKey({path}, {view: 'checks'})}
114+
disabled={!definition}
115+
icon="asset_check"
116+
/>
117+
<MenuLink
118+
text="View lineage"
119+
to={assetDetailsPathForKey({path}, {view: 'lineage', lineageScope: 'neighbors'})}
120+
disabled={!definition}
121+
icon="graph_neighbors"
122+
/>
123+
<MenuLink
124+
text="View upstream assets"
125+
to={assetDetailsPathForKey({path}, {view: 'lineage', lineageScope: 'upstream'})}
126+
disabled={!definition}
127+
icon="graph_upstream"
128+
/>
129+
<MenuLink
130+
text="View downstream assets"
131+
to={assetDetailsPathForKey({path}, {view: 'lineage', lineageScope: 'downstream'})}
132+
disabled={!definition}
133+
icon="graph_downstream"
134+
/>
135+
{canSeeMaterializeAction && definition?.hasMaterializePermission
136+
? reportEvents.dropdownOptions.map((option) => (
137+
<MenuItem
138+
key={option.label}
139+
text={option.label}
140+
icon={option.icon}
141+
onClick={option.onClick}
142+
/>
143+
))
144+
: undefined}
145+
<MenuDivider />
146+
{wipe.dropdownOptions}
147+
{deletePartitions.dropdownOptions}
148+
</Menu>
149+
) : (
150+
<></>
151+
)
140152
}
141153
>
142154
{unstyledButton ? (

‎js_modules/dagster-ui/packages/ui-core/src/assets/useAssetPermissions.tsx‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,12 @@ export const useAssetPermissions = (assetKeys: AssetKeyInput[], locationName: st
5151
}
5252

5353
// Permission is allowed only if ALL assets allow it
54-
const hasMaterializePermission = assetNodes.every((node) => node.hasMaterializePermission);
55-
const hasWipePermission = assetNodes.every((node) => node.hasWipePermission);
56-
const hasReportRunlessAssetEventPermission = assetNodes.every(
57-
(node) => node.hasReportRunlessAssetEventPermission,
58-
);
59-
6054
return {
61-
hasMaterializePermission,
62-
hasWipePermission,
63-
hasReportRunlessAssetEventPermission,
55+
hasMaterializePermission: assetNodes.every((node) => node.hasMaterializePermission),
56+
hasWipePermission: assetNodes.every((node) => node.hasWipePermission),
57+
hasReportRunlessAssetEventPermission: assetNodes.every(
58+
(node) => node.hasReportRunlessAssetEventPermission,
59+
),
6460
loading,
6561
};
6662
}, [data, loading, locationLoading, fallbackPermissions, assetKeys]);

‎js_modules/dagster-ui/packages/ui-core/src/assets/useDeleteDynamicPartitionsDialog.tsx‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import {Colors, Icon, MenuItem} from '@dagster-io/ui-components';
2-
import {useContext, useState} from 'react';
2+
import {Dispatch, SetStateAction, useContext, useState} from 'react';
33

44
import {DeleteDynamicPartitionsDialog} from './DeleteDynamicPartitionsDialog';
55
import {useAssetPermissions} from './useAssetPermissions';
66
import {CloudOSSContext} from '../app/CloudOSSContext';
77
import {AssetKeyInput, PartitionDefinitionType} from '../graphql/types';
88
import {RepoAddress} from '../workspace/types';
99

10-
export function useDeleteDynamicPartitionsDialog(
10+
export const useDeleteDynamicPartitionsDialog = (
1111
opts: {
1212
repoAddress: RepoAddress;
1313
assetKey: AssetKeyInput;
@@ -20,8 +20,8 @@ export function useDeleteDynamicPartitionsDialog(
2020
};
2121
} | null,
2222
refresh?: () => void,
23-
) {
24-
const [showing, setShowing] = useState(false);
23+
) => {
24+
const [isShowing, setIsShowing] = useState(false);
2525

2626
const {
2727
featureContext: {canSeeWipeMaterializationAction},
@@ -31,19 +31,14 @@ export function useDeleteDynamicPartitionsDialog(
3131
(d) => d.type === PartitionDefinitionType.DYNAMIC,
3232
);
3333

34-
const {hasWipePermission} = useAssetPermissions(
35-
opts?.assetKey ? [opts.assetKey] : [],
36-
opts?.repoAddress.location || '',
37-
);
38-
3934
if (
4035
!opts ||
4136
!dynamicDimension?.dynamicPartitionsDefinitionName ||
4237
!canSeeWipeMaterializationAction
4338
) {
4439
return {
4540
element: undefined,
46-
dropdownOptions: [] as JSX.Element[],
41+
dropdownOptions: [],
4742
};
4843
}
4944

@@ -53,20 +48,44 @@ export function useDeleteDynamicPartitionsDialog(
5348
repoAddress={opts.repoAddress}
5449
assetKey={opts.assetKey}
5550
partitionsDefName={dynamicDimension.dynamicPartitionsDefinitionName}
56-
isOpen={showing}
57-
onClose={() => setShowing(false)}
51+
isOpen={isShowing}
52+
onClose={() => setIsShowing(false)}
5853
onComplete={refresh}
5954
/>
6055
),
6156
dropdownOptions: [
62-
<MenuItem
57+
<DeleteDynamicPartitionsMenuItem
6358
key="delete"
64-
text="Delete partitions"
65-
icon={<Icon name="delete" color={Colors.accentRed()} />}
66-
disabled={!hasWipePermission}
67-
intent="danger"
68-
onClick={() => setShowing(true)}
59+
setIsShowing={setIsShowing}
60+
assetKeys={opts?.assetKey ? [opts.assetKey] : []}
61+
locationName={opts?.repoAddress.location || ''}
6962
/>,
7063
],
7164
};
72-
}
65+
};
66+
67+
type DeleteDynamicPartitionsMenuItemProps = {
68+
assetKeys: AssetKeyInput[];
69+
locationName: string;
70+
setIsShowing: Dispatch<SetStateAction<boolean>>;
71+
};
72+
73+
const DeleteDynamicPartitionsMenuItem = ({
74+
assetKeys,
75+
locationName,
76+
setIsShowing,
77+
}: DeleteDynamicPartitionsMenuItemProps) => {
78+
// this separate comp exists to defer this expensive query until
79+
// the menuItem is rendered instead of when the hook is called
80+
const {hasWipePermission} = useAssetPermissions(assetKeys, locationName);
81+
82+
return (
83+
<MenuItem
84+
text="Delete partitions"
85+
icon={<Icon name="delete" color={Colors.accentRed()} />}
86+
disabled={!hasWipePermission}
87+
intent="danger"
88+
onClick={() => setIsShowing(true)}
89+
/>
90+
);
91+
};

‎js_modules/dagster-ui/packages/ui-core/src/assets/useWipeDialog.tsx‎

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,62 @@
1-
import {Colors, Icon, MenuDivider, MenuItem} from '@dagster-io/ui-components';
2-
import {useContext, useState} from 'react';
1+
import {Colors, Icon, MenuItem} from '@dagster-io/ui-components';
2+
import {Dispatch, SetStateAction, useContext, useState} from 'react';
33
import {AssetWipeDialog} from 'shared/assets/AssetWipeDialog.oss';
44

55
import {useAssetPermissions} from './useAssetPermissions';
66
import {CloudOSSContext} from '../app/CloudOSSContext';
77
import {AssetKeyInput} from '../graphql/types';
88

9-
export function useWipeDialog(
9+
export const useWipeDialog = (
1010
opts: {assetKey: AssetKeyInput; repository: {location: {name: string}} | null} | null,
11-
refresh?: () => void,
12-
) {
13-
const [showing, setShowing] = useState(false);
11+
refresh?: VoidFunction,
12+
) => {
13+
const [isShowing, setIsShowing] = useState(false);
1414

1515
const {
1616
featureContext: {canSeeWipeMaterializationAction},
1717
} = useContext(CloudOSSContext);
1818

19-
const {hasWipePermission} = useAssetPermissions(
20-
opts?.assetKey ? [opts.assetKey] : [],
21-
opts?.repository?.location.name || '',
22-
);
23-
2419
return {
2520
element: (
2621
<AssetWipeDialog
2722
assetKeys={opts ? [opts.assetKey] : []}
28-
isOpen={showing}
29-
onClose={() => setShowing(false)}
23+
isOpen={isShowing}
24+
onClose={() => setIsShowing(false)}
3025
onComplete={refresh}
3126
/>
3227
),
3328
dropdownOptions:
3429
opts && canSeeWipeMaterializationAction
3530
? [
36-
<MenuDivider key="wipe-divider" />,
37-
<MenuItem
31+
<WipeDialogMenuItem
3832
key="wipe"
39-
text="Wipe materializations"
40-
icon={<Icon name="delete" color={Colors.accentRed()} />}
41-
disabled={!hasWipePermission}
42-
intent="danger"
43-
onClick={() => setShowing(true)}
33+
setIsShowing={setIsShowing}
34+
assetKeys={opts?.assetKey ? [opts.assetKey] : []}
35+
locationName={opts?.repository?.location.name || ''}
4436
/>,
4537
]
4638
: [],
4739
};
48-
}
40+
};
41+
42+
type WipeDialogMenuItemProps = {
43+
assetKeys: AssetKeyInput[];
44+
locationName: string;
45+
setIsShowing: Dispatch<SetStateAction<boolean>>;
46+
};
47+
48+
const WipeDialogMenuItem = ({assetKeys, locationName, setIsShowing}: WipeDialogMenuItemProps) => {
49+
// this separate comp exists to defer this expensive query until
50+
// the menuItem is rendered instead of when the hook is called
51+
const {hasWipePermission} = useAssetPermissions(assetKeys, locationName);
52+
53+
return (
54+
<MenuItem
55+
text="Wipe materializationz"
56+
icon={<Icon name="delete" color={Colors.accentRed()} />}
57+
disabled={!hasWipePermission}
58+
intent="danger"
59+
onClick={() => setIsShowing(true)}
60+
/>
61+
);
62+
};

0 commit comments

Comments
 (0)