Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ui] Polish runs feed UI #25712

Merged
merged 4 commits into from
Nov 7, 2024
Merged

[ui] Polish runs feed UI #25712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1edaa2
[ui] Polish runs feed UI
Nov 3, 2024
e06149c
PR feedback
Nov 5, 2024
de7719f
PR feedback, better fix than “page size= 29”
Nov 5, 2024
d81f5a3
fix circular import
Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions js_modules/dagster-ui/packages/ui-components/src/components/PageHeader.tsx
View file
Open in desktop
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
16 changes: 9 additions & 7 deletions js_modules/dagster-ui/packages/ui-core/src/assets/AssetNodeOverview.tsx
View file
Open in desktop
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}}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a drive-by fix for this scenario:

image

after:

image

Copy link
Member

Choose a reason for hiding this comment

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

I feel like it would be nice to have a mini-alert component, these always feel too big for sidebars.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I agree, there's at least one that appears in the asset graph's sidebar and I think in that scenario we place it at the top with no whitespace around it all. Will see if Josh has thoughts about this in a follow-up

<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
7 changes: 5 additions & 2 deletions js_modules/dagster-ui/packages/ui-core/src/instance/backfill/ExecutionTimeline.tsx
View file
Open in desktop
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
10 changes: 6 additions & 4 deletions js_modules/dagster-ui/packages/ui-core/src/runs/AssetTagCollections.tsx
View file
Open in desktop
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
1 change: 1 addition & 0 deletions js_modules/dagster-ui/packages/ui-core/src/runs/CreatedByTag.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion js_modules/dagster-ui/packages/ui-core/src/runs/RunTargetLink.tsx
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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