Skip to content

Commit

Permalink
[ui] Fix vertical centering of arrows on the asset graph, asset name …
Browse files Browse the repository at this point in the history 
…wrapping
  • Loading branch information
bengotow committed Nov 18, 2024
1 parent 75dc7ae commit 1f4dece
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 54 deletions.
47 changes: 29 additions & 18 deletions js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNode.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import {AssetNodeMenuProps, useAssetNodeMenu} from './AssetNodeMenu';
import {buildAssetNodeStatusContent} from './AssetNodeStatusContent';
import {ContextMenuWrapper} from './ContextMenuWrapper';
import {LiveDataForNode} from './Utils';
import {ASSET_NODE_NAME_MAX_LENGTH} from './layout';
import {
ASSET_NODE_NAME_MAX_LENGTH,
ASSET_NODE_STATUS_ROW_HEIGHT,
ASSET_NODE_TAGS_HEIGHT,
} from './layout';
import {gql} from '../apollo-client';
import {AssetNodeFragment} from './types/AssetNode.types';
import {withMiddleTruncation} from '../app/Util';
Expand All @@ -30,19 +34,23 @@ interface Props {

export const AssetNode = React.memo(({definition, selected, kindFilter}: Props) => {
const {liveData} = useAssetLiveData(definition.assetKey);
const hasChecks = (liveData?.assetChecks || []).length > 0;

const marginTopForCenteringNode = !hasChecks ? ASSET_NODE_STATUS_ROW_HEIGHT / 2 : 0;

return (
<AssetInsetForHoverEffect>
<Box
flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'center'}}
style={{minHeight: 24}}
>
<StaleReasonsTag liveData={liveData} assetKey={definition.assetKey} />
<ChangedReasonsTag
changedReasons={definition.changedReasons}
assetKey={definition.assetKey}
/>
</Box>
<AssetNodeContainer $selected={selected}>
<Box
flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'center'}}
style={{minHeight: ASSET_NODE_TAGS_HEIGHT, marginTop: marginTopForCenteringNode}}
>
<StaleReasonsTag liveData={liveData} assetKey={definition.assetKey} />
<ChangedReasonsTag
changedReasons={definition.changedReasons}
assetKey={definition.assetKey}
/>
</Box>
<AssetNodeBox $selected={selected} $isMaterializable={definition.isMaterializable}>
<AssetNameRow definition={definition} />
<Box style={{padding: '6px 8px'}} flex={{direction: 'column', gap: 4}} border="top">
Expand All @@ -59,16 +67,17 @@ export const AssetNode = React.memo(({definition, selected, kindFilter}: Props)
</Box>

<AssetNodeStatusRow definition={definition} liveData={liveData} />
{(liveData?.assetChecks || []).length > 0 && (
<AssetNodeChecksRow definition={definition} liveData={liveData} />
)}
{hasChecks && <AssetNodeChecksRow definition={definition} liveData={liveData} />}
</AssetNodeBox>
<Box flex={{direction: 'row-reverse', gap: 8}}>
<Box
style={{minHeight: ASSET_NODE_TAGS_HEIGHT}}
flex={{alignItems: 'center', direction: 'row-reverse', gap: 8}}
>
{definition.kinds.map((kind) => (
<AssetKind
key={kind}
kind={kind}
style={{position: 'relative', paddingTop: 7, margin: 0}}
style={{position: 'relative', margin: 0}}
currentPageFilter={kindFilter}
/>
))}
Expand Down Expand Up @@ -272,7 +281,7 @@ export const ASSET_NODE_FRAGMENT = gql`
`;

export const AssetInsetForHoverEffect = styled.div`
padding: 10px 4px 2px 4px;
padding: 2px 4px 2px 4px;
height: 100%;
& *:focus {
Expand All @@ -283,7 +292,7 @@ export const AssetInsetForHoverEffect = styled.div`
export const AssetNodeContainer = styled.div<{$selected: boolean}>`
user-select: none;
cursor: pointer;
padding: 6px;
padding: 0 6px;
overflow: clip;
`;

Expand All @@ -307,6 +316,7 @@ export const AssetNodeBox = styled.div<{
background: ${Colors.backgroundDefault()};
border-radius: 10px;
position: relative;
margin: 6px 0;
transition: all 150ms linear;
&:hover {
${(p) => !p.$selected && `border: 2px solid ${Colors.lineageNodeBorderHover()};`};
Expand All @@ -322,6 +332,7 @@ export const AssetNodeBox = styled.div<{
const NameCSS: CSSObject = {
padding: '3px 0 3px 6px',
color: Colors.textDefault(),
fontSize: 14,
fontFamily: FontFamily.monospace,
fontWeight: 600,
};
Expand Down
52 changes: 41 additions & 11 deletions js_modules/dagster-ui/packages/ui-core/src/asset-graph/__fixtures__/AssetNode.fixtures.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,7 @@ export const AssetNodeFragmentBasic: AssetNodeFragment = buildAssetNode({
jobNames: ['job1'],
opNames: ['asset1'],
opVersion: '1',
changedReasons: [
ChangeReason.NEW,
ChangeReason.CODE_VERSION,
ChangeReason.DEPENDENCIES,
ChangeReason.PARTITIONS_DEFINITION,
ChangeReason.TAGS,
ChangeReason.METADATA,
ChangeReason.REMOVED,
],
changedReasons: [],
});

export const AssetNodeFragmentSource = buildAssetNode({
Expand All @@ -88,13 +80,28 @@ export const AssetNodeFragmentSource = buildAssetNode({
});

export const AssetNodeFragmentSourceOverdue = buildAssetNode({
...AssetNodeFragmentBasic,
isMaterializable: false,
isObservable: false,
freshnessInfo: buildAssetFreshnessInfo({
currentMinutesLate: 12,
}),
});

export const AssetNodeFragmentChangedInBranch = buildAssetNode({
...AssetNodeFragmentBasic,
kinds: ['sql'],
changedReasons: [
ChangeReason.NEW,
ChangeReason.CODE_VERSION,
ChangeReason.DEPENDENCIES,
ChangeReason.PARTITIONS_DEFINITION,
ChangeReason.TAGS,
ChangeReason.METADATA,
ChangeReason.REMOVED,
],
});

export const AssetNodeFragmentPartitioned: AssetNodeFragment = buildAssetNode({
...AssetNodeFragmentBasic,
assetKey: buildAssetKey({path: ['asset_partioned']}),
Expand Down Expand Up @@ -789,6 +796,12 @@ export const AssetNodeScenariosBase = [
definition: AssetNodeFragmentBasic,
expectedText: ['Materialized', 'Checks'],
},
{
title: 'Changed in Branch',
liveData: LiveDataForNodeMaterializedWithChecks,
definition: AssetNodeFragmentChangedInBranch,
expectedText: ['New in branch'],
},
];

export const AssetNodeScenariosSource = [
Expand All @@ -801,10 +814,11 @@ export const AssetNodeScenariosSource = [

{
title: 'Source Asset - Not Observable',
liveData: undefined,
liveData: LiveDataForNodeNeverMaterialized,
definition: {
...AssetNodeFragmentSource,
isObservable: false,
isExecutable: false,
id: '["source_asset_no"]',
assetKey: buildAssetKey({path: ['source_asset_no']}),
},
Expand All @@ -813,10 +827,11 @@ export const AssetNodeScenariosSource = [

{
title: 'Source Asset - Not Observable, No Description',
liveData: undefined,
liveData: LiveDataForNodeNeverMaterialized,
definition: {
...AssetNodeFragmentSource,
isObservable: false,
isExecutable: false,
description: null,
id: '["source_asset_nono"]',
assetKey: buildAssetKey({path: ['source_asset_nono']}),
Expand Down Expand Up @@ -902,12 +917,27 @@ export const AssetNodeScenariosPartitioned = [
expectedText: ['1,500 partitions', 'All'],
},

{
title: 'Partitioned Asset - Checks and Tags',
liveData: {
...LiveDataForNodePartitionedFresh,
assetChecks: LiveDataForNodeMaterializedWithChecks.assetChecks,
},
definition: {
...AssetNodeFragmentPartitioned,
changedReasons: [ChangeReason.NEW],
kinds: ['ipynb'],
},
expectedText: ['1,500 partitions', 'All', 'Checks'],
},

{
title: 'Partitioned Asset - Last Run Failed',
liveData: LiveDataForNodePartitionedLatestRunFailed,
definition: AssetNodeFragmentPartitioned,
expectedText: ['4', '999+', '1,500 partitions'],
},

{
title: 'Partitioned Asset - Live Data Loading',
liveData: undefined,
Expand Down
21 changes: 15 additions & 6 deletions js_modules/dagster-ui/packages/ui-core/src/asset-graph/__stories__/AssetNode.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,32 @@ export const LiveStates = () => {
<>
<SetCacheEntry />
<Box flex={{direction: 'column', gap: 8, alignItems: 'flex-start'}}>
<code>
<strong>{scenario.title}</strong>
</code>
<div
style={{
position: 'relative',
width: dimensions.width,
height: dimensions.height,
background: `linear-gradient(to bottom, rgba(100,100,100,0.15) 49%, rgba(100,100,100,1) 50%, rgba(100,100,100,0.15) 51%)`,
overflowY: 'hidden',
}}
>
<AssetNode definition={definitionCopy} selected={false} />
</div>
<div style={{position: 'relative', width: dimensions.width, height: 104}}>
<div style={{position: 'absolute', width: dimensions.width}}>
<div
style={{
position: 'relative',
width: dimensions.width,
background: `rgba(100,100,100,0.15)`,
height: 90,
}}
>
<div style={{position: 'absolute', width: dimensions.width, transform: 'scale(0.4)'}}>
<AssetNodeMinimal definition={definitionCopy} selected={false} height={82} />
</div>
</div>
<code>
<strong>{scenario.title}</strong>
</code>
</Box>
</>
);
Expand Down Expand Up @@ -109,7 +117,7 @@ export const Links = () => {
};
export const PartnerTags = () => {
const caseWithComputeKind = (computeKind: string) => {
const def = {...Mocks.AssetNodeFragmentBasic, computeKind};
const def = {...Mocks.AssetNodeFragmentBasic, kinds: [computeKind]};
const liveData = Mocks.LiveDataForNodeMaterialized;

function SetCacheEntry() {
Expand Down Expand Up @@ -142,6 +150,7 @@ export const PartnerTags = () => {
width: 280,
height: dimensions.height,
overflowY: 'hidden',
background: `linear-gradient(to bottom, transparent 49%, gray 50%, transparent 51%)`,
}}
>
<AssetNode definition={def} selected={false} />
Expand Down
34 changes: 16 additions & 18 deletions js_modules/dagster-ui/packages/ui-core/src/asset-graph/layout.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const Config = {
nodesep: -10,
nodeHeight: 'auto',
groupPaddingTop: 65,
groupPaddingBottom: -15,
groupPaddingBottom: -4,
groupRendering: 'if-varied',
clusterpaddingtop: 100,
},
Expand All @@ -86,7 +86,7 @@ export const Config = {
edgesep: 10,
nodeHeight: 'auto',
groupPaddingTop: 55,
groupPaddingBottom: -5,
groupPaddingBottom: -4,
groupRendering: 'if-varied',
},
};
Expand Down Expand Up @@ -338,7 +338,10 @@ export const extendBounds = (a: IBounds, b: IBounds) => {
};

export const ASSET_NODE_WIDTH = 320;
export const ASSET_NODE_NAME_MAX_LENGTH = 38;
export const ASSET_NODE_TAGS_HEIGHT = 28;
export const ASSET_NODE_STATUS_ROW_HEIGHT = 25;

export const ASSET_NODE_NAME_MAX_LENGTH = 31;

export const getAssetNodeDimensions = (def: {
assetKey: {path: string[]};
Expand All @@ -351,24 +354,19 @@ export const getAssetNodeDimensions = (def: {
computeKind: string | null;
changedReasons?: ChangeReason[];
}) => {
const width = ASSET_NODE_WIDTH;
let height = 0;

let height = 106; // top tags area + name + description
height += ASSET_NODE_TAGS_HEIGHT; // top tags

if (!def.isMaterializable && def.isObservable) {
height += 30; // status row
} else {
height += 28; // status row
height += 28; // checks row
if (def.isPartitioned) {
height += 52;
}
}
if (def.changedReasons?.length) {
height += 30;
height += 76; // box padding + border + name + description

if (def.isPartitioned && def.isMaterializable) {
height += ASSET_NODE_STATUS_ROW_HEIGHT;
}

height += 36; // tags beneath
height += ASSET_NODE_STATUS_ROW_HEIGHT; // status row
height += ASSET_NODE_STATUS_ROW_HEIGHT; // checks row
height += ASSET_NODE_TAGS_HEIGHT; // bottom tags

return {width, height};
return {width: ASSET_NODE_WIDTH, height};
};
2 changes: 1 addition & 1 deletion js_modules/dagster-ui/packages/ui-core/src/graph/asyncGraphLayout.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const _assetLayoutCacheKey = (graphData: GraphData, opts: LayoutAssetGraphOption
}

return `${JSON.stringify(opts)}${JSON.stringify({
version: 2,
version: 3,
downstream: recreateObjectWithKeysSorted(graphData.downstream),
upstream: recreateObjectWithKeysSorted(graphData.upstream),
nodes: Object.keys(graphData.nodes)
Expand Down

0 comments on commit 1f4dece

Please sign in to comment.