From 095c1839b2aa760cb4ff21d7112b228f867091ca Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 3 Sep 2019 17:35:05 -0700 Subject: [PATCH] feat: add Wrapper support and bounding box for dynamic width/height (#215) * feat: add Wrapper support and bounding box for dynamic width/height * fix: unit tests * fix: address comments and update unit tests * docs: update storybook --- .../src/components/SuperChart.tsx | 132 +++++++++++++----- .../test/components/SuperChart.test.tsx | 68 ++++++++- .../createLoadableRenderer.test.tsx | 17 +++ .../superset-ui-chart/SuperChartStories.tsx | 81 +++++++---- 4 files changed, 231 insertions(+), 67 deletions(-) diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/SuperChart.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/SuperChart.tsx index f3cd7fbdc7bd..d04874027b69 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/SuperChart.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/SuperChart.tsx @@ -1,7 +1,8 @@ -import React from 'react'; +import React, { ReactNode } from 'react'; import ErrorBoundary, { ErrorBoundaryProps, FallbackProps } from 'react-error-boundary'; -import { parseLength } from '@superset-ui/dimension'; +import { parseLength, Dimension } from '@superset-ui/dimension'; import { ParentSize } from '@vx/responsive'; +import { createSelector } from 'reselect'; import SuperChartCore, { Props as SuperChartCoreProps } from './SuperChartCore'; import DefaultFallbackComponent from './FallbackComponent'; import ChartProps, { ChartPropsConfig } from '../models/ChartProps'; @@ -13,16 +14,40 @@ const defaultProps = { width: '100%' as string | number, }; -export type FallbackPropsWithDimension = FallbackProps & { width?: number; height?: number }; +export type FallbackPropsWithDimension = FallbackProps & Partial; + +export type WrapperProps = Dimension & { + children: ReactNode; +}; export type Props = Omit & Omit & { + /** + * Set this to true to disable error boundary built-in in SuperChart + * and let the error propagate to upper level + * and handle by yourself + */ disableErrorBoundary?: boolean; + /** debounceTime to check for container resize */ debounceTime?: number; + /** Component to render when there are unexpected errors */ FallbackComponent?: React.ComponentType; + /** Event listener for unexpected errors from chart */ onErrorBoundary?: ErrorBoundaryProps['onError']; + /** Chart width */ height?: number | string; + /** Chart height */ width?: number | string; + /** + * Component to wrap the actual chart + * after the dynamic width and height are determined. + * This can be useful for handling tooltip z-index, etc. + * e.g.
+ * You cannot just wrap this same component outside of SuperChart + * when using dynamic width or height + * because it will clash with auto-sizing. + */ + Wrapper?: React.ComponentType; }; type PropsWithDefault = Props & Readonly; @@ -37,6 +62,43 @@ export default class SuperChart extends React.PureComponent { private createChartProps = ChartProps.createSelector(); + private parseDimension = createSelector( + ({ width }: { width: string | number; height: string | number }) => width, + ({ height }) => height, + (width, height) => { + // Parse them in case they are % or 'auto' + const widthInfo = parseLength(width); + const heightInfo = parseLength(height); + + const boxHeight = heightInfo.isDynamic + ? // eslint-disable-next-line no-magic-numbers + `${heightInfo.multiplier * 100}%` + : heightInfo.value; + const boxWidth = widthInfo.isDynamic + ? // eslint-disable-next-line no-magic-numbers + `${widthInfo.multiplier * 100}%` + : widthInfo.value; + const style = { + height: boxHeight, + width: boxWidth, + }; + + // bounding box will ensure that when one dimension is not dynamic + // e.g. height = 300 + // the auto size will be bound to that value instead of being 100% by default + // e.g. height: 300 instead of height: '100%' + const BoundingBox = + widthInfo.isDynamic && + heightInfo.isDynamic && + widthInfo.multiplier === 1 && + heightInfo.multiplier === 1 + ? React.Fragment + : ({ children }: { children: ReactNode }) =>
{children}
; + + return { BoundingBox, heightInfo, widthInfo }; + }, + ); + private setRef = (core: SuperChartCore | null) => { this.core = core; }; @@ -54,26 +116,29 @@ export default class SuperChart extends React.PureComponent { disableErrorBoundary, FallbackComponent, onErrorBoundary, + Wrapper = React.Fragment, ...rest } = this.props as PropsWithDefault; const chart = ( - + + + ); // Include the error boundary by default unless it is specifically disabled. @@ -92,27 +157,26 @@ export default class SuperChart extends React.PureComponent { } render() { - const { width: inputWidth, height: inputHeight } = this.props as PropsWithDefault; - - // Parse them in case they are % or 'auto' - const widthInfo = parseLength(inputWidth); - const heightInfo = parseLength(inputHeight); + const { heightInfo, widthInfo, BoundingBox } = this.parseDimension(this + .props as PropsWithDefault); // If any of the dimension is dynamic, get parent's dimension if (widthInfo.isDynamic || heightInfo.isDynamic) { const { debounceTime } = this.props; return ( - - {({ width, height }) => - width > 0 && - height > 0 && - this.renderChart( - widthInfo.isDynamic ? Math.floor(width * widthInfo.multiplier) : widthInfo.value, - heightInfo.isDynamic ? Math.floor(height * heightInfo.multiplier) : heightInfo.value, - ) - } - + + + {({ width, height }) => + width > 0 && + height > 0 && + this.renderChart( + widthInfo.isDynamic ? Math.floor(width) : widthInfo.value, + heightInfo.isDynamic ? Math.floor(height) : heightInfo.value, + ) + } + + ); } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/SuperChart.test.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/SuperChart.test.tsx index 736715075313..f6195e342a59 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/SuperChart.test.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/SuperChart.test.tsx @@ -7,7 +7,7 @@ jest.mock('resize-observer-polyfill'); import { triggerResizeObserver } from 'resize-observer-polyfill'; import ErrorBoundary from 'react-error-boundary'; import { SuperChart } from '../../src'; -import RealSuperChart from '../../src/components/SuperChart'; +import RealSuperChart, { WrapperProps } from '../../src/components/SuperChart'; import { ChartKeys, DiligentChartPlugin, BuggyChartPlugin } from './MockChartPlugins'; import promiseTimeout from './promiseTimeout'; @@ -135,10 +135,17 @@ describe('SuperChart', () => { const wrapper = mount( , ); - triggerResizeObserver(); + triggerResizeObserver([{ contentRect: { height: 125, width: 150 } }]); return promiseTimeout(() => { const renderedWrapper = wrapper.render(); + const boundingBox = renderedWrapper + .find('div.test-component') + .parent() + .parent() + .parent(); + expect(boundingBox.css('width')).toEqual('50%'); + expect(boundingBox.css('height')).toEqual('125px'); expect(renderedWrapper.find('div.test-component')).toHaveLength(1); expectDimension(renderedWrapper, 150, 125); }, 100); @@ -147,10 +154,17 @@ describe('SuperChart', () => { const wrapper = mount( , ); - triggerResizeObserver(); + triggerResizeObserver([{ contentRect: { height: 75, width: 50 } }]); return promiseTimeout(() => { const renderedWrapper = wrapper.render(); + const boundingBox = renderedWrapper + .find('div.test-component') + .parent() + .parent() + .parent(); + expect(boundingBox.css('width')).toEqual('50px'); + expect(boundingBox.css('height')).toEqual('25%'); expect(renderedWrapper.find('div.test-component')).toHaveLength(1); expectDimension(renderedWrapper, 50, 75); }, 100); @@ -166,4 +180,52 @@ describe('SuperChart', () => { }, 100); }); }); + + describe('supports Wrapper', () => { + function MyWrapper({ width, height, children }: WrapperProps) { + return ( +
+
+ {width}x{height} +
+ {children} +
+ ); + } + + it('works with width and height that are numbers', () => { + const wrapper = mount( + , + ); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.wrapper-insert')).toHaveLength(1); + expect(renderedWrapper.find('div.wrapper-insert').text()).toEqual('100x100'); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 100, 100); + }, 100); + }); + + it('works when width and height are percent', () => { + const wrapper = mount( + , + ); + triggerResizeObserver(); + + return promiseTimeout(() => { + const renderedWrapper = wrapper.render(); + expect(renderedWrapper.find('div.wrapper-insert')).toHaveLength(1); + expect(renderedWrapper.find('div.wrapper-insert').text()).toEqual('300x300'); + expect(renderedWrapper.find('div.test-component')).toHaveLength(1); + expectDimension(renderedWrapper, 300, 300); + }, 100); + }); + }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx index a6b8efbed1a4..68813d03c86d 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx @@ -76,6 +76,23 @@ describe('createLoadableRenderer', () => { }, 10); }); + it('onRenderFailure is optional', done => { + const loadChartFailure = jest.fn(() => Promise.reject(new Error('Invalid chart'))); + const FailedRenderer = createLoadableRenderer({ + loader: { + Chart: loadChartFailure, + }, + loading, + render, + }); + shallow(); + expect(loadChartFailure).toHaveBeenCalledTimes(1); + setTimeout(() => { + expect(render).not.toHaveBeenCalled(); + done(); + }, 10); + }); + it('renders the lazy-load components', done => { const wrapper = shallow(); // lazy-loaded component not rendered immediately diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx index a94e3d0eab61..1e6c663f6bec 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/SuperChartStories.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { text } from '@storybook/addon-knobs'; -import { SuperChart, ChartProps } from '../../../../superset-ui-chart/src'; +import { SuperChart } from '../../../../superset-ui-chart/src'; import { DiligentChartPlugin, BuggyChartPlugin, @@ -13,8 +13,8 @@ new BuggyChartPlugin().configure({ key: ChartKeys.BUGGY }).register(); export default [ { renderStory: () => { - const width = text('Vis width', '50%'); - const height = text('Vis height', '75%'); + const width = text('Vis width', '100%'); + const height = text('Vis height', '100%'); return ( { - const width = text('Vis width', '500'); - const height = text('Vis height', '300'); + const width = text('Vis width', '50%'); + const height = text('Vis height', '50%'); return ( ); }, - storyName: 'passing ChartPropsConfig', + storyName: '50% of container', storyPath: '@superset-ui/chart|SuperChart', }, { @@ -51,39 +50,61 @@ export default [ const width = text('Vis width', '500'); const height = text('Vis height', '300'); - return ( - - ); + return ; }, - storyName: 'passing ChartProps', + storyName: 'fixed dimension', storyPath: '@superset-ui/chart|SuperChart', }, { renderStory: () => { const width = text('Vis width', '500'); + const height = text('Vis height', '100%'); + + return ; + }, + storyName: 'fixed width, 100% height', + storyPath: '@superset-ui/chart|SuperChart', + }, + { + renderStory: () => { + const width = text('Vis width', '100%'); const height = text('Vis height', '300'); + return ; + }, + storyName: 'fixed height, 100% width', + storyPath: '@superset-ui/chart|SuperChart', + }, + { + renderStory: () => { + const width = text('Vis width', '500'); + const height = text('Vis height', '300'); + + return ; + }, + storyName: 'With error boundary', + storyPath: '@superset-ui/chart|SuperChart', + }, + { + renderStory: () => { + const width = text('Vis width', '100%'); + const height = text('Vis height', '100%'); + return ( ( +
+
With wrapper!
+ {children} +
+ )} /> ); }, - storyName: 'With error boundary', + storyName: 'With Wrapper', storyPath: '@superset-ui/chart|SuperChart', }, ];