Skip to content

Commit

Permalink
[ui] For runs targeting assets, show >1 asset in the Target column (#…
Browse files Browse the repository at this point in the history
…26122)

We’ve had reports that users who launch small N numbers of assets
together find the new run feed UI difficult to use, since it shows
either “asset name” or “2 assets” in the Target column. The "2 assets"
form makes it difficult to scan for a run you have in mind:

<img width="1005" alt="image"
src="https://github.com/user-attachments/assets/fa2dcd55-5ae9-47a4-b402-e54fc67ec396">


This PR:
- Makes the target column a bit wider
- Shows as many asset / check tags as will fit in the available space,
and then “X more”

The implementation of this uses a new hook
`useAdjustChildVisibilityToFill`. The idea is that your component
renders some reasonable max number of tags (10) and a more tag, and the
hook uses a resize observer + layout effect to show/hide the tags to fit
the available space. I tried doing this using React state, but it looks
bad because you can see it adding / removing tags, especially as you
resize the viewport. I think the other approach would be to write a tag
“measure” function, or otherwise repeatedly render + size them
offscreen, but that’d still force layouts with the added cost of a
React.render (the tags are not identical react components - the “4 more”
tag is slightly different - so I think the offscreen approach would need
to actually render the tag...) The text + size of the "more" tag also
changes based on the number of tags displayed.

Sidenote: There’s a bunch of cruft here because the “Target” column
components all have to support a “tags” rendering and a “plain”
rendering. This is going away soon when we remove the FF allowing users
to revert to the old runs page, and I think it’ll clean up this code!

## How I Tested These Changes

I added a storybook that makes it easy to test what this looks like in a
bunch of scenarios and verify it works nicely.


https://github.com/user-attachments/assets/8dc2afdb-e15c-4fab-a139-eb5698428bf1


Before:

<img width="1191" alt="image"
src="https://github.com/user-attachments/assets/cbd74932-1b22-4ffa-9c03-9375980cf1d9">
<img width="1186" alt="image"
src="https://github.com/user-attachments/assets/926a1075-4aac-404e-9952-798df82d8184">

After:

<img width="1189" alt="image"
src="https://github.com/user-attachments/assets/7e33c045-43ba-4370-a03f-a4c505b3606b">

<img width="1183" alt="image"
src="https://github.com/user-attachments/assets/6e81a1d8-1a91-4b5e-b87f-dfdbb01b102e">


Related:
https://linear.app/dagster-labs/issue/FE-702/show-more-than-one-asset-tag-on-the-new-runs-feed-page

Co-authored-by: bengotow <[email protected]>
  • Loading branch information
