Fixing re-render issue for all charts when empty #28321

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

Merged

AtishayMsft merged 45 commits into microsoft:master from srmukher:users/srmukher/ReRenderFix

Aug 1, 2023

change/@fluentui-react-charting-556f2e0e-35f0-44cd-9bd6-aa9b24aaa57e.json

-Original file line number
+Diff line change
@@ -0,0 +1,7 @@
+    {
+      "type": "patch",
+      "comment": "Fixing re-render issue for all charts when empty",
+      "packageName": "@fluentui/react-charting",
+      "email": "[email protected]",
+      "dependentChangeType": "patch"
+    }

packages/react-charting/src/components/AreaChart/AreaChart.base.tsx

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -70,7 +70,6 @@ export interface IAreaChartState extends IBasestate {
  
      nearestCircleToHighlight: number | string | Date | null;

      xAxisCalloutAccessibilityData?: IAccessibilityProps;

      isShowCalloutPending: boolean;

      emptyChart?: boolean;

      /** focused point */

      activePoint: string;

    }

    @@ -107,6 +106,7 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
  
      //from O(n^2) to O(n) using a map.

      private _enableComputationOptimization: boolean;

      private _firstRenderOptimization: boolean;

      private _emptyChartId: string;

      public constructor(props: IAreaChartProps) {

        super(props);

    @@ -123,17 +123,6 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
  
          isCircleClicked: false,

          nearestCircleToHighlight: null,

          isShowCalloutPending: false,

          emptyChart: !(

            (

              this.props.data &&

              this.props.data.lineChartData &&

              this.props.data.lineChartData.length > 0 &&

              this.props.data.lineChartData.filter(item => item.data.length === 0).length === 0

            )

            // if all the data sets have no data

            // filtering all items which have no data and checking if the length of the filtered array is 0

            // which means chart is not empty

          ),

          activePoint: '',

        };

        warnDeprecations(COMPONENT_NAME, props, {

    @@ -146,6 +135,7 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
  
        this._tooltipId = getId('AreaChartTooltipID');

        this._enableComputationOptimization = true;

        this._firstRenderOptimization = true;

        this._emptyChartId = getId('_AreaChart_empty');

      }

      public componentDidUpdate() {

    @@ -159,7 +149,7 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
  
      }

      public render(): JSX.Element {

        if (!this.state.emptyChart) {

        if (!this._isChartEmpty()) {

          const { lineChartData, chartTitle } = this.props.data;

          const points = this._addDefaultColors(lineChartData);

          const { colors, opacity, stackedInfo, calloutPoints } = this._createSet(points);

    @@ -235,7 +225,7 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
  
        }

        return (

          <div

            id={getId('_AreaChart_')}

            id={this._emptyChartId}

            role={'alert'}

            style={{ opacity: '0' }}

            aria-label={'Graph has no data to display'}

    @@ -909,4 +899,18 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
  
        const yValue = point.yAxisCalloutData || point.y;

        return point.callOutAccessibilityData?.ariaLabel || `${xValue}. ${legend}, ${yValue}.`;

      };

      private _isChartEmpty(): boolean {

        return !(

          (

            this.props.data &&

            this.props.data.lineChartData &&

            this.props.data.lineChartData.length > 0 &&

            this.props.data.lineChartData.filter(item => item.data.length === 0).length === 0

          )

          // if all the data sets have no data

          // filtering all items which have no data and checking if the length of the filtered array is 0

          // which means chart is not empty

        );

      }

    }

packages/react-charting/src/components/AreaChart/AreaChart.test.tsx

-Original file line number
+Diff line change
@@ Expand Up / @@ -46,7 +46,7 @@ const points: ILineChartPoints[] = [ @@
         color: 'red',
       },
     ];
-    const chartPoints = {
+    export const chartPoints = {
       chartTitle: 'AreaChart',
       lineChartData: points,
     };
@@ Expand All / @@ -70,7 +70,7 @@ const emptyPoint = [ @@
         color: 'red',
       },
     ];
-    const emptyChartPoints = {
+    export const emptyChartPoints = {
       chartTitle: 'EmptyAreaChart',
       lineChartData: emptyPoint,
     };
@@ Expand Down @@

packages/react-charting/src/components/AreaChart/AreaChartRTL.test.tsx

-Original file line number
+Diff line change
@@ -0,0 +1,22 @@
+    import * as React from 'react';
+    import { queryAllByAttribute, render, waitFor } from '@testing-library/react';
+    import { chartPoints, emptyChartPoints } from './AreaChart.test';
+    import { AreaChart } from './index';
+    describe('Area chart rendering', () => {
+      test('Should re-render the Area chart with data', async () => {
+        // Arrange
+        const { container, rerender } = render(<AreaChart data={emptyChartPoints} />);
+        const getById = queryAllByAttribute.bind(null, 'id');
+        // Assert
+        expect(container).toMatchSnapshot();
+        expect(getById(container, /_AreaChart_empty/i)).toHaveLength(1);
+        // Act
+        rerender(<AreaChart data={chartPoints} />);
+        await waitFor(() => {
+          // Assert
+          expect(container).toMatchSnapshot();
+          expect(getById(container, /_AreaChart_empty/i)).toHaveLength(0);
+        });
+      });
+    });

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing re-render issue for all charts when empty #28321

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!