Skip to content

Commit

Permalink
[dagit] Fix text overflow behavior in the left nav, keep open if one …
Browse files Browse the repository at this point in the history
…repo (#8324)

Co-authored-by: bengotow <bgotow@elementl.com>
  • Loading branch information
bengotow and bengotow committed Jun 10, 2022
1 parent 70558e1 commit e4a00e3
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 29 deletions.
14 changes: 9 additions & 5 deletions js_modules/dagit/packages/core/src/assets/AssetTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export const AssetTable = ({
const {canWipeAssets} = usePermissions();

const groupedByFirstComponent: {[pathComponent: string]: Asset[]} = {};
const checkedAssets: Asset[] = [];

assets.forEach((asset) => {
const displayPathKey = JSON.stringify(displayPathForAsset(asset));
Expand All @@ -76,9 +75,13 @@ export const AssetTable = ({
Object.keys(groupedByFirstComponent),
);

const checkedAssets: Asset[] = [];
const checkedPathsOnscreen: string[] = [];

const pageDisplayPathKeys = Object.keys(groupedByFirstComponent).sort().slice(0, maxDisplayCount);
pageDisplayPathKeys.forEach((pathKey) => {
if (checkedPaths.has(pathKey)) {
checkedPathsOnscreen.push(pathKey);
checkedAssets.push(...(groupedByFirstComponent[pathKey] || []));
}
});
Expand All @@ -94,7 +97,7 @@ export const AssetTable = ({
{checkedAssets.some((c) => !c.definition) ? (
<Tooltip content="One or more selected assets are not software-defined and cannot be launched directly.">
<Button intent="primary" icon={<Icon name="materialization" />} disabled>
Materialize
{checkedAssets.length > 1 ? `Materialize (${checkedAssets.length})` : 'Materialize'}
</Button>
</Tooltip>
) : (
Expand All @@ -112,12 +115,13 @@ export const AssetTable = ({
<th style={{width: 42, paddingTop: 0, paddingBottom: 0}}>
<Checkbox
indeterminate={
checkedPaths.size > 0 && checkedPaths.size !== pageDisplayPathKeys.length
checkedPathsOnscreen.length > 0 &&
checkedPathsOnscreen.length !== pageDisplayPathKeys.length
}
checked={checkedPaths.size === pageDisplayPathKeys.length}
checked={checkedPathsOnscreen.length === pageDisplayPathKeys.length}
onChange={(e) => {
if (e.target instanceof HTMLInputElement) {
onToggleAll(checkedPaths.size !== pageDisplayPathKeys.length);
onToggleAll(checkedPathsOnscreen.length !== pageDisplayPathKeys.length);
}
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export const getJobItemsForOption = (option: DagsterRepoOption) => {
const address = buildRepoAddress(repository.name, repositoryLocation.name);

const {schedules, sensors} = repository;
const someInRepoHasIcon = !!(schedules.length || sensors.length);

for (const pipeline of repository.pipelines) {
if (isHiddenAssetGroupJob(pipeline.name)) {
continue;
Expand All @@ -61,7 +63,7 @@ export const getJobItemsForOption = (option: DagsterRepoOption) => {
isJob,
leftIcon: 'job',
label: (
<Label $hasIcon={!!(schedules.length || sensors.length) || !isJob}>
<Label $hasIcon={someInRepoHasIcon}>
<TruncatingName data-tooltip={name} data-tooltip-style={LabelTooltipStyles}>
{name}
</TruncatingName>
Expand All @@ -87,7 +89,8 @@ const Label = styled.div<{$hasIcon: boolean}>`
justify-content: flex-start;
align-items: center;
gap: 8px;
margin-right: ${({$hasIcon}) => ($hasIcon ? '20px' : '0')};
margin-right: ${({$hasIcon}) => ($hasIcon === true ? '20px' : '0px')};
white-space: nowrap;
`;

const LabelTooltipStyles = JSON.stringify({
Expand Down
3 changes: 2 additions & 1 deletion js_modules/dagit/packages/core/src/testing/defaultMocks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import faker from 'faker';

export const hyphenatedName = () => faker.random.words(2).replace(/ /g, '-').toLowerCase();
export const hyphenatedName = (wordCount = 2) =>
faker.random.words(wordCount).replace(/ /g, '-').toLowerCase();
const randomId = () => faker.datatype.uuid();

/**
Expand Down
81 changes: 70 additions & 11 deletions js_modules/dagit/packages/core/src/ui/SectionedLeftNav.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react';

import {LEFT_NAV_WIDTH} from '../nav/LeftNav';
import {StorybookProvider} from '../testing/StorybookProvider';
import {defaultMocks} from '../testing/defaultMocks';
import {defaultMocks, hyphenatedName} from '../testing/defaultMocks';

import {SectionedLeftNav} from './SectionedLeftNav';

Expand All @@ -13,18 +13,77 @@ export default {
component: SectionedLeftNav,
} as Meta;

const mocks = {
Repository: () => ({
...defaultMocks.Repository(),
pipelines: () => [...new Array(15)],
}),
Workspace: () => ({
...defaultMocks.Workspace(),
locationEntries: () => [...new Array(2)],
}),
};
const names = [hyphenatedName(4), hyphenatedName(4), hyphenatedName(4), hyphenatedName(4)];
let repoIndex = 0;
let nameIndex = 0;

export const Default = () => {
const mocks = {
Pipeline: () => ({
name: () => names[nameIndex++ % 4],
modes: () => [...new Array(1)],
isAssetJob: () => false,
}),
Schedule: () => ({
id: hyphenatedName,
name: hyphenatedName,
pipelineName: () => names[0],
results: () => [...new Array(1)],
}),
Sensor: () => ({
id: hyphenatedName,
name: hyphenatedName,
targets: () => [
{
pipelineName: names[1],
mode: 'mistmatching_mode',
},
],
results: () => [...new Array(1)],
}),
Repository: () => ({
...defaultMocks.Repository(),
name: () => (repoIndex++ % 2 === 0 ? 'default' : hyphenatedName(4)),
sensors: () => (repoIndex === 1 ? [...new Array(2)] : []),
schedules: () => (repoIndex === 1 ? [...new Array(2)] : []),
}),
RepositoryLocation: () => ({
...defaultMocks.RepositoryLocation(),
name: () => hyphenatedName(6),
}),
Workspace: () => ({
...defaultMocks.Workspace(),
locationEntries: () => [...new Array(2)],
}),
};

return (
<StorybookProvider apolloProps={{mocks}}>
<div style={{position: 'absolute', left: 0, top: 0, height: '100%', width: LEFT_NAV_WIDTH}}>
<SectionedLeftNav />
</div>
</StorybookProvider>
);
};

export const SingleRepo = () => {
const mocks = {
Repository: () => ({
...defaultMocks.Repository(),
pipelines: () => [...new Array(15)],
}),
RepositoryLocation: () => ({
environmentPath: () => 'what then',
id: () => 'my_location',
name: () => 'my_location',
repositories: () => [...new Array(1)],
}),
Workspace: () => ({
...defaultMocks.Workspace(),
locationEntries: () => [...new Array(1)],
}),
};

return (
<StorybookProvider apolloProps={{mocks}}>
<div style={{position: 'absolute', left: 0, top: 0, height: '100%', width: LEFT_NAV_WIDTH}}>
Expand Down
29 changes: 19 additions & 10 deletions js_modules/dagit/packages/core/src/ui/SectionedLeftNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ export const SectionedLeftNav = () => {
onToggle={onToggle}
option={repo}
repoAddress={repoAddress}
expanded={expandedKeys.includes(addressAsString)}
expanded={sortedRepos.length === 1 || expandedKeys.includes(addressAsString)}
collapsible={sortedRepos.length > 1}
showRepoLocation={duplicateRepoNames.has(repoName)}
match={match?.repoAddress === repoAddress ? match : null}
/>
Expand All @@ -123,6 +124,7 @@ const HEADER_HEIGHT_WITH_LOCATION = 64;
//
interface SectionProps {
expanded: boolean;
collapsible: boolean;
onToggle: (repoAddress: RepoAddress) => void;
option: DagsterRepoOption;
match: {itemName: string; itemType: 'asset-group' | 'job'} | null;
Expand All @@ -131,7 +133,7 @@ interface SectionProps {
}

export const Section: React.FC<SectionProps> = React.memo((props) => {
const {expanded, onToggle, option, match, repoAddress, showRepoLocation} = props;
const {expanded, collapsible, onToggle, option, match, repoAddress, showRepoLocation} = props;
const matchRef = React.useRef<HTMLDivElement>(null);

const jobItems = React.useMemo(() => getJobItemsForOption(option), [option]);
Expand Down Expand Up @@ -178,14 +180,17 @@ export const Section: React.FC<SectionProps> = React.memo((props) => {
$showTypeLabels={showTypeLabels}
$showRepoLocation={showRepoLocation}
disabled={empty}
onClick={() => onToggle(repoAddress)}
onClick={collapsible ? () => onToggle(repoAddress) : undefined}
>
<Box flex={{direction: 'row', alignItems: 'flex-start', gap: 8}}>
<Box
flex={{direction: 'row', alignItems: 'flex-start', gap: 8}}
style={{flex: 1, maxWidth: '100%'}}
>
<Box margin={{top: 2}}>
<Icon name="folder_open" size={16} />
</Box>
<RepoNameContainer>
<div style={{minWidth: 0}}>
<Box flex={{direction: 'column'}} style={{flex: 1, minWidth: 0}}>
<RepoName style={{fontWeight: 500}} data-tooltip={option.repository.name}>
{option.repository.name}
</RepoName>
Expand All @@ -194,7 +199,8 @@ export const Section: React.FC<SectionProps> = React.memo((props) => {
@{option.repositoryLocation.name}
</RepoLocation>
) : null}
</div>
</Box>

{/* Wrapper div to prevent tag from stretching vertically */}
<div>
<BaseTag
Expand All @@ -204,9 +210,11 @@ export const Section: React.FC<SectionProps> = React.memo((props) => {
/>
</div>
</RepoNameContainer>
<Box margin={{top: 2}}>
<Icon name="arrow_drop_down" />
</Box>
{collapsible && (
<Box margin={{top: 2}}>
<Icon name="arrow_drop_down" />
</Box>
)}
</Box>
</SectionHeader>
{visibleItems({type: 'job', items: jobItems})}
Expand Down Expand Up @@ -335,7 +343,8 @@ const RepoNameContainer = styled.div`
display: flex;
justify-content: space-between;
margin-top: 2px;
width: 244px;
flex: 1;
min-width: 0;
`;

const RepoName = styled.div`
Expand Down

0 comments on commit e4a00e3

Please sign in to comment.