bengotow and bengotow authored Dec 11, 2024
1 parent 0fe2d5e commit 5b628db
Show file tree
Hide file tree
Showing 9 changed files with 452 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const BackfillOpJobPartitionsTable = ({
<tr>
<td>
<Box flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'baseline'}}>
<BackfillTarget backfill={backfill} repoAddress={repoAddress} />
<BackfillTarget backfill={backfill} repoAddress={repoAddress} useTags={false} />
<StatusBar
targeted={results.length}
inProgress={inProgress.length}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {FIFTEEN_SECONDS, useQueryRefreshAtInterval} from '../../app/QueryRefresh
import {isHiddenAssetGroupJob} from '../../asset-graph/Utils';
import {RunStatus} from '../../graphql/types';
import {PartitionStatus, PartitionStatusHealthSourceOps} from '../../partitions/PartitionStatus';
import {PipelineReference} from '../../pipelines/PipelineReference';
import {PipelineReference, PipelineTag} from '../../pipelines/PipelineReference';
import {AssetKeyTagCollection} from '../../runs/AssetTagCollections';
import {CreatedByTagCell} from '../../runs/CreatedByTag';
import {getBackfillPath} from '../../runs/RunsFeedUtils';
Expand Down Expand Up @@ -118,7 +118,7 @@ export const BackfillRowContent = ({
</td>
{showBackfillTarget ? (
<td style={{width: '20%'}}>
<BackfillTarget backfill={backfill} repoAddress={repoAddress} />
<BackfillTarget backfill={backfill} repoAddress={repoAddress} useTags={false} />
</td>
) : null}
<td style={{width: allPartitions ? 300 : 140}}>
Expand Down Expand Up @@ -146,9 +146,11 @@ export const BackfillRowContent = ({
export const BackfillTarget = ({
backfill,
repoAddress,
useTags,
}: {
backfill: Pick<BackfillTableFragment, 'assetSelection' | 'partitionSet' | 'partitionSetName'>;
repoAddress: RepoAddress | null;
useTags: boolean;
}) => {
const repo = useRepository(repoAddress);
const {assetSelection, partitionSet, partitionSetName} = backfill;
Expand All @@ -160,22 +162,30 @@ export const BackfillTarget = ({
return null;
}
if (partitionSet && repo) {
const link = workspacePipelinePath({
repoName: partitionSet.repositoryOrigin.repositoryName,
repoLocation: partitionSet.repositoryOrigin.repositoryLocationName,
pipelineName: partitionSet.pipelineName,
isJob: isThisThingAJob(repo, partitionSet.pipelineName),
path: `/partitions?partitionSet=${encodeURIComponent(partitionSet.name)}`,
});
if (useTags) {
return (
<Tag icon="partition_set">
<Link to={link}>{partitionSet.name}</Link>
</Tag>
);
}
return (
<Link
style={{fontWeight: 500}}
to={workspacePipelinePath({
repoName: partitionSet.repositoryOrigin.repositoryName,
repoLocation: partitionSet.repositoryOrigin.repositoryLocationName,
pipelineName: partitionSet.pipelineName,
isJob: isThisThingAJob(repo, partitionSet.pipelineName),
path: `/partitions?partitionSet=${encodeURIComponent(partitionSet.name)}`,
})}
>
<Link style={{fontWeight: 500}} to={link}>
{partitionSet.name}
</Link>
);
}
if (partitionSetName) {
if (useTags) {
return <Tag icon="partition_set">{partitionSetName}</Tag>;
}
return <span style={{fontWeight: 500}}>{partitionSetName}</span>;
}
return null;
Expand All @@ -193,10 +203,28 @@ export const BackfillTarget = ({

const buildPipelineOrAssets = () => {
if (assetSelection?.length) {
return <AssetKeyTagCollection assetKeys={assetSelection} dialogTitle="Assets in backfill" />;
return (
<AssetKeyTagCollection
assetKeys={assetSelection}
dialogTitle="Assets in backfill"
useTags={useTags}
maxRows={2}
/>
);
}
if (partitionSet && repo) {
return (
return useTags ? (
<PipelineTag
showIcon
size="small"
pipelineName={partitionSet.pipelineName}
pipelineHrefContext={{
name: partitionSet.repositoryOrigin.repositoryName,
location: partitionSet.repositoryOrigin.repositoryLocationName,
}}
isJob={isThisThingAJob(repo, partitionSet.pipelineName)}
/>
) : (
<PipelineReference
showIcon
size="small"
Expand All @@ -215,11 +243,11 @@ export const BackfillTarget = ({
const repoLink = buildRepoLink();
const pipelineOrAssets = buildPipelineOrAssets();
return (
<Box flex={{direction: 'column', gap: 8}}>
<Box flex={{direction: 'column', gap: 8, alignItems: 'start'}}>
{buildHeader()}
{(pipelineOrAssets || repoLink) && (
<Box flex={{direction: 'column', gap: 4}} style={{fontSize: '12px'}}>
{repoLink}
{repoLink && useTags ? <Tag>{repoLink}</Tag> : repoLink}
{pipelineOrAssets}
</Box>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {Box, Colors, Icon, Tag} from '@dagster-io/ui-components';
import {Link} from 'react-router-dom';
import styled from 'styled-components';

import {PipelineSnapshotLink} from './PipelinePathUtils';
import {RepoAddress} from '../workspace/types';
Expand Down Expand Up @@ -54,7 +53,7 @@ export const PipelineReference = ({
return (
<Box flex={{direction: 'row', alignItems: 'center', display: 'inline-flex'}}>
{showIcon && (
<Box margin={{right: 8}}>
<Box margin={{right: 4}}>
<Icon color={Colors.accentGray()} name="job" />
</Box>
)}
Expand All @@ -71,16 +70,8 @@ export const PipelineReference = ({

export const PipelineTag = (props: Props) => {
return (
<PipelineTagWrap>
<Tag tooltipText={props.pipelineName}>
<PipelineReference {...props} />
</Tag>
</PipelineTagWrap>
<Tag tooltipText={props.pipelineName} icon="job">
<PipelineReference {...props} showIcon={false} />
</Tag>
);
};

const PipelineTagWrap = styled.span`
span {
line-height: 0;
}
`;
Loading

1 comment on commit 5b628db

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-llgprt84y-elementl.vercel.app

Built with commit 5b628db.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.