Focus indicator bug in bar charts

change/@fluentui-react-charting-20c8ae70-f2a4-4ef4-89b6-7ca932f7481d.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Fixed the focus indicator bug in horizontal bar chart and multi stack bar chart",
+  "packageName": "@fluentui/react-charting",
+  "email": "yushsingla@microsoft.com",
+  "dependentChangeType": "patch"
+}

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

@@ -30,6 +30,7 @@ export interface IHorizontalBarChartState {
   yCalloutValue?: string;
   barCalloutProps?: IChartDataPoint;
   callOutAccessibilityData?: IAccessibilityProps;
+  barSpacingInPercent: number;
 }
 
 export class HorizontalBarChartBase extends React.Component<IHorizontalBarChartProps, IHorizontalBarChartState> {
@@ -40,6 +41,7 @@ export class HorizontalBarChartBase extends React.Component<IHorizontalBarChartP
   private _refArray: IRefArrayData[];
   private _calloutAnchorPoint: IChartDataPoint | null;
   private _isRTL: boolean = getRTL();
+  private barChartSvgRef: React.RefObject<SVGSVGElement>;
   private _emptyChartId: string;
 
   constructor(props: IHorizontalBarChartProps) {
@@ -54,12 +56,23 @@ export class HorizontalBarChartBase extends React.Component<IHorizontalBarChartP
       color: '',
       xCalloutValue: '',
       yCalloutValue: '',
+      barSpacingInPercent: 0,
     };
     this._refArray = [];
     this._uniqLineText = '_HorizontalLine_' + Math.random().toString(36).substring(7);
     this._hoverOff = this._hoverOff.bind(this);
     this._calloutId = getId('callout');
     this._emptyChartId = getId('_HBC_empty');
+    this.barChartSvgRef = React.createRef<SVGSVGElement>();
+  }
+
+  public componentDidMount(): void {
+    const svgWidth = this.barChartSvgRef.current?.getBoundingClientRect().width || 0;
+    const MARGIN_WIDTH_IN_PX = 2;
+    if (svgWidth) {
+      const currentBarSpacing = (MARGIN_WIDTH_IN_PX / svgWidth) * 100;
+      this.setState({ barSpacingInPercent: currentBarSpacing });
+    }
   }
 
   public render(): JSX.Element {
@@ -113,7 +126,7 @@ export class HorizontalBarChartBase extends React.Component<IHorizontalBarChartP
                 </FocusZone>
                 {points!.chartData![0].data && this._createBenchmark(points!)}
                 <FocusZone direction={FocusZoneDirection.horizontal} className={this._classNames.chartWrapper}>
-                  <svg className={this._classNames.chart} aria-label={points!.chartTitle}>
+                  <svg ref={this.barChartSvgRef} className={this._classNames.chart} aria-label={points!.chartTitle}>
                     <g
                       id={keyVal}
                       key={keyVal}
@@ -286,7 +299,18 @@ export class HorizontalBarChartBase extends React.Component<IHorizontalBarChartP
     );
   }
 
+  /**
+   * This functions returns an array of <rect> elements, which form the bars
+   * For each bar an x value, and a width needs to be specified
+   * The computations are done based on percentages
+   * Extra margin is also provided, in the x value to provide some spacing in between the bars
+   */
+
   private _createBars(data: IChartProps, palette: IPalette): JSX.Element[] {
+    const noOfBars =
+      data.chartData?.reduce((count: number, point: IChartDataPoint) => (count += (point.data || 0) > 0 ? 1 : 0), 0) ||
+      1;
+    const totalMarginPercent = this.state.barSpacingInPercent * (noOfBars - 1);
     const defaultPalette: string[] = [palette.blueLight, palette.blue, palette.blueMid, palette.red, palette.black];
     // calculating starting point of each bar and it's range
     const startingPoint: number[] = [];
@@ -312,7 +336,15 @@ export class HorizontalBarChartBase extends React.Component<IHorizontalBarChartP
       return sumOfPercent;
     });
 
