Skip to content

Commit

Permalink
Update expand/collapse section buttons on Runs page (#16622)
Browse files Browse the repository at this point in the history
## Summary & Motivation

Updating these buttons based on the [new figma
design](https://www.figma.com/file/OnaH8ynIsPg3yh7BpwyDQX/Logging-Improvements?node-id=147%3A818&mode=dev)

Since SplitPanelContianer isn't a controlled component I opted to just
track whether the top/bottom is expanded outside of SplitPanelContainer
so that we can determine the icon / tooltip to render. I think ideally
we make SplitPanelContainer a controlled component but I didn't want to
change a bunch of callsites in this PR

## How I Tested These Changes

locally
<img width="1274" alt="Screenshot 2023-09-19 at 10 23 54 AM"
src="https://github.com/dagster-io/dagster/assets/2286579/c043f5de-3c36-4e72-ae53-22c382bc2a8d">
<img width="1016" alt="Screenshot 2023-09-19 at 10 23 49 AM"
src="https://github.com/dagster-io/dagster/assets/2286579/94b4acc8-5768-4de1-9246-cf311dd2c052">
<img width="441" alt="Screenshot 2023-09-19 at 10 23 42 AM"
src="https://github.com/dagster-io/dagster/assets/2286579/c366f644-86da-471a-b227-74e682271d50">
<img width="438" alt="Screenshot 2023-09-19 at 10 23 38 AM"
src="https://github.com/dagster-io/dagster/assets/2286579/b315ebaf-784b-4103-82d3-6c83e1171755">
  • Loading branch information
salazarm authored Sep 19, 2023
1 parent 30f2373 commit 761cb7f
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import checklist from '../icon-svgs/checklist.svg';
import chevron_left from '../icon-svgs/chevron_left.svg';
import chevron_right from '../icon-svgs/chevron_right.svg';
import close from '../icon-svgs/close.svg';
import collapse_arrows from '../icon-svgs/collapse_arrows.svg';
import concept_book from '../icon-svgs/concept-book.svg';
import console_icon from '../icon-svgs/console.svg';
import content_copy from '../icon-svgs/content_copy.svg';
Expand All @@ -55,6 +56,7 @@ import error from '../icon-svgs/error.svg';
import error_outline from '../icon-svgs/error_outline.svg';
import execute from '../icon-svgs/execute.svg';
import expand from '../icon-svgs/expand.svg';
import expand_arrows from '../icon-svgs/expand_arrows.svg';
import expand_less from '../icon-svgs/expand_less.svg';
import expand_more from '../icon-svgs/expand_more.svg';
import filter_alt from '../icon-svgs/filter_alt.svg';
Expand Down Expand Up @@ -246,6 +248,7 @@ export const Icons = {
close,
console: console_icon,
content_copy,
collapse_arrows,
delete: deleteSVG,
done,
dot,
Expand All @@ -257,6 +260,7 @@ export const Icons = {
error,
error_outline,
expand,
expand_arrows,
expand_less,
expand_more,
filter_alt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react';
import styled from 'styled-components';

import {Button} from './Button';
import {ButtonGroup} from './ButtonGroup';
import {Colors} from './Colors';
import {Icon} from './Icon';

Expand Down Expand Up @@ -45,6 +44,10 @@ export class SplitPanelContainer extends React.Component<
window.localStorage.setItem(this.state.key, `${size}`);
};

getSize = () => {
return this.state.size;
};

render() {
const {firstMinSize, first, second} = this.props;
const {size: _size, resizing} = this.state;
Expand Down Expand Up @@ -137,41 +140,6 @@ interface PanelToggleProps {
container: React.RefObject<SplitPanelContainer>;
}

export const FirstOrSecondPanelToggle = ({container, axis}: PanelToggleProps) => {
const onClick = (id: string) => {
let size = 50;
if (id === 'first-pane') {
size = 100;
} else if (id === 'second-pane') {
size = 0;
}
container.current?.onChangeSize(size);
};

return (
<ButtonGroup
buttons={[
{
id: 'first-pane',
icon: axis === 'vertical' ? 'panel_show_top' : 'panel_show_left',
tooltip: axis === 'vertical' ? 'Show only top pane' : 'Show only left pane',
},
{
id: 'split',
icon: 'panel_show_both',
tooltip: 'Show both panes',
},
{
id: 'second-pane',
icon: axis === 'vertical' ? 'panel_show_bottom' : 'panel_show_right',
tooltip: axis === 'vertical' ? 'Show only bottom pane' : 'Show only right pane',
},
]}
onClick={onClick}
/>
);
};

// Todo: This component attempts to sync itself with the container, but it can't
// observe the container's width without a React context or adding a listener, etc.
// If we keep making components that manipulate panel state we may want to move it
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
23 changes: 22 additions & 1 deletion js_modules/dagster-ui/packages/ui-core/src/runs/LogsToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Tooltip,
Suggest,
ExternalAnchorButton,
Button,
} from '@dagster-io/ui-components';
import * as React from 'react';
import styled from 'styled-components';
Expand Down Expand Up @@ -45,10 +46,15 @@ interface ILogsToolbarProps {
children?: React.ReactNode;
}

interface WithExpandCollapseProps extends ILogsToolbarProps {
isSectionExpanded: boolean;
toggleExpanded: () => void;
}

const logQueryToString = (logQuery: LogFilterValue[]) =>
logQuery.map(({token, value}) => (token ? `${token}:${value}` : value)).join(' ');

export const LogsToolbar: React.FC<ILogsToolbarProps> = (props) => {
export const LogsToolbar: React.FC<ILogsToolbarProps | WithExpandCollapseProps> = (props) => {
const {
steps,
metadata,
Expand All @@ -62,6 +68,13 @@ export const LogsToolbar: React.FC<ILogsToolbarProps> = (props) => {
computeLogUrl,
children,
} = props;
let isSectionExpanded;
let toggleExpanded;

if ('isSectionExpanded' in props) {
isSectionExpanded = props.isSectionExpanded;
toggleExpanded = props.toggleExpanded;
}

const activeItems = React.useMemo(() => new Set([logType]), [logType]);

Expand Down Expand Up @@ -95,6 +108,14 @@ export const LogsToolbar: React.FC<ILogsToolbarProps> = (props) => {
/>
)}
{children}
{toggleExpanded ? (
<Tooltip content={isSectionExpanded ? 'Collapse' : 'Expand'}>
<Button
icon={<Icon name={isSectionExpanded ? 'collapse_arrows' : 'expand_arrows'} />}
onClick={toggleExpanded}
/>
</Tooltip>
) : null}
</OptionsContainer>
);
};
Expand Down
50 changes: 46 additions & 4 deletions js_modules/dagster-ui/packages/ui-core/src/runs/Run.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import {
Box,
NonIdealState,
FirstOrSecondPanelToggle,
SplitPanelContainer,
ErrorBoundary,
Button,
Icon,
Tooltip,
} from '@dagster-io/ui-components';
import * as React from 'react';
import styled from 'styled-components';
Expand Down Expand Up @@ -166,7 +168,6 @@ const RunWithData: React.FC<RunWithDataProps> = ({
onSetSelectionQuery,
}) => {
const onLaunch = useJobReExecution(run);
const splitPanelContainer = React.createRef<SplitPanelContainer>();

const [queryLogType, setQueryLogType] = useQueryPersistedState<string>({
queryKey: 'logType',
Expand Down Expand Up @@ -242,6 +243,38 @@ const RunWithData: React.FC<RunWithDataProps> = ({
onSetSelectionQuery(newSelected.join(', ') || '*');
};

const [splitPanelContainer, setSplitPanelContainer] = React.useState<null | SplitPanelContainer>(
null,
);
React.useEffect(() => {
const initialSize = splitPanelContainer?.getSize();
switch (initialSize) {
case 100:
setExpandedPanel('top');
return;
case 0:
setExpandedPanel('bottom');
return;
}
}, [splitPanelContainer]);

const [expandedPanel, setExpandedPanel] = React.useState<null | 'top' | 'bottom'>(null);
const isTopExpanded = expandedPanel === 'top';
const isBottomExpanded = expandedPanel === 'bottom';

const expandBottomPanel = () => {
splitPanelContainer?.onChangeSize(0);
setExpandedPanel('bottom');
};
const expandTopPanel = () => {
splitPanelContainer?.onChangeSize(100);
setExpandedPanel('top');
};
const resetPanels = () => {
splitPanelContainer?.onChangeSize(50);
setExpandedPanel(null);
};

const gantt = (metadata: IRunMetadataDict) => {
if (!run) {
return <GanttChartLoadingState runId={runId} />;
Expand All @@ -260,7 +293,12 @@ const RunWithData: React.FC<RunWithDataProps> = ({
}}
toolbarActions={
<Box flex={{direction: 'row', alignItems: 'center', gap: 12}}>
<FirstOrSecondPanelToggle axis="vertical" container={splitPanelContainer} />
<Tooltip content={isTopExpanded ? 'Collapse' : 'Expand'}>
<Button
icon={<Icon name={isTopExpanded ? 'collapse_arrows' : 'expand_arrows'} />}
onClick={isTopExpanded ? resetPanels : expandTopPanel}
/>
</Tooltip>
<RunActionButtons
run={run}
onLaunch={onLaunch}
Expand Down Expand Up @@ -288,7 +326,9 @@ const RunWithData: React.FC<RunWithDataProps> = ({
return (
<>
<SplitPanelContainer
ref={splitPanelContainer}
ref={(container) => {
setSplitPanelContainer(container);
}}
axis="vertical"
identifier="run-gantt"
firstInitialPercent={35}
Expand All @@ -308,6 +348,8 @@ const RunWithData: React.FC<RunWithDataProps> = ({
onSetComputeLogKey={setComputeLogFileKey}
computeLogUrl={computeLogUrl}
counts={logs.counts}
isSectionExpanded={isBottomExpanded}
toggleExpanded={isBottomExpanded ? resetPanels : expandBottomPanel}
/>
{logType !== LogType.structured ? (
!computeLogFileKey ? (
Expand Down

2 comments on commit 761cb7f

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

Built with commit 761cb7f.
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-qkf7dnvs3-elementl.vercel.app

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

Please sign in to comment.