From ebfc8fcb2d0d2313b8ecd3dbed6034ee9588fcca Mon Sep 17 00:00:00 2001 From: Claire Lin Date: Tue, 5 Mar 2024 15:19:12 -0800 Subject: [PATCH] update per pr feedback --- .../ui-components/src/components/BaseTag.tsx | 12 +- .../ui-core/src/assets/AssetPageHeader.tsx | 9 +- .../ui-core/src/assets/AssetsOverview.tsx | 156 +++++++++--------- .../packages/ui-core/src/graph/OpTags.tsx | 6 +- .../packages/ui-core/src/runs/UserDisplay.tsx | 21 +-- 5 files changed, 90 insertions(+), 114 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-components/src/components/BaseTag.tsx b/js_modules/dagster-ui/packages/ui-components/src/components/BaseTag.tsx index 5e62dd7952013..fade0b5caf56c 100644 --- a/js_modules/dagster-ui/packages/ui-components/src/components/BaseTag.tsx +++ b/js_modules/dagster-ui/packages/ui-components/src/components/BaseTag.tsx @@ -13,7 +13,6 @@ interface Props { rightIcon?: React.ReactNode; label?: React.ReactNode; tooltipText?: string; - fontSize?: string; } const BaseTagTooltipStyle: React.CSSProperties = { @@ -38,15 +37,9 @@ export const BaseTag = (props: Props) => { rightIcon, label, tooltipText, - fontSize = '12px', } = props; return ( - + {icon || null} {label !== undefined && label !== null ? ( ` @@ -79,7 +71,7 @@ export const StyledTag = styled.div` cursor: ${({$interactive}) => ($interactive ? 'pointer' : 'inherit')}; display: inline-flex; flex-direction: row; - font-size: ${({$fontSize}) => $fontSize}; + font-size: 12px; line-height: 16px; align-items: center; padding: 4px 8px; diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/AssetPageHeader.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/AssetPageHeader.tsx index 408b9963bdf54..9c085b5e8ac37 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/assets/AssetPageHeader.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/assets/AssetPageHeader.tsx @@ -16,6 +16,7 @@ import styled from 'styled-components'; import {showSharedToaster} from '../app/DomUtils'; import {useCopyToClipboard} from '../app/browser'; +import {AnchorButton} from '../ui/AnchorButton'; type Props = {assetKey: {path: string[]}} & Partial>; @@ -116,11 +117,9 @@ export const AssetGlobalLineageLink = () => ( ); export const AssetGlobalLineageButton = () => ( - - - + } to="/asset-groups"> + View global asset lineage + ); const BreadcrumbsWithSlashes = styled(Breadcrumbs)` diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/AssetsOverview.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/AssetsOverview.tsx index 4f2b53e43382c..714e7b2ac890f 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/assets/AssetsOverview.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/assets/AssetsOverview.tsx @@ -1,5 +1,7 @@ import {useQuery} from '@apollo/client'; import {Box, Colors, Heading, Icon, Page, Spinner, TextInput} from '@dagster-io/ui-components'; +import qs from 'qs'; +import {useContext} from 'react'; import {Link, useParams} from 'react-router-dom'; import styled from 'styled-components'; @@ -11,19 +13,17 @@ import { AssetCatalogTableQueryVariables, } from './types/AssetsCatalogTable.types'; import {useTrackPageView} from '../app/analytics'; +import {TimeContext} from '../app/time/TimeContext'; +import {browserTimezone} from '../app/time/browserTimezone'; import {TagIcon} from '../graph/OpTags'; import {useDocumentTitle} from '../hooks/useDocumentTitle'; import {useLaunchPadHooks} from '../launchpad/LaunchpadHooksContext'; import {ReloadAllButton} from '../workspace/ReloadAllButton'; +import {buildRepoPathForHuman} from '../workspace/buildRepoAddress'; import {repoAddressAsHumanString, repoAddressAsURLString} from '../workspace/repoAddressAsString'; +import {repoAddressFromPath} from '../workspace/repoAddressFromPath'; import {RepoAddress} from '../workspace/types'; -const ViewAllAssetsLink = () => ( - - View all - -); - type AssetCountsResult = { countsByOwner: Record; countsByComputeKind: Record; @@ -53,21 +53,23 @@ function buildAssetCountBySection(assets: AssetTableFragment[]): AssetCountsResu const assetCountByGroup: Record = {}; const assetCountByCodeLocation: Record = {}; - assets.forEach((asset) => { - if (asset.definition) { - asset.definition.owners.forEach((owner) => { + assets + .filter((asset) => asset.definition) + .forEach((asset) => { + const assetDefinition = asset.definition!; + assetDefinition.owners.forEach((owner) => { const ownerKey = owner.__typename === 'UserAssetOwner' ? owner.email : owner.team; assetCountByOwner[ownerKey] = (assetCountByOwner[ownerKey] || 0) + 1; }); - const computeKind = asset.definition.computeKind; + const computeKind = assetDefinition.computeKind; if (computeKind) { assetCountByComputeKind[computeKind] = (assetCountByComputeKind[computeKind] || 0) + 1; } - const groupName = asset.definition.groupName; - const locationName = asset.definition.repository.location.name; - const repositoryName = asset.definition.repository.name; + const groupName = assetDefinition.groupName; + const locationName = assetDefinition.repository.location.name; + const repositoryName = assetDefinition.repository.name; if (groupName) { const metadata: GroupMetadata = { @@ -79,33 +81,23 @@ function buildAssetCountBySection(assets: AssetTableFragment[]): AssetCountsResu assetCountByGroup[groupIdentifier] = (assetCountByGroup[groupIdentifier] || 0) + 1; } - const stringifiedCodeLocation = JSON.stringify({ - name: repositoryName, - location: locationName, - }); + const stringifiedCodeLocation = buildRepoPathForHuman(repositoryName, locationName); assetCountByCodeLocation[stringifiedCodeLocation] = (assetCountByCodeLocation[stringifiedCodeLocation] || 0) + 1; - } - }); - - const countPerAssetGroup: CountPerGroupName[] = []; - Object.entries(assetCountByGroup).forEach(([groupIdentifier, count]) => { - const metadata: GroupMetadata = JSON.parse(groupIdentifier); - countPerAssetGroup.push({ - assetCount: count, - groupMetadata: metadata, }); - }); + + const countPerAssetGroup = Object.entries(assetCountByGroup).map(([groupIdentifier, count]) => ({ + assetCount: count, + groupMetadata: JSON.parse(groupIdentifier), + })); return { countsByOwner: assetCountByOwner, countsByComputeKind: assetCountByComputeKind, countPerAssetGroup, - countPerCodeLocation: Object.entries(assetCountByCodeLocation).reduce( - (a, [stringifiedRepoLocation, count]) => - a.concat({repoAddress: JSON.parse(stringifiedRepoLocation), assetCount: count}), - [] as CountPerCodeLocation[], - ), + countPerCodeLocation: Object.entries(assetCountByCodeLocation).map(([key, count]) => { + return {repoAddress: repoAddressFromPath(key)!, assetCount: count}; + }), }; } @@ -114,8 +106,14 @@ interface AssetOverviewCategoryProps { assetsCount: number; } -function getGreeting() { - const hour = new Date().getHours(); +function getGreeting(timezone: string) { + const hour = Number( + new Date().toLocaleTimeString('en-US', { + hour: '2-digit', + hourCycle: 'h23', + timeZone: timezone === 'Automatic' ? browserTimezone() : timezone, + }), + ); if (hour < 12) { return 'Good morning'; } else if (hour < 18) { @@ -144,10 +142,7 @@ const SectionHeader = ({sectionName}: {sectionName: string}) => { {sectionName} @@ -166,15 +161,17 @@ const SectionBody = ({children}: {children: React.ReactNode}) => { ); }; -const LinkToAssetGraphGroup = (groupMetadata: GroupMetadata) => { - return `/asset-groups?groups=[${encodeURIComponent(JSON.stringify(groupMetadata))}]`; +const linkToAssetGraphGroup = (groupMetadata: GroupMetadata) => { + return `/asset-groups?${qs.stringify({groups: JSON.stringify([groupMetadata])})}`; }; -const LinkToAssetGraphComputeKind = (computeKind: string) => { - return `/asset-groups?computeKindTags=${encodeURIComponent(JSON.stringify([computeKind]))}`; +const linkToAssetGraphComputeKind = (computeKind: string) => { + return `/asset-groups?${qs.stringify({ + computeKindTags: JSON.stringify([computeKind]), + })}`; }; -const LinkToCodeLocation = (repoAddress: RepoAddress) => { +const linkToCodeLocation = (repoAddress: RepoAddress) => { return `/locations/${repoAddressAsURLString(repoAddress)}/assets`; }; @@ -201,6 +198,9 @@ export const AssetsOverview = ({viewerName}: {viewerName?: string}) => { : [], ); const {UserDisplay} = useLaunchPadHooks(); + const { + timezone: [timezone], + } = useContext(TimeContext); if (assetsQuery.loading) { return ( @@ -234,11 +234,11 @@ export const AssetsOverview = ({viewerName}: {viewerName?: string}) => { - {getGreeting()} + {getGreeting(timezone)} {viewerName ? `, ${viewerName}` : ''} - - + + View all @@ -250,9 +250,9 @@ export const AssetsOverview = ({viewerName}: {viewerName?: string}) => { <> - {Object.entries(assetCountBySection.countsByOwner).map(([label, count], idx) => ( - - + {Object.entries(assetCountBySection.countsByOwner).map(([label, count]) => ( + + ))} @@ -262,16 +262,14 @@ export const AssetsOverview = ({viewerName}: {viewerName?: string}) => { <> - {Object.entries(assetCountBySection.countsByComputeKind).map( - ([label, count], idx) => ( - - - - {label} - - - ), - )} + {Object.entries(assetCountBySection.countsByComputeKind).map(([label, count]) => ( + + + + {label} + + + ))} )} @@ -279,21 +277,22 @@ export const AssetsOverview = ({viewerName}: {viewerName?: string}) => { <> - {assetCountBySection.countPerAssetGroup.map( - (assetGroupCount: CountPerGroupName, idx) => ( - - - - - {assetGroupCount.groupMetadata.groupName} - - - {assetGroupCount.groupMetadata.repositoryLocationName} - - - - ), - )} + {assetCountBySection.countPerAssetGroup.map((assetGroupCount) => ( + + + + + {assetGroupCount.groupMetadata.groupName} + + + {assetGroupCount.groupMetadata.repositoryLocationName} + + + + ))} )} @@ -301,11 +300,14 @@ export const AssetsOverview = ({viewerName}: {viewerName?: string}) => { <> - {assetCountBySection.countPerCodeLocation.map((countPerCodeLocation, idx) => ( - + {assetCountBySection.countPerCodeLocation.map((countPerCodeLocation) => ( + - + {repoAddressAsHumanString(countPerCodeLocation.repoAddress)} diff --git a/js_modules/dagster-ui/packages/ui-core/src/graph/OpTags.tsx b/js_modules/dagster-ui/packages/ui-core/src/graph/OpTags.tsx index 308005e9a6572..2b649d9088697 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/graph/OpTags.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/graph/OpTags.tsx @@ -616,8 +616,8 @@ export const OpTags = React.memo(({tags, style, reduceColor, reduceText}: OpTags ); }); -export const TagIcon = React.memo(({tag_label}: {tag_label: string}) => { - const known = KNOWN_TAGS[coerceToStandardLabel(tag_label) as keyof typeof KNOWN_TAGS]; +export const TagIcon = React.memo(({tagLabel}: {tagLabel: string}) => { + const known = KNOWN_TAGS[coerceToStandardLabel(tagLabel) as keyof typeof KNOWN_TAGS]; const color = known?.color || null; const reversed = known && 'reversed' in known ? known.reversed : false; if (known && 'icon' in known) { @@ -628,7 +628,7 @@ export const TagIcon = React.memo(({tag_label}: {tag_label: string}) => { $img={known.icon.src} $color={reversed ? Colors.accentPrimary() : color} $rotation={null} - aria-label={tag_label} + aria-label={tagLabel} /> ); } diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/UserDisplay.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/UserDisplay.tsx index ce1009562ceef..a97ff0e9f9a3d 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/UserDisplay.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/UserDisplay.tsx @@ -10,19 +10,7 @@ type Props = { * Can be overridden using `LaunchpadHooksContext` * This is primarily used to display users in filter dropdown + users in table cells */ -export function UserDisplay({email, isFilter, size}: Props) { - let fontSize; - switch (size) { - case 'small': - fontSize = '14px'; - break; - case 'normal': - fontSize = '14px'; - break; - default: - fontSize = '12px'; - } - +export function UserDisplay({email, isFilter}: Props) { const icon = ; return isFilter ? ( @@ -30,11 +18,6 @@ export function UserDisplay({email, isFilter, size}: Props) { {email} ) : ( - {icon}} - label={email} - fontSize={fontSize} - /> + {icon}} label={email} /> ); }