Skip to content

Commit

Permalink
[ui] Show tooltips on metadata entries explaining the data origin (#2…
Browse files Browse the repository at this point in the history
…1322)

## Summary & Motivation

Fixes FE-215

This adds tooltips that make it more obvious that the asset overview's
metadata table is combining metadata from different sources. Previously
you had to look at the icons and understand what they meant:

<img width="585" alt="image"
src="https://github.com/dagster-io/dagster/assets/1037212/4676c84e-a737-42e9-888b-f6cff60d6cc9">

I also fixed some CSS issues with nested tables here. The CSS for the
asset metadata table (the outer one) was colliding with the styles of
the inner nested table, giving it double borders. I also applied the
compact style to the inner one, which I think looks better.

<img width="1208" alt="image"
src="https://github.com/dagster-io/dagster/assets/1037212/97e1666a-3c9a-442c-baab-68ed1e692818">

<img width="1210" alt="image"
src="https://github.com/dagster-io/dagster/assets/1037212/66319039-c665-4fba-b39c-f277cd5df17a">


## How I Tested These Changes

---------

Co-authored-by: bengotow <[email protected]>
  • Loading branch information
bengotow and bengotow authored Apr 22, 2024
1 parent 7fa91e4 commit d18f269
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const Table = styled(HTMLTable)<TableProps>`
& tr th,
& tr td {
border: none;
box-shadow:
inset 0 1px 0 ${Colors.keylineDefault()},
inset 1px 0 0 ${Colors.keylineDefault()} !important;
Expand All @@ -26,7 +27,7 @@ export const Table = styled(HTMLTable)<TableProps>`
font-family: ${FontFamily.default};
font-size: 12px;
font-weight: 400;
padding: ${({$compact}) => ($compact ? '0 8px' : ' 8px 12px')};
padding: ${({$compact}) => ($compact ? '4px 8px' : ' 8px 12px')};
min-height: 32px;
white-space: nowrap;
vertical-align: bottom;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,12 @@ export const AssetEventDetail = ({

<Box padding={{top: 24}} flex={{direction: 'column', gap: 8}}>
<Subheading>Metadata</Subheading>
<AssetEventMetadataEntriesTable assetKey={assetKey} event={event} showDescriptions />
<AssetEventMetadataEntriesTable
repoAddress={repoAddress}
assetKey={assetKey}
event={event}
showDescriptions
/>
</Box>

{event.__typename === 'MaterializationEvent' && (
Expand Down Expand Up @@ -192,7 +197,7 @@ export const AssetEventDetailEmpty = () => (

<Box padding={{top: 24}} flex={{direction: 'column', gap: 8}}>
<Subheading>Metadata</Subheading>
<AssetEventMetadataEntriesTable event={null} showDescriptions />
<AssetEventMetadataEntriesTable event={null} repoAddress={null} showDescriptions />
</Box>
</Box>
);
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Mono,
Tag,
TextInput,
Tooltip,
} from '@dagster-io/ui-components';
import dayjs from 'dayjs';
import uniqBy from 'lodash/uniqBy';
Expand All @@ -26,6 +27,8 @@ import {HIDDEN_METADATA_ENTRY_LABELS, MetadataEntry} from '../metadata/MetadataE
import {isCanonicalColumnLineageEntry, isCanonicalColumnSchemaEntry} from '../metadata/TableSchema';
import {MetadataEntryFragment} from '../metadata/types/MetadataEntryFragment.types';
import {titleForRun} from '../runs/RunUtils';
import {repoAddressAsHumanString} from '../workspace/repoAddressAsString';
import {RepoAddress} from '../workspace/types';

type TableEvent = Pick<
AssetObservationFragment | AssetMaterializationFragment,
Expand All @@ -37,6 +40,7 @@ type TableEvent = Pick<

interface Props {
assetKey?: AssetKey;
repoAddress: RepoAddress | null;
event: TableEvent | null;
observations?: TableEvent[] | null;
definitionMetadata?: MetadataEntryFragment[];
Expand Down Expand Up @@ -69,6 +73,7 @@ export const AssetEventMetadataEntriesTable = ({
hideTableSchema,
displayedByDefault = 100,
emptyState,
repoAddress,
}: Props) => {
const [filter, setFilter] = useState('');
const [displayedCount, setDisplayedCount] = useState(displayedByDefault);
Expand All @@ -83,6 +88,9 @@ export const AssetEventMetadataEntriesTable = ({
const allRows = useMemo(() => {
const eventRows = event
? event.metadataEntries.map((entry) => ({
tooltip: `Materialized ${dayjs(Number(event.timestamp)).fromNow()}${
event.runId ? ` in run ${event.runId?.slice(0, 8)}` : ``
}`,
icon: 'materialization' as const,
timestamp: event.timestamp,
runId: null,
Expand All @@ -92,6 +100,9 @@ export const AssetEventMetadataEntriesTable = ({

const observationRows = (observations || []).flatMap((o) =>
o.metadataEntries.map((entry) => ({
tooltip: `Observed ${dayjs(Number(o.timestamp)).fromNow()}${
o.runId ? ` in run ${o.runId.slice(0, 8)}` : ``
}`,
icon: 'observation' as const,
timestamp: o.timestamp,
runId: o.runId,
Expand All @@ -100,14 +111,17 @@ export const AssetEventMetadataEntriesTable = ({
);

const definitionRows = (definitionMetadata || []).map((entry) => ({
tooltip: `Loaded ${dayjs(definitionLoadTimestamp).fromNow()}${
repoAddress ? ` from ${repoAddressAsHumanString(repoAddress)}` : ''
}`,
icon: 'asset' as const,
timestamp: definitionLoadTimestamp,
runId: null,
entry,
}));

return uniqBy([...observationRows, ...eventRows, ...definitionRows], (e) => e.entry.label);
}, [definitionLoadTimestamp, definitionMetadata, event, observations]);
}, [definitionLoadTimestamp, definitionMetadata, event, observations, repoAddress]);

const filteredRows = useMemo(
() =>
Expand Down Expand Up @@ -186,40 +200,44 @@ export const AssetEventMetadataEntriesTable = ({
</td>
</tr>
)}
{filteredRows.slice(0, displayedCount).map(({entry, timestamp, runId, icon}) => (
<tr key={`metadata-${timestamp}-${entry.label}`}>
<td>
<Mono>{entry.label}</Mono>
</td>
{showTimestamps && (
{filteredRows
.slice(0, displayedCount)
.map(({entry, timestamp, runId, icon, tooltip}) => (
<tr key={`metadata-${timestamp}-${entry.label}`}>
<td>
<Tag>
<Box flex={{gap: 4, alignItems: 'center'}}>
<Icon name={icon} />
<Timestamp timestamp={{ms: Number(timestamp)}} />
</Box>
</Tag>
<Mono>{entry.label}</Mono>
</td>
)}
<td>
<Mono>
<MetadataEntry entry={entry} expandSmallValues={true} />
</Mono>
</td>
{showDescriptions && (
<td style={{opacity: 0.7}}>
{runId && (
<ObservedInRun
runId={runId}
timestamp={timestamp}
relativeTo={event?.timestamp}
/>
)}
{entry.description}
{showTimestamps && (
<td>
<Tooltip content={tooltip}>
<Tag>
<Box flex={{gap: 4, alignItems: 'center'}}>
<Icon name={icon} />
<Timestamp timestamp={{ms: Number(timestamp)}} />
</Box>
</Tag>
</Tooltip>
</td>
)}
<td>
<Mono>
<MetadataEntry entry={entry} expandSmallValues={true} />
</Mono>
</td>
)}
</tr>
))}
{showDescriptions && (
<td style={{opacity: 0.7}}>
{runId && (
<ObservedInRun
runId={runId}
timestamp={timestamp}
relativeTo={event?.timestamp}
/>
)}
{entry.description}
</td>
)}
</tr>
))}
</tbody>
</StyledTableWithHeader>
{displayedCount < filteredRows.length ? (
Expand Down Expand Up @@ -282,22 +300,24 @@ export const StyledTableWithHeader = styled.table`
border-spacing: 0;
border-collapse: collapse;
thead tr td {
& > thead > tr > td {
color: ${Colors.textLighter()};
font-size: 12px;
line-height: 16px;
}
tr td:first-child {
max-width: 300px;
word-wrap: break-word;
width: 25%;
}
tr td {
& > tbody > tr > td,
& > thead > tr > td {
border: 1px solid ${Colors.keylineDefault()};
padding: 8px 12px;
font-size: 14px;
line-height: 20px;
vertical-align: top;
&:first-child {
max-width: 300px;
word-wrap: break-word;
width: 25%;
}
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ export const AssetNodeOverview = ({
definitionMetadata={assetMetadata}
definitionLoadTimestamp={assetNodeLoadTimestamp}
assetHasDefinedPartitions={!!assetNode.partitionDefinition}
repoAddress={repoAddress}
event={materialization || observation || null}
emptyState={
<SectionEmptyState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ export const AssetPartitionDetail = ({
<AssetEventMetadataEntriesTable
event={latest}
observations={observationsAboutLatest}
repoAddress={repoAddress}
showDescriptions
/>
</Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const ViewDetailsButton = ({
setShowDetails(false);
}}
>
<AssetEventMetadataEntriesTable event={evaluation} showDescriptions />
<AssetEventMetadataEntriesTable showDescriptions event={evaluation} repoAddress={null} />
</Dialog>
<Button
onClick={() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ export const TableMetadataEntryComponent = ({entry}: {entry: TableMetadataEntry}
return (
<Box flex={{direction: 'column', gap: 8}}>
<MetadataEntryAction onClick={() => setShowSchema(true)}>Show schema</MetadataEntryAction>
<Table style={{borderRight: `1px solid ${Colors.keylineDefault()}`}}>
<Table style={{borderRight: `1px solid ${Colors.keylineDefault()}`}} $compact>
<thead>
<tr>
{schema.columns.map((column) => (
Expand Down

2 comments on commit d18f269

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

Built with commit d18f269.
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-storybook ready!

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

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

Please sign in to comment.