Skip to content

Commit

Permalink
[ui] Refactor SplitPanelContainer (#20909)
Browse files Browse the repository at this point in the history
## Summary & Motivation

I set out to add a minimum size for the second panel in a
`SplitPanelContainer`, and I fell into a refactoring rabbit hole.

- Add a minimum size (width/height) for the second pane, as originally
planned.
- Rewrite into functional components, with an imperative handle on
`SplitPanelContainer` to replicate its imperative API.
- Set a minimum for a few different panes around the app (global asset
graph, run log pane)
- Fix up the expand/collapse panel on launchpad config, which has some
tricky ref usage.
- Tidy up a few other things that I found hard to follow or confusing.

Note that this may lead to some panel size allocations being slightly
different from before, in cases where we have a minimum size for the
second panel. For instance, if I set a minimum of 400px for the second
panel, and the first panel is set to occupy 75% of the container, that
75% will now apply to the available size *minus* 400px, instead of 75%
of the full container. FWIW, I don't think this will be a noticeable
change for most people.


https://github.com/dagster-io/dagster/assets/2823852/b4bbc92d-c57b-468b-87a2-9216172ef4b1

## How I Tested These Changes

View launchpad, run page, global asset graph. Drag vertical and
horizontal split panels, verify correct behavior. Verify that
second-panel minimums are respected.

Test split panel contain
  • Loading branch information
hellendag authored Mar 29, 2024
1 parent 41e03b8 commit c1740a1
Show file tree
Hide file tree
Showing 6 changed files with 194 additions and 171 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {createGlobalStyle} from 'styled-components';
import {Box} from './Box';
import {ConfigEditorHandle, ConfigSchema, NewConfigEditor} from './NewConfigEditor';
import {Spinner} from './Spinner';
import {SplitPanelContainer} from './SplitPanelContainer';
import {SplitPanelContainer, SplitPanelContainerHandle} from './SplitPanelContainer';
import {ConfigEditorHelp} from './configeditor/ConfigEditorHelp';
import {isHelpContextEqual} from './configeditor/isHelpContextEqual';
import {ConfigEditorHelpContext} from './configeditor/types/ConfigEditorHelpContext';
Expand Down Expand Up @@ -32,7 +32,7 @@ export const ConfigEditorWithSchema = ({
onConfigChange,
configSchema,
}: Props) => {
const editorSplitPanelContainer = useRef<SplitPanelContainer | null>(null);
const editorSplitPanelContainer = useRef<SplitPanelContainerHandle | null>(null);
const [editorHelpContext, setEditorHelpContext] = useState<ConfigEditorHelpContext | null>(null);
const editor = useRef<ConfigEditorHandle | null>(null);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import * as React from 'react';
import {forwardRef, useCallback, useImperativeHandle, useRef, useState} from 'react';
import styled from 'styled-components';

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

const DIVIDER_THICKNESS = 2;

Expand All @@ -13,220 +11,185 @@ interface SplitPanelContainerProps {
first: React.ReactNode;
firstInitialPercent: number;
firstMinSize?: number;
secondMinSize?: number;
second: React.ReactNode | null; // Note: pass null to hide / animate away the second panel
}

interface SplitPanelContainerState {
size: number;
key: string;
resizing: boolean;
}

export class SplitPanelContainer extends React.Component<
SplitPanelContainerProps,
SplitPanelContainerState
> {
constructor(props: SplitPanelContainerProps) {
super(props);

const key = `dagster.panel-width.${this.props.identifier}`;
const value = window.localStorage.getItem(key);
let size = Number(value);
if (value === null || isNaN(size)) {
size = this.props.firstInitialPercent;
}

this.state = {size, key, resizing: false};
}
export type SplitPanelContainerHandle = {
getSize: () => number;
changeSize: (value: number) => void;
};

onChangeSize = (size: number) => {
this.setState({size});
window.localStorage.setItem(this.state.key, `${size}`);
};
export const SplitPanelContainer = forwardRef<SplitPanelContainerHandle, SplitPanelContainerProps>(
(props, ref) => {
const {
axis = 'horizontal',
identifier,
first,
firstInitialPercent,
firstMinSize,
secondMinSize = 0,
second,
} = props;

const [_, setTrigger] = useState(0);
const [resizing, setResizing] = useState(false);
const key = `dagster.panel-width.${identifier}`;

const getSize = useCallback(() => {
if (!second) {
return 100;
}
const storedSize = window.localStorage.getItem(key);
const parsed = storedSize === null ? null : parseFloat(storedSize);
return parsed === null || isNaN(parsed) ? firstInitialPercent : parsed;
}, [firstInitialPercent, key, second]);

const onChangeSize = useCallback(
(newValue: number) => {
window.localStorage.setItem(key, `${newValue}`);
setTrigger((current) => (current ? 0 : 1));
},
[key],
);

getSize = () => {
return this.state.size;
};
useImperativeHandle(ref, () => ({getSize, changeSize: onChangeSize}), [onChangeSize, getSize]);

render() {
const {firstMinSize, first, second} = this.props;
const {size: _size, resizing} = this.state;
const axis = this.props.axis || 'horizontal';
const firstSize = getSize();

const firstPaneStyles: React.CSSProperties = {flexShrink: 0};
const firstSize = second ? _size : 100;

// Note: The divider appears after the first panel, so making the first panel 100% wide
// hides the divider offscreen. To prevent this, we subtract the divider depth.
if (axis === 'horizontal') {
firstPaneStyles.minWidth = firstMinSize;
firstPaneStyles.width = `calc(${firstSize}% - ${DIVIDER_THICKNESS}px)`;
firstPaneStyles.width = `calc(${firstSize / 100} * (100% - ${
DIVIDER_THICKNESS + (second ? secondMinSize : 0)
}px))`;
} else {
firstPaneStyles.minHeight = firstMinSize;
firstPaneStyles.height = `calc(${firstSize}% - ${DIVIDER_THICKNESS}px)`;
firstPaneStyles.height = `calc(${firstSize / 100} * (100% - ${
DIVIDER_THICKNESS + (second ? secondMinSize : 0)
}px))`;
}

return (
<Container axis={axis} id="split-panel-container" resizing={resizing}>
<Container axis={axis} className="split-panel-container" resizing={resizing}>
<div className="split-panel" style={firstPaneStyles}>
{first}
</div>
<PanelDivider
axis={axis}
resizing={resizing}
onSetResizing={(resizing) => this.setState({resizing})}
onMove={this.onChangeSize}
secondMinSize={secondMinSize}
onSetResizing={setResizing}
onMove={onChangeSize}
/>
<div className="split-panel" style={{flex: 1}}>
{second}
</div>
</Container>
);
}
}
},
);

interface IDividerProps {
axis: 'horizontal' | 'vertical';
resizing: boolean;
secondMinSize: number;
onSetResizing: (resizing: boolean) => void;
onMove: (vw: number) => void;
}

class PanelDivider extends React.Component<IDividerProps> {
ref = React.createRef<any>();
const PanelDivider = (props: IDividerProps) => {
const {axis, resizing, secondMinSize, onSetResizing, onMove} = props;
const ref = useRef<HTMLDivElement>(null);

onMouseDown = (e: React.MouseEvent) => {
const onMouseDown = (e: React.MouseEvent) => {
e.preventDefault();

this.props.onSetResizing(true);
onSetResizing(true);

const onMouseMove = (event: MouseEvent) => {
const parent = this.ref.current?.closest('#split-panel-container');
const parent = ref.current?.closest('.split-panel-container');
if (!parent) {
return;
}
const parentRect = parent.getBoundingClientRect();

const firstPanelPercent =
this.props.axis === 'horizontal'
? ((event.clientX - parentRect.left) * 100) / parentRect.width
: ((event.clientY - parentRect.top) * 100) / parentRect.height;
axis === 'horizontal'
? ((event.clientX - parentRect.left) * 100) /
(parentRect.width - secondMinSize - DIVIDER_THICKNESS)
: ((event.clientY - parentRect.top) * 100) /
(parentRect.height - secondMinSize - DIVIDER_THICKNESS);

this.props.onMove(Math.min(100, Math.max(0, firstPanelPercent)));
onMove(Math.min(100, Math.max(0, firstPanelPercent)));
};

const onMouseUp = () => {
this.props.onSetResizing(false);
onSetResizing(false);
document.removeEventListener('mousemove', onMouseMove);
document.removeEventListener('mouseup', onMouseUp);
};
document.addEventListener('mousemove', onMouseMove);
document.addEventListener('mouseup', onMouseUp);
};

render() {
const Wrapper = DividerWrapper[this.props.axis];
const HitArea = DividerHitArea[this.props.axis];
return (
<Wrapper resizing={this.props.resizing} ref={this.ref}>
<HitArea onMouseDown={this.onMouseDown} />
</Wrapper>
const hitArea =
axis === 'horizontal' ? (
<HorizontalDividerHitArea onMouseDown={onMouseDown} />
) : (
<VerticalDividerHitArea onMouseDown={onMouseDown} />
);
}
}

interface PanelToggleProps {
axis: 'horizontal' | 'vertical';
container: React.RefObject<SplitPanelContainer>;
}

// 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
// all to a context consumed by both.
//
export const SecondPanelToggle = ({container, axis}: PanelToggleProps) => {
const [prevSize, setPrevSize] = React.useState<number | 'unknown'>('unknown');
const initialIsOpen = (container.current?.state.size || 0) < 100;

const [open, setOpen] = React.useState<boolean>(initialIsOpen);
React.useEffect(() => setOpen(initialIsOpen), [initialIsOpen]);

return (
<Button
active={open}
title="Toggle second pane"
icon={
<Icon
name={
axis === 'horizontal'
? open
? 'panel_hide_right'
: 'panel_show_right'
: 'panel_show_bottom'
}
/>
}
onClick={() => {
if (!container.current) {
return;
}
const current = container.current.state.size;
if (current < 90) {
setPrevSize(current);
setOpen(false);
container.current.onChangeSize(100);
} else {
setOpen(true);
container.current.onChangeSize(
prevSize === 'unknown' ? container.current.props.firstInitialPercent : prevSize,
);
}
}}
/>
return axis === 'horizontal' ? (
<HorizontalDividerWrapper $resizing={resizing} ref={ref}>
{hitArea}
</HorizontalDividerWrapper>
) : (
<VerticalDividerWrapper $resizing={resizing} ref={ref}>
{hitArea}
</VerticalDividerWrapper>
);
};

// Note: -1px margins here let the divider cover the last 1px of the previous box, hiding
// any scrollbar border it might have.

const DividerWrapper = {
horizontal: styled.div<{resizing: boolean}>`
width: ${DIVIDER_THICKNESS}px;
z-index: 1;
background: ${(p) => (p.resizing ? Colors.accentGray() : Colors.keylineDefault())};
margin-left: -1px;
overflow: visible;
position: relative;
`,
vertical: styled.div<{resizing: boolean}>`
height: ${DIVIDER_THICKNESS}px;
z-index: 1;
background: ${(p) => (p.resizing ? Colors.accentGray() : Colors.keylineDefault())};
margin-top: -1px;
overflow: visible;
position: relative;
`,
};
const HorizontalDividerWrapper = styled.div<{$resizing: boolean}>`
width: ${DIVIDER_THICKNESS}px;
z-index: 1;
background: ${(p) => (p.$resizing ? Colors.accentGray() : Colors.keylineDefault())};
margin-left: -1px;
overflow: visible;
position: relative;
`;
const VerticalDividerWrapper = styled.div<{$resizing: boolean}>`
height: ${DIVIDER_THICKNESS}px;
z-index: 1;
background: ${(p) => (p.$resizing ? Colors.accentGray() : Colors.keylineDefault())};
margin-top: -1px;
overflow: visible;
position: relative;
`;

const DividerHitArea = {
horizontal: styled.div`
width: 17px;
height: 100%;
z-index: 1;
cursor: ew-resize;
position: relative;
left: -8px;
`,
vertical: styled.div`
height: 17px;
width: 100%;
z-index: 1;
cursor: ns-resize;
position: relative;
top: -8px;
`,
};
const HorizontalDividerHitArea = styled.div`
width: 17px;
height: 100%;
z-index: 1;
cursor: ew-resize;
position: relative;
left: -8px;
`;
const VerticalDividerHitArea = styled.div`
height: 17px;
width: 100%;
z-index: 1;
cursor: ns-resize;
position: relative;
top: -8px;
`;

const Container = styled.div<{
axis?: 'horizontal' | 'vertical';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,7 @@ const AssetGraphExplorerWithData = ({
identifier="asset-graph-explorer"
firstInitialPercent={70}
firstMinSize={400}
secondMinSize={400}
first={
<ErrorBoundary region="graph">
{graphQueryItems.length === 0 ? (
Expand Down Expand Up @@ -876,6 +877,7 @@ const AssetGraphExplorerWithData = ({
identifier="explorer-wrapper"
firstMinSize={300}
firstInitialPercent={0}
secondMinSize={400}
first={
showSidebar ? (
<AssetGraphExplorerSidebar
Expand Down
Loading

2 comments on commit c1740a1

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

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

✅ Preview
https://dagit-core-storybook-8my2ivtn0-elementl.vercel.app

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

Please sign in to comment.