Skip to content

Commit

Permalink
[ui] Polish runs feed UI (#25712)
Browse files Browse the repository at this point in the history
## Summary & Motivation

Related:
https://linear.app/dagster-labs/issue/FE-515/backfills-and-runs-consolidation-ui-frontend

This is a collection of small tweaks to improve the UI/UX of the runs
feed pulled from this doc of dogfooding feedback:
https://www.notion.so/dagster/new-Runs-page-1-9-dogfooding-12918b92e462807688fee0e644a34641

- UI does not reset to page 1 when you change filters
- “Target” column occupies more space than necessary
- When paginating, should reset scroll to top
- Might be good to use middle-truncate or end-truncate on single
assets/jobs in the Target column
- Column cuts off “Launched by” content
- “Automation condition” tag tooltip is applied to the entire width of
the cell instead of just the tag, probably need a wrapper div on the
- During Loading, the header of the table is shown, then it disappears
if there are no results
- 26 runs per page?
- Run page header vertical padding is lost on narrow viewport
- Run timeline: use monospace font for run ID

## How I Tested These Changes

I tested all of these manually using the runs feed page and other
impacted pages (for the middle truncate items)

---------

Co-authored-by: bengotow <[email protected]>
  • Loading branch information
bengotow and bengotow authored Nov 7, 2024
1 parent 5a1c831 commit b3d14fe
Show file tree
Hide file tree
Showing 15 changed files with 199 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ export const PageHeader = (props: Props) => {
>
{title && (
<Box
padding={{vertical: 8}}
style={{minHeight: 52, alignContent: 'center'}}
flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'center', gap: 8}}
flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'center'}}
>
<Box flex={{direction: 'row', alignItems: 'center', gap: 12, wrap: 'wrap'}}>
{title}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,17 +317,19 @@ export const AssetNodeOverview = ({
</AttributeAndValue>

<AttributeAndValue label="Code location">
<Box flex={{direction: 'column'}}>
<Box flex={{direction: 'column', alignItems: 'flex-start', gap: 8}}>
<AssetDefinedInMultipleReposNotice
assetKey={cachedOrLiveAssetNode.assetKey}
loadedFromRepo={repoAddress!}
/>
<RepositoryLink repoAddress={repoAddress!} />
{location && (
<Caption color={Colors.textLighter()}>
Loaded {dayjs.unix(location.updatedTimestamp).fromNow()}
</Caption>
)}
<Box flex={{direction: 'column'}}>
<RepositoryLink repoAddress={repoAddress!} />
{location && (
<Caption color={Colors.textLighter()}>
Loaded {dayjs.unix(location.updatedTimestamp).fromNow()}
</Caption>
)}
</Box>
</Box>
</AttributeAndValue>
<AttributeAndValue label="Owners">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Box, Colors, Spinner, useViewport} from '@dagster-io/ui-components';
import {Box, Colors, Mono, Spinner, useViewport} from '@dagster-io/ui-components';
import {useVirtualizer} from '@tanstack/react-virtual';
import React from 'react';
import {Link} from 'react-router-dom';
Expand All @@ -12,6 +12,7 @@ import {
TimelineRowContainer,
} from '../../runs/RunTimeline';
import {TimelineRun} from '../../runs/RunTimelineTypes';
import {titleForRun} from '../../runs/RunUtils';
import {TimeElapsed} from '../../runs/TimeElapsed';
import {RunBatch, batchRunsForTimeline} from '../../runs/batchRunsForTimeline';
import {mergeStatusToBackground} from '../../runs/mergeStatusToBackground';
Expand Down Expand Up @@ -165,7 +166,9 @@ export const ExecutionTimelineRow = ({
>
<Box flex={{alignItems: 'center', gap: 4}}>
<RunStatusDot status={run.status} size={12} />
<Link to={`/runs/${run.id}`}>{run.id.slice(0, 8)}</Link>
<Link to={`/runs/${run.id}`}>
<Mono>{titleForRun(run)}</Mono>
</Link>
</Box>
<TimeElapsed startUnix={run.startTime / 1000} endUnix={run.endTime / 1000} />
</Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export const AssetKeyTagCollection = React.memo((props: AssetKeyTagCollectionPro
const assetKey = assetKeys[0]!;
return (
<TagActionsPopover
childrenMiddleTruncate
data={{key: '', value: ''}}
actions={[
{
Expand All @@ -105,13 +106,13 @@ export const AssetKeyTagCollection = React.memo((props: AssetKeyTagCollectionPro
>
{useTags ? (
<Tag intent="none" interactive icon="asset">
{displayNameForAssetKey(assetKey)}
<MiddleTruncate text={displayNameForAssetKey(assetKey)} />
</Tag>
) : (
<Link to={assetDetailsPathForKey(assetKey)}>
<Box flex={{direction: 'row', gap: 8, alignItems: 'center'}}>
<Icon color={Colors.accentGray()} name="asset" size={16} />
{displayNameForAssetKey(assetKey)}
<MiddleTruncate text={displayNameForAssetKey(assetKey)} />
</Box>
</Link>
)}
Expand Down Expand Up @@ -176,16 +177,17 @@ export const AssetCheckTagCollection = React.memo((props: AssetCheckTagCollectio
<TagActionsPopover
data={{key: '', value: ''}}
actions={[{label: 'View asset check', to: assetDetailsPathForAssetCheck(check)}]}
childrenMiddleTruncate
>
{useTags ? (
<Tag intent="none" interactive icon="asset_check">
{labelForAssetCheck(check)}
<MiddleTruncate text={labelForAssetCheck(check)} />
</Tag>
) : (
<Link to={assetDetailsPathForAssetCheck(check)}>
<Box flex={{direction: 'row', gap: 8, alignItems: 'center'}}>
<Icon color={Colors.accentGray()} name="asset_check" size={16} />
{labelForAssetCheck(check)}
<MiddleTruncate text={labelForAssetCheck(check)} />
</Box>
</Link>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export const CreatedByTag = ({repoAddress, tags, onAddTag}: Props) => {
return (
<TagActionsPopover
data={tag}
childrenMiddleTruncate
actions={[
{
label: 'Add to filter',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const RunTargetLink = ({
repoAddress: RepoAddress | null;
}) => {
return isHiddenAssetGroupJob(run.pipelineName) ? (
<Box flex={{gap: 16, alignItems: 'end', wrap: 'wrap'}}>
<Box flex={{gap: 16, alignItems: 'end', wrap: 'wrap'}} style={{minWidth: 0, maxWidth: '100%'}}>
<AssetKeyTagCollection assetKeys={assetKeysForRun(run)} />
<AssetCheckTagCollection assetChecks={run.assetCheckSelection} />
</Box>
Expand Down
69 changes: 7 additions & 62 deletions js_modules/dagster-ui/packages/ui-core/src/runs/RunsFeedRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,19 @@ import styled from 'styled-components';

import {CreatedByTagCell, CreatedByTagCellWrapper} from './CreatedByTag';
import {QueuedRunCriteriaDialog} from './QueuedRunCriteriaDialog';
import {RUN_ACTIONS_MENU_RUN_FRAGMENT, RunActionsMenu} from './RunActionsMenu';
import {RunActionsMenu} from './RunActionsMenu';
import {RunRowTags} from './RunRowTags';
import {RunStatusTag, RunStatusTagWithStats} from './RunStatusTag';
import {DagsterTag} from './RunTag';
import {RunTargetLink} from './RunTargetLink';
import {RunStateSummary, RunTime, titleForRun} from './RunUtils';
import {getBackfillPath} from './RunsFeedUtils';
import {RunFilterToken} from './RunsFilterInput';
import {gql} from '../apollo-client';
import {RunTimeFragment} from './types/RunUtils.types';
import {RunsFeedTableEntryFragment} from './types/RunsFeedRow.types';
import {RunsFeedTableEntryFragment} from './types/RunsFeedTableEntryFragment.types';
import {RunStatus} from '../graphql/types';
import {BackfillActionsMenu, backfillCanCancelRuns} from '../instance/backfill/BackfillActionsMenu';
import {BACKFILL_STEP_STATUS_DIALOG_BACKFILL_FRAGMENT} from '../instance/backfill/BackfillFragments';
import {BackfillTarget} from '../instance/backfill/BackfillRow';
import {PARTITION_SET_FOR_BACKFILL_TABLE_FRAGMENT} from '../instance/backfill/BackfillTable';
import {HeaderCell, HeaderRow, RowCell} from '../ui/VirtualizedTable';
import {appendCurrentQueryParams} from '../util/appendCurrentQueryParams';

Expand Down Expand Up @@ -104,6 +101,10 @@ export const RunsFeedRow = ({
flex={{direction: 'row', alignItems: 'center', wrap: 'wrap'}}
style={{gap: '4px 8px', lineHeight: 0}}
>
{entry.__typename === 'PartitionBackfill' ? (
<Tag intent="none">Backfill</Tag>
) : undefined}

<RunRowTags
run={{...entry, mode: 'default'}}
isJob={true}
Expand Down Expand Up @@ -187,7 +188,7 @@ export const RunsFeedRow = ({
};

const TEMPLATE_COLUMNS =
'60px minmax(0, 2fr) minmax(0, 2fr) minmax(0, 1fr) 140px 150px 120px 132px';
'60px minmax(0, 2fr) minmax(0, 1.2fr) minmax(0, 1fr) 140px 150px 120px 132px';

export const RunsFeedTableHeader = ({checkbox}: {checkbox: React.ReactNode}) => {
return (
Expand Down Expand Up @@ -217,59 +218,3 @@ const RowGrid = styled(Box)`
display: block;
}
`;

export const RUNS_FEED_TABLE_ENTRY_FRAGMENT = gql`
fragment RunsFeedTableEntryFragment on RunsFeedEntry {
__typename
id
runStatus
creationTime
startTime
endTime
tags {
key
value
}
jobName
assetSelection {
... on AssetKey {
path
}
}
assetCheckSelection {
name
assetKey {
path
}
}
... on Run {
repositoryOrigin {
id
repositoryLocationName
repositoryName
}
...RunActionsMenuRunFragment
}
... on PartitionBackfill {
backfillStatus: status
partitionSetName
partitionSet {
id
...PartitionSetForBackfillTableFragment
}
assetSelection {
path
}
hasCancelPermission
hasResumePermission
isAssetBackfill
numCancelable
...BackfillStepStatusDialogBackfillFragment
}
}
${RUN_ACTIONS_MENU_RUN_FRAGMENT}
${PARTITION_SET_FOR_BACKFILL_TABLE_FRAGMENT}
${BACKFILL_STEP_STATUS_DIALOG_BACKFILL_FRAGMENT}
`;
66 changes: 53 additions & 13 deletions js_modules/dagster-ui/packages/ui-core/src/runs/RunsFeedTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
ifPlural,
} from '@dagster-io/ui-components';
import {useVirtualizer} from '@tanstack/react-virtual';
import React, {useMemo, useRef} from 'react';
import React, {useEffect, useMemo, useRef} from 'react';

import {RunBulkActionsMenu} from './RunActionsMenu';
import {RunTableEmptyState} from './RunTableEmptyState';
Expand All @@ -19,7 +19,7 @@ import {RunFilterToken} from './RunsFilterInput';
import {
RunsFeedTableEntryFragment,
RunsFeedTableEntryFragment_Run,
} from './types/RunsFeedRow.types';
} from './types/RunsFeedTableEntryFragment.types';
import {useRunsFeedEntries} from './useRunsFeedEntries';
import {FIFTEEN_SECONDS, useQueryRefreshAtInterval} from '../app/QueryRefresh';
import {RunsFilter} from '../graphql/types';
Expand Down Expand Up @@ -81,6 +81,20 @@ export const RunsFeedTable = ({
);
const backfillsExcluded = selectedEntries.length - selectedRuns.length;

const resetScrollOnLoad = useRef(false);
useEffect(() => {
// When you click "Next page" from the bottom of page 1, we show the indeterminate
// loading state and want to scroll to the top when the new results arrive. It looks
// bad to do it immediately, and the `entries` can also change on their own (and
// sometimes with new rows), so we do this explicitly for pagination cases using a ref.
if (!loading && resetScrollOnLoad.current) {
resetScrollOnLoad.current = false;
if (parentRef.current) {
parentRef.current.scrollTop = 0;
}
}
}, [loading]);

const actionBar = (
<Box flex={{direction: 'column', gap: 8}}>
<Box
Expand All @@ -90,7 +104,23 @@ export const RunsFeedTable = ({
>
{actionBarComponents ?? <span />}
<Box flex={{gap: 12, alignItems: 'center'}} style={{marginRight: 8}}>
<CursorHistoryControls {...paginationProps} style={{marginTop: 0}} />
<CursorHistoryControls
style={{marginTop: 0}}
hasPrevCursor={paginationProps.hasPrevCursor}
hasNextCursor={paginationProps.hasNextCursor}
popCursor={() => {
resetScrollOnLoad.current = true;
paginationProps.popCursor();
}}
advanceCursor={() => {
resetScrollOnLoad.current = true;
paginationProps.advanceCursor();
}}
reset={() => {
resetScrollOnLoad.current = true;
paginationProps.reset();
}}
/>
<RunBulkActionsMenu
clearSelection={() => onToggleAll(false)}
selected={selectedRuns}
Expand Down Expand Up @@ -122,27 +152,37 @@ export const RunsFeedTable = ({
);

function content() {
const header = (
<RunsFeedTableHeader
checkbox={
<CheckAllBox
checkedCount={checkedIds.size}
totalCount={entries.length}
onToggleAll={onToggleAll}
/>
}
/>
);

if (entries.length === 0 && !loading) {
const anyFilter = !!Object.keys(filter || {}).length;
if (emptyState) {
return <>{emptyState()}</>;
}

return <RunTableEmptyState anyFilter={anyFilter} />;
return (
<div style={{overflow: 'hidden'}}>
{header}
<RunTableEmptyState anyFilter={anyFilter} />
</div>
);
}

return (
<div style={{overflow: 'hidden'}}>
<IndeterminateLoadingBar $loading={loading} />
<Container ref={parentRef} style={scroll ? {overflow: 'auto'} : {overflow: 'visible'}}>
<RunsFeedTableHeader
checkbox={
<CheckAllBox
checkedCount={checkedIds.size}
totalCount={entries.length}
onToggleAll={onToggleAll}
/>
}
/>
{header}
{entries.length === 0 && loading && (
<Box flex={{direction: 'row', justifyContent: 'center'}} padding={32}>
<SpinnerWithText label="Loading runs…" />
Expand Down
Loading

2 comments on commit b3d14fe

@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-storybook ready!

✅ Preview
https://dagit-storybook-9wf4tyg75-elementl.vercel.app

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

@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-8vvvwbjjm-elementl.vercel.app

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

Please sign in to comment.