-    const scalingRatio = sumOfPercent !== 0 ? sumOfPercent / 100 : 1;
+    /**
+     * The %age of the space occupied by the margin needs to subtracted
+     * while computing the scaling ratio, since the margins are not being
+     * scaled down, only the data is being scaled down from a higher percentage to lower percentage
+     * Eg: 95% of the space is taken by the bars, 5% by the margins
+     * Now if the sumOfPercent is 120% -> This needs to be scaled down to 95%, not 100%
+     * since that's only space available to the bars
+     */
+    const scalingRatio = sumOfPercent !== 0 ? (sumOfPercent - totalMarginPercent) / 100 : 1;
 
     const bars = data.chartData!.map((point: IChartDataPoint, index: number) => {
       const color: string = point.color ? point.color : defaultPalette[Math.floor(Math.random() * 4 + 1)];
@@ -359,7 +391,11 @@ export class HorizontalBarChartBase extends React.Component<IHorizontalBarChartP
       return (
         <rect
           key={index}
-          x={`${this._isRTL ? 100 - startingPoint[index] - value : startingPoint[index]}%`}
+          x={`${
+            this._isRTL
+              ? 100 - startingPoint[index] - value - index * this.state.barSpacingInPercent
+              : startingPoint[index] + index * this.state.barSpacingInPercent
+          }%`}
           y={0}
           data-is-focusable={point.legend !== '' ? true : false}
           width={value + '%'}

packages/react-charting/src/components/HorizontalBarChart/HorizontalBarChart.styles.ts

@@ -27,10 +27,7 @@ export const getHorizontalBarChartStyles = (props: IHorizontalBarChartStyleProps
       display: 'block',
       overflow: 'visible',
     },
-    barWrapper: {
-      stroke: theme.palette.white,
-      strokeWidth: 2,
-    },
+    barWrapper: {},
     chartTitle: {
       ...theme.fonts.small,
       display: 'flex',

packages/react-charting/src/components/HorizontalBarChart/__snapshots__/HorizontalBarChart.test.tsx.snap

@@ -137,10 +137,7 @@ exports[`HorizontalBarChart - mouse events Should render callout correctly on mo
               aria-label="2020/04/30, 94%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#004b50"
               height={12}
@@ -158,10 +155,7 @@ exports[`HorizontalBarChart - mouse events Should render callout correctly on mo
               aria-label="13457/15000."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={false}
               fill="#edebe9"
               height={12}
@@ -304,10 +298,7 @@ exports[`HorizontalBarChart - mouse events Should render callout correctly on mo
               aria-label="2020/04/30, 19%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#5c2d91"
               height={12}
@@ -325,10 +316,7 @@ exports[`HorizontalBarChart - mouse events Should render callout correctly on mo
               aria-label="14200/15000."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={false}
               fill="#edebe9"
               height={12}
@@ -647,10 +635,7 @@ exports[`HorizontalBarChart - mouse events Should render customized callout on m
               aria-label="2020/04/30, 94%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#004b50"
               height={12}
@@ -668,10 +653,7 @@ exports[`HorizontalBarChart - mouse events Should render customized callout on m
               aria-label="13457/15000."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={false}
               fill="#edebe9"
               height={12}
@@ -814,10 +796,7 @@ exports[`HorizontalBarChart - mouse events Should render customized callout on m
               aria-label="2020/04/30, 19%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#5c2d91"
               height={12}
@@ -835,10 +814,7 @@ exports[`HorizontalBarChart - mouse events Should render customized callout on m
               aria-label="14200/15000."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={false}
               fill="#edebe9"
               height={12}
@@ -1063,10 +1039,7 @@ exports[`HorizontalBarChart snapShot testing Should not render bar labels in abs
               aria-label="2020/04/30, 94%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#004b50"
               height={12}
@@ -1195,10 +1168,7 @@ exports[`HorizontalBarChart snapShot testing Should not render bar labels in abs
               aria-label="2020/04/30, 19%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#5c2d91"
               height={12}
@@ -1344,10 +1314,7 @@ exports[`HorizontalBarChart snapShot testing Should render absolute-scale varian
               aria-label="2020/04/30, 94%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#004b50"
               height={12}
@@ -1494,10 +1461,7 @@ exports[`HorizontalBarChart snapShot testing Should render absolute-scale varian
               aria-label="2020/04/30, 19%."
               className=
 
-                  {
-                    stroke-width: 2px;
-                    stroke: #ffffff;
-                  }
+
               data-is-focusable={true}
               fill="#5c2d91"
               height={12}

packages/react-charting/src/components/StackedBarChart/MultiStackedBarChart.base.tsx

@@ -38,6 +38,7 @@ export interface IMultiStackedBarChartState {
   dataPointCalloutProps?: IChartDataPoint;
   callOutAccessibilityData?: IAccessibilityProps;
   calloutLegend: string;
+  barSpacingInPercent: number;
 }
 
 export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarChartProps, IMultiStackedBarChartState> {
@@ -52,6 +53,7 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
   private _calloutAnchorPoint: IChartDataPoint | null;
   private _longestBarTotalValue: number;
   private _isRTL: boolean = getRTL();
+  private barChartSvgRef: React.RefObject<SVGSVGElement>;
   private _emptyChartId: string;
   private _barId: string;
   private _barIdPlaceholderPartToWhole: string;
@@ -70,16 +72,27 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
       xCalloutValue: '',
       yCalloutValue: '',
       calloutLegend: '',
+      barSpacingInPercent: 0,
     };
     this._onLeave = this._onLeave.bind(this);
     this._onBarLeave = this._onBarLeave.bind(this);
     this._calloutId = getId('callout');
+    this.barChartSvgRef = React.createRef<SVGSVGElement>();
     this._emptyChartId = getId('_MSBC_empty');
     this._barId = getId('_MSBC_rect_');
     this._barIdPlaceholderPartToWhole = getId('_MSBC_rect_partToWhole_');
     this._barIdEmpty = getId('_MSBC_rect_empty');
   }
 
+  public componentDidMount(): void {
+    const svgWidth = this.barChartSvgRef.current?.getBoundingClientRect().width || 0;
+    const MARGIN_WIDTH_IN_PX = 2;
+    if (svgWidth) {
+      const currentBarSpacing = (MARGIN_WIDTH_IN_PX / svgWidth) * 100;
+      this.setState({ barSpacingInPercent: currentBarSpacing });
+    }
+  }
+
   public render(): JSX.Element {
     if (!this._isChartEmpty()) {
       const { data, theme, culture } = this.props;
@@ -155,6 +168,13 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
     );
   }
 
+  /**
+   * The Create bar functions returns an array of <rect> elements, which form the bars
+   * For each bar an x value, and a width needs to be specified
+   * The computations are done based on percentages
+   * Extra margin is also provided, in the x value to provide some spacing
+   */
+
   private _createBarsAndLegends(
     data: IChartProps,
     barHeight: number,
@@ -163,6 +183,10 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
     hideDenominator: boolean,
     href?: string,
   ): JSX.Element {
+    const noOfBars =
+      data.chartData?.reduce((count: number, point: IChartDataPoint) => (count += (point.data || 0) > 0 ? 1 : 0), 0) ||
+      1;
+    const totalMarginPercent = this.state.barSpacingInPercent * (noOfBars - 1);
     const { culture } = this.props;
     const defaultPalette: string[] = [palette.blueLight, palette.blue, palette.blueMid, palette.red, palette.black];
     // calculating starting point of each bar and it's range
@@ -177,7 +201,9 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
     let sumOfPercent = 0;
     data.chartData!.map((point: IChartDataPoint, index: number) => {
       const pointData = point.data ? point.data : 0;
-      value = (pointData / total) * 100 ? (pointData / total) * 100 : 0;
+      const currValue = (pointData / total) * 100;
+      value = currValue ? currValue : 0;
+
       if (value < 1 && value !== 0) {
         value = 1;
       } else if (value > 99 && value !== 100) {
@@ -201,7 +227,16 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
       sumOfPercent += value;
     }
 
-    const scalingRatio = sumOfPercent !== 0 ? sumOfPercent / 100 : 1;
+    /**
+     * The %age of the space occupied by the margin needs to subtracted
+     * while computing the scaling ratio, since the margins are not being
+     * scaled down, only the data is being scaled down from a higher percentage to lower percentage
+     * Eg: 95% of the space is taken by the bars, 5% by the margins
+     * Now if the sumOfPercent is 120% -> This needs to be scaled down to 95%, not 100%
+     * since that's only space available to the bars
+     */
+
+    const scalingRatio = sumOfPercent !== 0 ? sumOfPercent / (100 - totalMarginPercent) : 1;
 
     let prevPosition = 0;
     let value = 0;
@@ -257,7 +292,11 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
           <rect
             key={index}
             id={`${this._barId}-${index}`}
-            x={`${this._isRTL ? 100 - startingPoint[index] - value : startingPoint[index]}%`}
+            x={`${
+              this._isRTL
+                ? 100 - startingPoint[index] - value - this.state.barSpacingInPercent * index
+                : startingPoint[index] + this.state.barSpacingInPercent * index
+            }%`}
             y={0}
             width={value + '%'}
             height={barHeight}
@@ -273,9 +312,10 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
             key="text"
             x={`${
               this._isRTL
-                ? 100 - (startingPoint[startingPoint.length - 1] || 0) - value
-                : (startingPoint[startingPoint.length - 1] || 0) + value
+                ? 100 - (startingPoint[startingPoint.length - 1] || 0) - value - totalMarginPercent
+                : (startingPoint[startingPoint.length - 1] || 0) + value + totalMarginPercent
             }%`}
+            textAnchor={this._isRTL ? 'end' : 'start'}
             y={barHeight / 2}
             dominantBaseline="central"
             transform={`translate(${this._isRTL ? -4 : 4})`}
@@ -362,7 +402,7 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
         </FocusZone>
         <FocusZone direction={FocusZoneDirection.horizontal}>
           <div className={this._classNames.chartWrapper}>
-            <svg className={this._classNames.chart} aria-label={data?.chartTitle}>
+            <svg ref={this.barChartSvgRef} className={this._classNames.chart} aria-label={data?.chartTitle}>
               {bars}
             </svg>
           </div>