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.

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

feat: expand/collapse domain settings section #358

Merged
merged 4 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/basics/LocalCache/defaultConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export enum LocalCacheItem {

// Production flags
ShowWorkflowVersions = 'flyte.show-workflow-versions',
ShowDomainSettings = 'flyte.show-domain-settings',
}

type LocalCacheConfig = { [k: string]: string };
Expand All @@ -21,4 +22,5 @@ export const defaultLocalCacheConfig: LocalCacheConfig = {

// Production
'flyte.show-workflow-versions': 'true',
'flyte.show-domain-settings': 'true',
};
33 changes: 8 additions & 25 deletions src/components/Entities/EntityExecutionsBarChart.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import * as React from 'react';
import Typography from '@material-ui/core/Typography';
import { makeStyles, Theme } from '@material-ui/core/styles';
import { formatDateUTC, millisecondsToHMS } from 'common/formatters';
import { timestampToDate } from 'common/utils';
import { BarChart } from 'components/common/BarChart';
Expand All @@ -16,17 +14,7 @@ import {
getWorkflowExecutionPhaseConstants,
getWorkflowExecutionTimingMS,
} from '../Executions/utils';

const useStyles = makeStyles((theme: Theme) => ({
header: {
paddingBottom: theme.spacing(1),
paddingLeft: theme.spacing(1),
borderBottom: `1px solid ${theme.palette.divider}`,
},
body: {
margin: theme.spacing(1),
},
}));
import t from './strings';

export interface EntityExecutionsBarChartProps {
id: ResourceIdentifier;
Expand Down Expand Up @@ -89,7 +77,6 @@ export const EntityExecutionsBarChart: React.FC<EntityExecutionsBarChartProps> =
onToggle,
chartIds,
}) => {
const styles = useStyles();
const { domain, project, resourceType } = id;
const filtersState = useWorkflowExecutionFiltersState();
const sort = {
Expand Down Expand Up @@ -122,17 +109,13 @@ export const EntityExecutionsBarChart: React.FC<EntityExecutionsBarChartProps> =

return (
<WaitForData {...executions}>
<Typography className={styles.header} variant="h6">
All Executions in the Workflow
</Typography>
<div className={styles.body}>
<BarChart
chartIds={chartIds}
data={getExecutionTimeData(executions.value)}
startDate={getStartExecutionTime(executions.value)}
onClickItem={handleClickItem}
/>
</div>
<BarChart
title={t('allExecutionsChartTitle')}
chartIds={chartIds}
data={getExecutionTimeData(executions.value)}
startDate={getStartExecutionTime(executions.value)}
onClickItem={handleClickItem}
/>
</WaitForData>
);
};
1 change: 1 addition & 0 deletions src/components/Entities/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createLocalizedString } from 'basics/Locale';
import { startCase } from 'lodash';

