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

plot resize with draggable panel #352

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
47 changes: 47 additions & 0 deletions packages/libs/components/src/stories/plots/Barplot.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import FacetedBarplot from '../../plots/facetedPlots/FacetedBarplot';
import AxisRangeControl from '../../components/plotControls/AxisRangeControl';
import { NumberRange, NumberOrDateRange } from '../../types/general';
import { Toggle } from '@veupathdb/coreui';
import DraggablePanel, {
HeightAndWidthInPixels,
} from '@veupathdb/coreui/lib/components/containers/DraggablePanel';

export default {
title: 'Plots/Barplot',
Expand Down Expand Up @@ -185,3 +188,47 @@ LogScale.args = {
width: '750px',
},
};

// demo plot resize with draggable panel
export const PlotResizeWithDraggablePanel: Story<BarplotProps> = (args) => {
const panelTitle = 'Panel 1';
const draggablePanelHeight = 600;
const draggablePanelWidth = 900;

const [panelDimension, setPanelDimension] = useState<HeightAndWidthInPixels>({
height: draggablePanelHeight,
width: draggablePanelWidth,
});

return (
<DraggablePanel
defaultPosition={{ x: 200, y: 200 }}
confineToParentContainer
key={panelTitle}
isOpen
panelTitle={panelTitle}
showPanelTitle={true}
styleOverrides={{
resize: 'both',
}}
onPanelResize={(dimensions: HeightAndWidthInPixels) => {
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to use useCallback on the function here and put the dependency back (see below)

Copy link
Member

@bobular bobular Jul 20, 2023

Choose a reason for hiding this comment

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

Or even just onPanelResize={setPanelDimension}?

Copy link
Member

Choose a reason for hiding this comment

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

useState setters are guaranteed to be stable

setPanelDimension(dimensions);
}}
>
<Barplot
data={dataSet}
dependentAxisLabel={'Awesomeness'}
independentAxisLabel={'Animal'}
displayLegend={false}
containerStyles={{
// perhaps plot height may be set to be proportional to to plot width
// as mostly resize invloves the change of width
// Otherwise, if DraggablePanel has other elements like InputVariables, Plot controls,
// then, height should substract heights of those elements to be better represented
height: panelDimension.height,
width: panelDimension.width,
}}
/>
</DraggablePanel>
);
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CSSProperties, ReactNode, useEffect, useState } from 'react';
import { CSSProperties, ReactNode, useEffect, useState, useMemo } from 'react';
import Draggable, { DraggableEvent, DraggableData } from 'react-draggable';
import { css } from '@emotion/react';
import useResizeObserver from 'use-resize-observer';
Expand Down Expand Up @@ -102,18 +102,31 @@ export default function DraggablePanel({
}
}

// convert rem to px
const dragHandleHeightValue = 2;
const dragHandleHeight = useMemo(() => {
return (
dragHandleHeightValue *
parseFloat(getComputedStyle(document.documentElement).fontSize)
);
}, []);

const { ref, height, width } = useResizeObserver();

useEffect(
function invokeOnPanelResize() {
if (!onPanelResize || !height || !width) return;

onPanelResize({
height: height,
// Note: since height from useResizeObserver includes dragHandle's height, 2rem,
// dragHandle's height should be substracted from height to compute main panel size
// without this, the use of useResizeObserver's height may lead to infinite loop
// when changing plot size inside the pannel
height: height - dragHandleHeight,
width: width,
});
},
[height, width, onPanelResize]
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs to be reinstated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bobular By having onPanelResize, it causes infinite loop, thus I removed it in the dependencies, only to use [height, width] dependencies in the next line: Line 117.

[height, width]
);

const finalPosition = confineToParentContainer
Expand Down Expand Up @@ -162,7 +175,9 @@ export default function DraggablePanel({
background: ${theme?.palette?.primary?.hue[100] ?? gray[100]};
cursor: ${isDragging ? 'grabbing' : 'grab'};
display: flex;
height: 2rem;
height: ${dragHandleHeightValue != null
? dragHandleHeightValue + 'rem'
: '2rem'};
justify-content: center;
// Because the panels are positioned absolutely and overflow auto,
// the handle will get lost when the user scrolls down. We can pin the
Expand Down