Skip to content

Commit

Permalink
[dagit] Fix asset graph navigation edge cases (#6955, #7208) (#7532)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bengotow
bengotow committed Apr 26, 2022
1 parent db0bf2b commit 81a09ff
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 34 deletions.
114 changes: 90 additions & 24 deletions js_modules/dagit/packages/core/src/asset-graph/AssetGraphExplorer.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import {Box, Checkbox, NonIdealState, SplitPanelContainer} from '@dagster-io/ui';
import flatMap from 'lodash/flatMap';
import pickBy from 'lodash/pickBy';
import uniq from 'lodash/uniq';
import uniqBy from 'lodash/uniqBy';
import without from 'lodash/without';
import React from 'react';
import {useHistory} from 'react-router-dom';
import styled from 'styled-components/macro';

import {GraphQueryItem} from '../app/GraphQueryImpl';
Expand Down Expand Up @@ -32,7 +34,7 @@ import {
LargeDAGNotice,
LoadingNotice,
} from '../pipelines/GraphNotices';
import {ExplorerPath} from '../pipelines/PipelinePathUtils';
import {ExplorerPath, instanceAssetsExplorerPathToURL} from '../pipelines/PipelinePathUtils';
import {SidebarPipelineOrJobOverview} from '../pipelines/SidebarPipelineOrJobOverview';
import {useDidLaunchEvent} from '../runs/RunUtils';
import {PipelineSelector} from '../types/globalTypes';
Expand All @@ -52,9 +54,10 @@ import {
isSourceAsset,
tokenForAssetKey,
} from './Utils';
import {AssetGraphLayout} from './layout';
import {AssetGraphQuery_assetNodes} from './types/AssetGraphQuery';
import {useAssetGraphData} from './useAssetGraphData';
import {useFindAssetInWorkspace} from './useFindAssetInWorkspace';
import {useFindJobForAsset} from './useFindJobForAsset';
import {useLiveDataForAssetKeys} from './useLiveDataForAssetKeys';

type AssetNode = AssetGraphQuery_assetNodes;
Expand Down Expand Up @@ -109,7 +112,6 @@ export const AssetGraphExplorer: React.FC<Props> = (props) => {
/>
);
}

return (
<AssetGraphExplorerWithData
key={props.explorerPath.pipelineName}
Expand Down Expand Up @@ -150,7 +152,8 @@ const AssetGraphExplorerWithData: React.FC<
pipelineSelector,
} = props;

const findAssetInWorkspace = useFindAssetInWorkspace();
const findJobForAsset = useFindJobForAsset();
const history = useHistory();

