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
apache · Nov 26, 2021 · 095c183 · 095c183
1 parent ed45bdf
commit 095c183
Show file tree

Hide file tree

Showing 4 changed files with 231 additions and 67 deletions.
diff --git a/...emporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/SuperChart.tsx b/...emporary_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<Dimension>;
+
+export type WrapperProps = Dimension & {
+  children: ReactNode;
+};
 
 export type Props = Omit<SuperChartCoreProps, 'chartProps'> &
   Omit<ChartPropsConfig, 'width' | 'height'> & {
+    /**
+     * 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<FallbackPropsWithDimension>;
+    /** 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. <div style={{ position: 'fixed' }} />
+     * 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<WrapperProps>;
   };
 
 type PropsWithDefault = Props & Readonly<typeof defaultProps>;
@@ -37,6 +62,43 @@ export default class SuperChart extends React.PureComponent<Props, {}> {
 
   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 }) => <div style={style}>{children}</div>;
+
+      return { BoundingBox, heightInfo, widthInfo };
+    },
+  );
+
   private setRef = (core: SuperChartCore | null) => {
     this.core = core;
   };
@@ -54,26 +116,29 @@ export default class SuperChart extends React.PureComponent<Props, {}> {
       disableErrorBoundary,
       FallbackComponent,
       onErrorBoundary,
+      Wrapper = React.Fragment,
       ...rest
     } = this.props as PropsWithDefault;
 
     const chart = (
-      <SuperChartCore
-        ref={this.setRef}
-        id={id}
-        className={className}
-        chartType={chartType}
-        chartProps={this.createChartProps({
-          ...rest,
-          height,
-          width,
-        })}
-        preTransformProps={preTransformProps}
-        overrideTransformProps={overrideTransformProps}
-        postTransformProps={postTransformProps}
-        onRenderSuccess={onRenderSuccess}
-        onRenderFailure={onRenderFailure}
-      />
+      <Wrapper width={width} height={height}>
+        <SuperChartCore
+          ref={this.setRef}
+          id={id}
+          className={className}
+          chartType={chartType}
+          chartProps={this.createChartProps({
+            ...rest,
+            height,
+            width,
+          })}
+          preTransformProps={preTransformProps}
+          overrideTransformProps={overrideTransformProps}
+          postTransformProps={postTransformProps}
+          onRenderSuccess={onRenderSuccess}
+          onRenderFailure={onRenderFailure}
+        />
+      </Wrapper>
     );
 
     // Include the error boundary by default unless it is specifically disabled.
@@ -92,27 +157,26 @@ export default class SuperChart extends React.PureComponent<Props, {}> {
   }
 
   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 (
-        <ParentSize debounceTime={debounceTime}>
-          {({ 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,
-            )
-          }
-        </ParentSize>
+        <BoundingBox>
+          <ParentSize debounceTime={debounceTime}>
+            {({ width, height }) =>
+              width > 0 &&
+              height > 0 &&
+              this.renderChart(
+                widthInfo.isDynamic ? Math.floor(width) : widthInfo.value,
+                heightInfo.isDynamic ? Math.floor(height) : heightInfo.value,
+              )
+            }
+          </ParentSize>
+        </BoundingBox>
       );
     }
 

diff --git a/...ry_superset_ui/superset-ui/packages/superset-ui-chart/test/components/SuperChart.test.tsx b/...ry_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(
         <SuperChart chartType={ChartKeys.DILIGENT} debounceTime={1} width="50%" height="125" />,
       );
-      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(
         <SuperChart chartType={ChartKeys.DILIGENT} debounceTime={1} width="50" height="25%" />,
       );
-      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 (
+        <div>
+          <div className="wrapper-insert">
+            {width}x{height}
+          </div>
+          {children}
+        </div>
+      );
+    }
+
+    it('works with width and height that are numbers', () => {
+      const wrapper = mount(
+        <SuperChart chartType={ChartKeys.DILIGENT} width={100} height={100} Wrapper={MyWrapper} />,
+      );
+
+      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(
+        <SuperChart
+          chartType={ChartKeys.DILIGENT}
+          debounceTime={1}
+          width="100%"
+          height="100%"
+          Wrapper={MyWrapper}
+        />,
+      );
+      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/...ui/superset-ui/packages/superset-ui-chart/test/components/createLoadableRenderer.test.tsx b/...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(<FailedRenderer />);
+      expect(loadChartFailure).toHaveBeenCalledTimes(1);
+      setTimeout(() => {
+        expect(render).not.toHaveBeenCalled();
+        done();
+      }, 10);
+    });
+
     it('renders the lazy-load components', done => {
       const wrapper = shallow(<LoadableRenderer />);
       // lazy-loaded component not rendered immediately