const str = {
allExecutionsChartTitle: 'All Executions in the Workflow',
workflowVersionsTitle: 'Recent Workflow Versions',
viewAll: 'View All',
schedulesHeader: 'Schedules',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import { tableHeaderColor } from 'components/Theme/constants';
import { timestampToDate } from 'common/utils';
import { NodeExecution } from 'models/Execution/types';
import { dNode } from 'models/Graph/types';
import { getChartDurationData } from './BarChart/chartData';
import { convertToPlainNodes } from './helpers';
import { ChartHeader } from './chartHeader';
import { useScaleContext } from './scaleContext';
import { TaskNames } from './taskNames';
import { BarChart } from './BarChart';
import { getChartDurationData } from './TimelineChart/chartData';
import { TimelineChart } from './TimelineChart';

interface StyleProps {
chartWidth: number;
Expand Down Expand Up @@ -116,7 +116,7 @@ export const ExecutionTimeline: React.FC<ExProps> = ({ nodeExecutions, chartTime
const styles = useStyles({ chartWidth: chartWidth, itemsShown: showNodes.length });

React.useEffect(() => {
// Sync width of elements and intervals of ChartHeader (time labels) and BarChart
// Sync width of elements and intervals of ChartHeader (time labels) and TimelineChart
const calcWidth = Math.ceil(totalDurationSec / chartTimeInterval) * INTERVAL_LENGTH;
if (durationsRef.current && calcWidth < durationsRef.current.clientWidth) {
setLabelInterval(
Expand Down Expand Up @@ -196,7 +196,7 @@ export const ExecutionTimeline: React.FC<ExProps> = ({ nodeExecutions, chartTime
</div>
<div className={styles.taskDurationsView} ref={durationsRef} onScroll={onGraphScroll}>
<div className={styles.chartHeader}>
<BarChart items={barItemsData} chartTimeIntervalSec={chartTimeInterval} />
<TimelineChart items={barItemsData} chartTimeIntervalSec={chartTimeInterval} />
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ComponentMeta, ComponentStory } from '@storybook/react';
import * as React from 'react';
import { NodeExecutionPhase } from 'models/Execution/enums';
import { BarChart } from '.';
import { TimelineChart } from '.';

const barItems = [
{ phase: NodeExecutionPhase.FAILED, startOffsetSec: 0, durationSec: 5, isFromCache: false },
Expand All @@ -14,10 +14,10 @@ const barItems = [

export default {
title: 'Workflow/Timeline',
component: BarChart,
} as ComponentMeta<typeof BarChart>;
component: TimelineChart,
} as ComponentMeta<typeof TimelineChart>;

const Template: ComponentStory<typeof BarChart> = (args) => <BarChart {...args} />;
const Template: ComponentStory<typeof TimelineChart> = (args) => <TimelineChart {...args} />;
export const BarSection = Template.bind({});
BarSection.args = {
items: barItems,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ComponentMeta, ComponentStory } from '@storybook/react';
import * as React from 'react';
import { NodeExecutionPhase } from 'models/Execution/enums';
import { BarItemData } from './utils';
import { BarChart } from '.';
import { TimelineChart } from '.';

const phaseEnumTyping = {
options: Object.values(NodeExecutionPhase),
Expand All @@ -22,7 +22,7 @@ interface SingleItemProps extends BarItemData {
*/
const SingleBarItem = (props: SingleItemProps) => {
const items = [props];
return <BarChart items={items} chartTimeIntervalSec={props.chartTimeIntervalSec} />;
return <TimelineChart items={items} chartTimeIntervalSec={props.chartTimeIntervalSec} />;
};

export default {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { Bar } from 'react-chartjs-2';
import { getBarOptions } from './barOptions';
import { BarItemData, generateChartData, getChartData } from './utils';

interface BarChartProps {
interface TimelineChartProps {
items: BarItemData[];
chartTimeIntervalSec: number;
}

export const BarChart = (props: BarChartProps) => {
export const TimelineChart = (props: TimelineChartProps) => {
const phaseData = generateChartData(props.items);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Mark } from '@material-ui/core/Slider';
import { log } from 'common/log';
import * as React from 'react';
import { createContext, useContext } from 'react';
import { formatSecondsToHmsFormat } from './BarChart/utils';
import { formatSecondsToHmsFormat } from './TimelineChart/utils';

const MIN_SCALE_VALUE = 60; // 1 min
const MAX_SCALE_VALUE = 3600; // 1h
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
generateChartData,
getOffsetColor,
TRANSPARENT,
} from '../Timeline/BarChart/utils';
} from '../Timeline/TimelineChart/utils';
import { mockbarItems } from './__mocks__/NodeExecution.mock';

describe('ExecutionDetails > Timeline > BarChart', () => {
Expand Down
18 changes: 5 additions & 13 deletions src/components/Project/ProjectDashboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
getExecutionTimeData,
getStartExecutionTime,
} from 'components/Entities/EntityExecutionsBarChart';
import classNames from 'classnames';
import { useExecutionShowArchivedState } from 'components/Executions/filters/useExecutionArchiveState';
import { useOnlyMyExecutionsFilterState } from 'components/Executions/filters/useOnlyMyExecutionsFilterState';
import { WaitForData } from 'components/common/WaitForData';
Expand All @@ -37,7 +36,7 @@ import { failedToLoadExecutionsString } from './constants';
const useStyles = makeStyles((theme: Theme) => ({
projectStats: {
paddingTop: theme.spacing(7),
paddingBottom: theme.spacing(7),
paddingBottom: theme.spacing(5),
anrusina marked this conversation as resolved.
Show resolved Hide resolved
display: 'flex',
justifyContent: 'space-evenly',
alignItems: 'center',
Expand All @@ -52,13 +51,8 @@ const useStyles = makeStyles((theme: Theme) => ({
paddingLeft: theme.spacing(1),
borderBottom: `1px solid ${theme.palette.divider}`,
},
marginTop: {
marginTop: theme.spacing(2),
},
chartContainer: {
paddingLeft: theme.spacing(1),
paddingRight: theme.spacing(3),
paddingTop: theme.spacing(1),
withPaddingX: {
padding: theme.spacing(0, 2, 0, 2),
},
}));

Expand Down Expand Up @@ -190,12 +184,10 @@ export const ProjectDashboard: React.FC<ProjectDashboardProps> = ({
{renderDomainSettingsSection}
</WaitForQuery>
<div className={styles.container}>
<Typography className={classNames(styles.header, styles.marginTop)} variant="h6">
{t('last100ExecutionsTitle')}
</Typography>
<div className={styles.chartContainer}>
<div className={styles.withPaddingX}>
<WaitForData {...last100Executions}>
<BarChart
title={t('last100ExecutionsTitle')}
chartIds={[]}
data={getExecutionTimeData(last100Executions.value)}
startDate={getStartExecutionTime(last100Executions.value)}
Expand Down
3 changes: 3 additions & 0 deletions src/components/Project/test/ProjectDashboard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { mockAPIContextValue } from 'components/data/__mocks__/apiContext';
import { getUserProfile } from 'models/Common/api';
import { getProjectDomainAttributes } from 'models/Project/api';
import { Admin } from 'flyteidl';
import * as LocalCache from 'basics/LocalCache';
import { ProjectDashboard } from '../ProjectDashboard';
import { failedToLoadExecutionsString } from '../constants';

Expand Down Expand Up @@ -69,6 +70,8 @@ describe('ProjectDashboard', () => {
[sortQueryKeys.key]: executionSortFields.createdAt,
};

jest.spyOn(LocalCache, 'useLocalCache');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed? don't see where the function return or isCalled checked or mocked.
Looks like without it tests should also pass

Copy link
Contributor Author

Choose a reason for hiding this comment

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

useLocalCache was added to a child component DomainSettingsSection and tests fail when parent tries to render a child


beforeEach(() => {
mockGetUserProfile = jest.fn().mockResolvedValue(null);
queryClient = createTestQueryClient();
Expand Down
70 changes: 49 additions & 21 deletions src/components/common/BarChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,22 @@ import * as React from 'react';
import { makeStyles, Theme } from '@material-ui/core/styles';
import { smallFontSize } from 'components/Theme/constants';
import { COLOR_SPECTRUM } from 'components/Theme/colorSpectrum';
import { Tooltip, Zoom } from '@material-ui/core';
import { Tooltip, Typography, Zoom } from '@material-ui/core';

const useStyles = makeStyles((theme: Theme) => ({
container: {
chartContainer: {
paddingLeft: theme.spacing(1),
paddingRight: theme.spacing(3),
paddingTop: theme.spacing(1),
minHeight: '135px',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added min height, so the chart is rendered inside the reserved space without any shifts of the content below it

},
title: {
marginTop: theme.spacing(2),
paddingBottom: theme.spacing(1),
paddingLeft: theme.spacing(1),
borderBottom: `1px solid ${theme.palette.divider}`,
},
wrapper: {
display: 'flex',
flexDirection: 'column',
marginBottom: theme.spacing(2.5),
Expand Down Expand Up @@ -57,6 +69,7 @@ interface BarChartItemProps extends BarChartData {
}

interface BarChartProps {
title: string;
data: BarChartData[];
startDate?: string;
onClickItem?: (item: any) => void;
Expand Down Expand Up @@ -109,7 +122,13 @@ export const BarChartItem: React.FC<BarChartItemProps> = ({
* @param startDate
* @constructor
*/
export const BarChart: React.FC<BarChartProps> = ({ chartIds, data, startDate, onClickItem }) => {
export const BarChart: React.FC<BarChartProps> = ({
title,
chartIds,
data,
startDate,
onClickItem,
}) => {
const styles = useStyles();

const maxHeight = React.useMemo(() => {
Expand All @@ -126,24 +145,33 @@ export const BarChart: React.FC<BarChartProps> = ({ chartIds, data, startDate, o
);

return (
<div className={styles.container}>
<div className={styles.header}>
<span>{startDate}</span>
<span>Most Recent</span>
</div>
<div className={styles.body}>
{data.map((item, index) => (
<BarChartItem
value={(Math.log2(item.value) / maxHeight) * 100}
color={item.color}
tooltip={item.tooltip}
onClick={handleClickItem(item)}
key={`bar-chart-item-${index}`}
isSelected={
chartIds.length === 0 ? true : item.metadata && chartIds.includes(item.metadata.name)
}
/>
))}
<div>
<Typography className={styles.title} variant="h6">
{title}
</Typography>
<div className={styles.chartContainer}>
<div className={styles.wrapper}>
<div className={styles.header}>
<span>{startDate}</span>
<span>Most Recent</span>
</div>
<div className={styles.body}>
{data.map((item, index) => (
<BarChartItem
value={(Math.log2(item.value) / maxHeight) * 100}
color={item.color}
tooltip={item.tooltip}
onClick={handleClickItem(item)}
key={`bar-chart-item-${index}`}
isSelected={
chartIds.length === 0
? true
: item.metadata && chartIds.includes(item.metadata.name)
}
/>
))}
</div>
</div>
</div>
</div>
);
Expand Down
10 changes: 6 additions & 4 deletions src/components/common/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
TableRow,
} from '@material-ui/core';
import { COLOR_SPECTRUM } from 'components/Theme/colorSpectrum';
import classNames from 'classnames';

const useStyles = makeStyles((theme: Theme) => ({
headerCell: {
Expand All @@ -20,9 +21,8 @@ const useStyles = makeStyles((theme: Theme) => ({
padding: theme.spacing(1, 0, 1, 0),
minWidth: '100px',
},
cellLeft: {
padding: theme.spacing(1, 1, 1, 0),
minWidth: '100px',
withRightPadding: {
paddingRight: theme.spacing(1),
},
}));

Expand All @@ -45,7 +45,9 @@ export const DataTable: React.FC<DataTableProps> = ({ data }) => {
<TableBody>
{Object.keys(data).map((key) => (
<TableRow key={key}>
<TableCell className={styles.cellLeft}>{key}</TableCell>
<TableCell className={classNames(styles.cell, styles.withRightPadding)}>
{key}
</TableCell>
<TableCell className={styles.cell}>{data[key]}</TableCell>
</TableRow>
))}
Expand Down
Loading