From 4164a457007154fd6cba49c4f67facf8d2306360 Mon Sep 17 00:00:00 2001 From: Viktor Bersch Date: Wed, 8 Apr 2020 17:43:33 +0200 Subject: [PATCH 1/3] fix(useSizeMonitor): separate size monitoring for height and width in Charts --- packages/charts/src/hooks/useSizeMonitor.ts | 26 +++++++++++++-------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/charts/src/hooks/useSizeMonitor.ts b/packages/charts/src/hooks/useSizeMonitor.ts index b0e9231f5b2..7dcb9b1b038 100644 --- a/packages/charts/src/hooks/useSizeMonitor.ts +++ b/packages/charts/src/hooks/useSizeMonitor.ts @@ -4,8 +4,11 @@ export const useSizeMonitor = (props, container) => { const { height: heightProp, width: widthProp, minHeight, minWidth } = props; const [height, setHeight] = useState(null); const [width, setWidth] = useState(null); + const observer = useRef(null); - const enableSizeMonitor = typeof heightProp === 'string' || typeof widthProp === 'string'; + const dynamicHeightProp = typeof heightProp === 'string'; + const dynamicWidthProp = typeof heightProp === 'string'; + const enableSizeMonitor = dynamicHeightProp || dynamicWidthProp; const recalculateSize = useCallback( (e?) => { @@ -20,30 +23,33 @@ export const useSizeMonitor = (props, container) => { clientRectWidth = e[0].contentRect.width; } - // console.log(props); - - setHeight(Math.max(minHeight, clientRectHeight)); + if (dynamicHeightProp) setHeight(Math.max(minHeight, clientRectHeight)); setWidth(Math.max(minWidth, clientRectWidth)); }, - [container.current, setHeight, setWidth] + [setHeight, setWidth, dynamicHeightProp, dynamicWidthProp] ); - const observer = useRef(new ResizeObserver(recalculateSize)); - // @ts-ignore useEffect(() => { if (enableSizeMonitor && container.current) { + observer.current = new ResizeObserver(recalculateSize); observer.current.observe(container.current); - recalculateSize(); return () => { observer.current.disconnect(); }; } + }, [recalculateSize]); + + // call recalculateSize once on mount + useEffect(() => { + if (enableSizeMonitor && container.current) { + recalculateSize(); + } }, []); return { - height: enableSizeMonitor ? height : heightProp, - width: enableSizeMonitor ? width : widthProp + height: dynamicHeightProp ? height : heightProp, + width: dynamicWidthProp ? width : widthProp }; }; From cfabcab1068b60ea3a6ae70a06b772c04f95beda Mon Sep 17 00:00:00 2001 From: Viktor Bersch Date: Wed, 8 Apr 2020 17:59:51 +0200 Subject: [PATCH 2/3] WIP: update useSizeMonitor --- .../src/components/BarChart/demo.stories.tsx | 2 +- packages/charts/src/hooks/useSizeMonitor.ts | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/charts/src/components/BarChart/demo.stories.tsx b/packages/charts/src/components/BarChart/demo.stories.tsx index 34dce12d3dc..b706b0626c8 100644 --- a/packages/charts/src/components/BarChart/demo.stories.tsx +++ b/packages/charts/src/components/BarChart/demo.stories.tsx @@ -40,7 +40,7 @@ export default { }; function Demo() { - const [full, setFull] = useState(true); + const [full, setFull] = useState(false); return (
{ clientRectWidth = e[0].contentRect.width; } - if (dynamicHeightProp) setHeight(Math.max(minHeight, clientRectHeight)); - setWidth(Math.max(minWidth, clientRectWidth)); + if (dynamicHeightProp) { + setHeight(Math.max(minHeight, clientRectHeight)); + } + if (dynamicWidthProp) { + setWidth(Math.max(minWidth, clientRectWidth)); + } }, [setHeight, setWidth, dynamicHeightProp, dynamicWidthProp] ); - // @ts-ignore useEffect(() => { if (enableSizeMonitor && container.current) { observer.current = new ResizeObserver(recalculateSize); observer.current.observe(container.current); - - return () => { - observer.current.disconnect(); - }; } + return () => { + if (observer.current) { + observer.current.disconnect(); + } + }; }, [recalculateSize]); - // call recalculateSize once on mount useEffect(() => { if (enableSizeMonitor && container.current) { recalculateSize(); From 6c48de858f9da94a3faf01be500ad41bbf6c47ab Mon Sep 17 00:00:00 2001 From: Viktor Bersch Date: Thu, 9 Apr 2020 07:50:43 +0200 Subject: [PATCH 3/3] remove calling recalculateStyles redundantly --- packages/charts/src/hooks/useSizeMonitor.ts | 30 +++++++++---------- .../src/internal/withChartContainer.tsx | 10 ++++++- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/charts/src/hooks/useSizeMonitor.ts b/packages/charts/src/hooks/useSizeMonitor.ts index 006998fdef2..dd83cd50adb 100644 --- a/packages/charts/src/hooks/useSizeMonitor.ts +++ b/packages/charts/src/hooks/useSizeMonitor.ts @@ -2,8 +2,10 @@ import { useCallback, useEffect, useRef, useState } from 'react'; export const useSizeMonitor = (props, container) => { const { height: heightProp, width: widthProp, minHeight, minWidth } = props; - const [height, setHeight] = useState(null); - const [width, setWidth] = useState(null); + const [sizeState, setSizeState] = useState({ + height: null, + width: null + }); const observer = useRef(null); const dynamicHeightProp = typeof heightProp === 'string'; @@ -23,18 +25,20 @@ export const useSizeMonitor = (props, container) => { clientRectWidth = e[0].contentRect.width; } - if (dynamicHeightProp) { - setHeight(Math.max(minHeight, clientRectHeight)); - } - if (dynamicWidthProp) { - setWidth(Math.max(minWidth, clientRectWidth)); + if (dynamicHeightProp || dynamicWidthProp) { + setSizeState((state) => ({ + ...state, + ...(dynamicHeightProp && { height: Math.max(minHeight, clientRectHeight) }), + ...(dynamicWidthProp && { width: Math.max(minWidth, clientRectWidth) }) + })); } }, - [setHeight, setWidth, dynamicHeightProp, dynamicWidthProp] + [setSizeState, minWidth, minHeight, dynamicHeightProp, dynamicWidthProp] ); useEffect(() => { if (enableSizeMonitor && container.current) { + // @ts-ignore observer.current = new ResizeObserver(recalculateSize); observer.current.observe(container.current); } @@ -45,14 +49,8 @@ export const useSizeMonitor = (props, container) => { }; }, [recalculateSize]); - useEffect(() => { - if (enableSizeMonitor && container.current) { - recalculateSize(); - } - }, []); - return { - height: dynamicHeightProp ? height : heightProp, - width: dynamicWidthProp ? width : widthProp + height: dynamicHeightProp ? sizeState.height : heightProp, + width: dynamicWidthProp ? sizeState.width : widthProp }; }; diff --git a/packages/charts/src/internal/withChartContainer.tsx b/packages/charts/src/internal/withChartContainer.tsx index c38c3c89546..ce20d892744 100644 --- a/packages/charts/src/internal/withChartContainer.tsx +++ b/packages/charts/src/internal/withChartContainer.tsx @@ -64,7 +64,15 @@ export const withChartContainer = (Component: ComponentType) => { const chartHeight = useMemo(() => (noLegend ? height : height - 60), [noLegend, height]); const chartWrapperStyles: CSSProperties = useMemo( - () => ({ position: 'relative', height: `${chartHeight}px`, width: `${width}px` }), + () => { + let innerChartWrapperStyles : CSSProperties = { + position: 'relative', + height: chartHeight >= 0 ? `${chartHeight}px` : 'auto', + width: width ? `${width}px` : 'auto' + }; + + return innerChartWrapperStyles + }, [chartHeight, width] );