const selectedAssetValues = explorerPath.opNames[explorerPath.opNames.length - 1].split(',');
const selectedGraphNodes = Object.values(assetGraphData.nodes).filter((node) =>
Expand All @@ -161,6 +164,8 @@ const AssetGraphExplorerWithData: React.FC<
? selectedGraphNodes
: Object.values(assetGraphData.nodes).filter((a) => !isSourceAsset(a.definition));

const isGlobalGraph = !pipelineSelector;

const onSelectNode = React.useCallback(
async (
e: React.MouseEvent<any> | React.KeyboardEvent<any>,
Expand All @@ -170,24 +175,54 @@ const AssetGraphExplorerWithData: React.FC<
e.stopPropagation();

const token = tokenForAssetKey(assetKey);
const nodeIsInDisplayedGraph = node?.definition;

let clicked: {opNames: string[]; jobName: string | null} = {opNames: [], jobName: null};

if (node?.definition) {
if (nodeIsInDisplayedGraph) {
// The asset's defintion was provided in our job.assetNodes query. Show it in the current graph.
clicked = {opNames: node.definition.opNames, jobName: explorerPath.pipelineName};
} else {
// The asset's definition was not provided in our query for job.assetNodes. This means
// it's in another job or is a source asset not defined in the repository at all.
clicked = await findAssetInWorkspace(assetKey);
// The asset's definition was not provided in our query for job.assetNodes. It's either
// in another job or asset group, or is a source asset not defined in any repository.
clicked = await findJobForAsset(assetKey);
}

if (!clicked.opNames.length) {
// This op has no definition in any loaded repository (source asset).
// The best we can do is show the asset page. This will still be mostly empty,
// but there can be a description.
history.push(`/instance/assets/${assetKey.path.join('/')}?view=definition`);
return;
}

if (!clicked.jobName && !isGlobalGraph) {
// This asset has a definition (opName) but isn't in any non asset-group jobs.
// We can switch to the instance-wide asset graph and see it in context there.
history.push(
instanceAssetsExplorerPathToURL({
opsQuery: `++"${token}"++`,
opNames: [token],
}),
);
return;
}

// This asset is in different job (and we're in the job explorer),
// go to the other job.
if (!isGlobalGraph && clicked.jobName !== explorerPath.pipelineName) {
onChangeExplorerPath(
{...explorerPath, opNames: [token], opsQuery: '', pipelineName: clicked.jobName!},
'replace',
);
return;
}

let nextOpsQuery = explorerPath.opsQuery;
// This asset is in a job and we can stay in the job graph explorer!
// If it's in our current job, allow shift / meta multi-selection.
let nextOpsNameSelection = token;

// If no opName, this is a source asset.
if (clicked.jobName !== explorerPath.pipelineName || !clicked.opNames.length) {
nextOpsQuery = '';
} else if (e.shiftKey || e.metaKey) {
if (e.shiftKey || e.metaKey) {
const existing = explorerPath.opNames[0].split(',');
const added =
e.shiftKey && lastSelectedNode && node
Expand All @@ -204,21 +239,47 @@ const AssetGraphExplorerWithData: React.FC<
{
...explorerPath,
opNames: [nextOpsNameSelection],
opsQuery: nextOpsQuery,
pipelineName: clicked.jobName || explorerPath.pipelineName,
opsQuery: nodeIsInDisplayedGraph
? explorerPath.opsQuery
: `${explorerPath.opsQuery},++"${token}"++`,
pipelineName: explorerPath.pipelineName,
},
'replace',
);
},
[explorerPath, onChangeExplorerPath, findAssetInWorkspace, lastSelectedNode, assetGraphData],
[
isGlobalGraph,
explorerPath,
onChangeExplorerPath,
findJobForAsset,
history,
lastSelectedNode,
assetGraphData,
],
);

const {layout, loading, async} = useAssetLayout(assetGraphData);

const viewportEl = React.useRef<SVGViewport>();

const [lastRenderedLayout, setLastRenderedLayout] = React.useState<AssetGraphLayout | null>(null);
const renderingNewLayout = lastRenderedLayout !== layout;

React.useEffect(() => {
viewportEl.current?.autocenter();
}, [layout, viewportEl]);
if (!renderingNewLayout || !layout || !viewportEl.current) {
return;
}
// The first render where we have our layout and viewport, autocenter or
// focus on the selected node. (If selection was specified in the URL).
// Don't animate this change.
if (lastSelectedNode) {
viewportEl.current.zoomToSVGBox(layout.nodes[lastSelectedNode.id].bounds, false);
viewportEl.current.focus();
} else {
viewportEl.current.autocenter(false);
}
setLastRenderedLayout(layout);
}, [renderingNewLayout, lastSelectedNode, layout, viewportEl]);

const onClickBackground = () =>
onChangeExplorerPath(
Expand All @@ -227,11 +288,17 @@ const AssetGraphExplorerWithData: React.FC<
);

const onArrowKeyDown = (e: React.KeyboardEvent<any>, dir: string) => {
const nextId = layout && closestNodeInDirection(layout, lastSelectedNode.id, dir);
if (!layout) {
return;
}
const hasDefinition = (node: {id: string}) => !!assetGraphData.nodes[node.id]?.definition;
const layoutWithoutExternalLinks = {...layout, nodes: pickBy(layout.nodes, hasDefinition)};

const nextId = closestNodeInDirection(layoutWithoutExternalLinks, lastSelectedNode.id, dir);
const node = nextId && assetGraphData.nodes[nextId];
if (node && viewportEl.current) {
onSelectNode(e, node.assetKey, node);
viewportEl.current.smoothZoomToSVGBox(layout.nodes[nextId].bounds);
viewportEl.current.zoomToSVGBox(layout.nodes[nextId].bounds, true);
}
};

Expand Down Expand Up @@ -273,8 +340,7 @@ const AssetGraphExplorerWithData: React.FC<
{Object.values(layout.nodes).map(({id, bounds}, index) => {
const graphNode = assetGraphData.nodes[id];
const path = JSON.parse(id);

if (isNodeOffscreen(bounds, viewportRect)) {
if (!renderingNewLayout && isNodeOffscreen(bounds, viewportRect)) {
return id === lastSelectedNode?.id ? (
<RecenterGraph
key={index}
Expand All @@ -291,7 +357,7 @@ const AssetGraphExplorerWithData: React.FC<
key={id}
onClick={(e) => onSelectNode(e, {path}, graphNode)}
onDoubleClick={(e) => {
viewportEl.current?.smoothZoomToSVGBox(bounds, 1.2);
viewportEl.current?.zoomToSVGBox(bounds, true, 1.2);
e.stopPropagation();
}}
style={{overflow: 'visible'}}
Expand Down Expand Up @@ -479,7 +545,7 @@ const RecenterGraph: React.FC<{
y: number;
}> = ({viewportRef, x, y}) => {
React.useEffect(() => {
viewportRef.current?.smoothZoomToSVGCoords(x, y, viewportRef.current.state.scale);
viewportRef.current?.zoomToSVGCoords(x, y, true);
}, [viewportRef, x, y]);

return <span />;
Expand Down
8 changes: 6 additions & 2 deletions ...c/asset-graph/useFindAssetInWorkspace.tsx → ...re/src/asset-graph/useFindJobForAsset.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import React from 'react';

import {AssetKeyInput} from '../types/globalTypes';

import {isAssetGroup} from './Utils';
import {
AssetForNavigationQuery,
AssetForNavigationQueryVariables,
} from './types/AssetForNavigationQuery';

export function useFindAssetInWorkspace() {
export function useFindJobForAsset() {
const apollo = useApolloClient();

return React.useCallback(
Expand All @@ -19,7 +20,10 @@ export function useFindAssetInWorkspace() {
});
if (data?.assetOrError.__typename === 'Asset' && data?.assetOrError.definition) {
const def = data.assetOrError.definition;
return {opNames: def.opNames, jobName: def.jobNames[0] || null};
return {
opNames: def.opNames,
jobName: def.jobNames.find((jobName) => !isAssetGroup(jobName)) || null,
};
}
return {opNames: [], jobName: null};
},
Expand Down
4 changes: 2 additions & 2 deletions js_modules/dagit/packages/core/src/graph/OpGraph.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,14 @@ export class OpGraph extends React.Component<OpGraphProps> {
centerOp = (arg: OpNameOrPath) => {
const opLayout = this.argToOpLayout(arg);
if (opLayout && this.viewportEl.current) {
this.viewportEl.current.smoothZoomToSVGBox(opLayout.bounds);
this.viewportEl.current.zoomToSVGBox(opLayout.bounds, true);
}
};

focusOnOp = (arg: OpNameOrPath) => {
const opLayout = this.argToOpLayout(arg);
if (opLayout && this.viewportEl.current) {
this.viewportEl.current?.smoothZoomToSVGBox(opLayout.bounds, DETAIL_ZOOM);
this.viewportEl.current?.zoomToSVGBox(opLayout.bounds, true, DETAIL_ZOOM);
}
};

Expand Down
20 changes: 14 additions & 6 deletions js_modules/dagit/packages/core/src/graph/SVGViewport.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,10 @@ export class SVGViewport extends React.Component<SVGViewportProps, SVGViewportSt
}
}

focus() {
this.element.current?.focus();
}

autocenter(animate = false) {
const el = this.element.current!;
const ownerRect = {width: el.clientWidth, height: el.clientHeight};
Expand Down Expand Up @@ -327,17 +331,21 @@ export class SVGViewport extends React.Component<SVGViewportProps, SVGViewportSt
this.setState({x, y, scale: nextScale});
}

public smoothZoomToSVGBox(box: IBounds, newScale = this.state.scale) {
this.smoothZoomToSVGCoords(box.x + box.width / 2, box.y + box.height / 2, newScale);
public zoomToSVGBox(box: IBounds, animate: boolean, newScale = this.state.scale) {
this.zoomToSVGCoords(box.x + box.width / 2, box.y + box.height / 2, animate, newScale);
}

public smoothZoomToSVGCoords(x: number, y: number, scale: number) {
public zoomToSVGCoords(x: number, y: number, animate: boolean, scale = this.state.scale) {
const el = this.element.current!;
const ownerRect = el.getBoundingClientRect();
x = -x * scale + ownerRect.width / 2;
y = -y * scale + ownerRect.height / 2;

this.smoothZoom({x, y, scale});
if (animate) {
this.smoothZoom({x, y, scale});
} else {
this.setState({x, y, scale});
}
}

public smoothZoom(to: {x: number; y: number; scale: number}) {
Expand Down Expand Up @@ -390,9 +398,9 @@ export class SVGViewport extends React.Component<SVGViewportProps, SVGViewportSt
const maxZoom = this.props.maxZoom || DEFAULT_ZOOM;

if (Math.abs(maxZoom - this.state.scale) < 0.01) {
this.smoothZoomToSVGCoords(offset.x, offset.y, this.state.minScale);
this.zoomToSVGCoords(offset.x, offset.y, true, this.state.minScale);
} else {
this.smoothZoomToSVGCoords(offset.x, offset.y, maxZoom);
this.zoomToSVGCoords(offset.x, offset.y, true, maxZoom);
}
};

Expand Down

0 comments on commit 81a09ff

Please sign in to comment.