From 26ce5d6cddc160738ba5cbecd136de044d6b0240 Mon Sep 17 00:00:00 2001 From: Makoto Morimoto Date: Mon, 26 Oct 2020 13:26:29 -0700 Subject: [PATCH] Charting: Fixing uncaught type error in VerticalStackedBarChart and made minor edits to margins (#15699) Cherry-pick of #15447. --- ...8-verticalStackedBarChartMinorChanges.json | 8 ++ .../CommonComponents/CartesianChart.base.tsx | 19 +-- .../CommonComponents/CartesianChart.types.ts | 2 + .../src/components/Legends/Legends.base.tsx | 2 - .../VerticalStackedBarChart.base.tsx | 111 +++++++----------- .../react-charting/src/utilities/utilities.ts | 13 +- ...VerticalStackedBarChart.Styled.Example.tsx | 19 ++- 7 files changed, 85 insertions(+), 89 deletions(-) create mode 100644 change/@fluentui-react-charting-2020-10-26-12-32-58-verticalStackedBarChartMinorChanges.json diff --git a/change/@fluentui-react-charting-2020-10-26-12-32-58-verticalStackedBarChartMinorChanges.json b/change/@fluentui-react-charting-2020-10-26-12-32-58-verticalStackedBarChartMinorChanges.json new file mode 100644 index 00000000000000..a7b118b4cea27a --- /dev/null +++ b/change/@fluentui-react-charting-2020-10-26-12-32-58-verticalStackedBarChartMinorChanges.json @@ -0,0 +1,8 @@ +{ + "type": "prerelease", + "comment": "Charting: Fixing uncaught type error in VerticalStackedBarChart and made minor edits to margins.", + "packageName": "@fluentui/react-charting", + "email": "humbertomakotomorimoto@gmail.com", + "dependentChangeType": "patch", + "date": "2020-10-26T19:32:58.064Z" +} diff --git a/packages/react-charting/src/components/CommonComponents/CartesianChart.base.tsx b/packages/react-charting/src/components/CommonComponents/CartesianChart.base.tsx index 31d43f3c9de6d2..e70edb2cc8f4df 100644 --- a/packages/react-charting/src/components/CommonComponents/CartesianChart.base.tsx +++ b/packages/react-charting/src/components/CommonComponents/CartesianChart.base.tsx @@ -15,7 +15,6 @@ import { createDateXAxis, createYAxis, createStringYAxis, - additionalMarginRight, IMargins, getMinMaxOfYAxis, XAxisTypes, @@ -60,11 +59,17 @@ export class CartesianChartBase extends React.Component diff --git a/packages/react-charting/src/components/CommonComponents/CartesianChart.types.ts b/packages/react-charting/src/components/CommonComponents/CartesianChart.types.ts index a2c317bbc05c69..1ef28454e5d1a1 100644 --- a/packages/react-charting/src/components/CommonComponents/CartesianChart.types.ts +++ b/packages/react-charting/src/components/CommonComponents/CartesianChart.types.ts @@ -157,6 +157,8 @@ export interface ICartesianChartProps { /** * Margins for the chart + * @default `{ top: 20, bottom: 35, left: 40, right: 20 }` + * To avoid edge cuttings to the chart, we recommend you use default values or greater then default values */ margins?: IMargins; diff --git a/packages/react-charting/src/components/Legends/Legends.base.tsx b/packages/react-charting/src/components/Legends/Legends.base.tsx index f738c9507b9fd8..058028e9cc2166 100644 --- a/packages/react-charting/src/components/Legends/Legends.base.tsx +++ b/packages/react-charting/src/components/Legends/Legends.base.tsx @@ -298,7 +298,6 @@ export class LegendsBase extends React.Component { >
(this._hoverCardRef = rootElem)} {...(allowFocusOnLegends && { 'aria-expanded': this.state.isHoverCardVisible, @@ -378,7 +377,6 @@ export class LegendsBase extends React.Component { {...(data.nativeButtonProps && { ...data.nativeButtonProps })} key={index} className={classNames.legend} - /* eslint-disable react/jsx-no-bind */ onClick={onClickHandler} onMouseOver={onHoverHandler} onMouseOut={onMouseOut} diff --git a/packages/react-charting/src/components/VerticalStackedBarChart/VerticalStackedBarChart.base.tsx b/packages/react-charting/src/components/VerticalStackedBarChart/VerticalStackedBarChart.base.tsx index 3e3c72ccb70ea4..80183448dc2678 100644 --- a/packages/react-charting/src/components/VerticalStackedBarChart/VerticalStackedBarChart.base.tsx +++ b/packages/react-charting/src/components/VerticalStackedBarChart/VerticalStackedBarChart.base.tsx @@ -13,7 +13,6 @@ import { IChildProps, IDataPoint, IMargins, - IRefArrayData, IVerticalStackedBarChartProps, IVerticalStackedBarChartStyleProps, IVerticalStackedBarChartStyles, @@ -21,16 +20,16 @@ import { IVSChartDataPoint, } from '../../index'; import { FocusZoneDirection } from '@fluentui/react-focus'; -import { ChartTypes, XAxisTypes, additionalMarginRight } from '../../utilities/index'; +import { ChartTypes, XAxisTypes } from '../../utilities/index'; const getClassNames = classNamesFunction(); type NumericAxis = D3Axis; type NumericScale = D3ScaleLinear; type StringScale = D3ScaleLinear; const COMPONENT_NAME = 'VERTICAL STACKED BAR CHART'; -interface IBarRef { - index: number | string; - refElement: SVGGElement; + +interface IRefArrayData { + refElement?: SVGGElement | null; } export interface IVerticalStackedBarChartState extends IBasestate { @@ -50,8 +49,6 @@ export class VerticalStackedBarChartBase extends React.Component< private _barWidth: number; private _calloutId: string; private _classNames: IProcessedStyleSet; - private _refArray: IRefArrayData[]; - private _barRefArray: IBarRef[]; private _colors: string[]; private margins: IMargins; private _isRtl: boolean = getRTL(); @@ -78,8 +75,6 @@ export class VerticalStackedBarChartBase extends React.Component< this._onLegendLeave = this._onLegendLeave.bind(this); this._handleMouseOut = this._handleMouseOut.bind(this); this._calloutId = getId('callout'); - this._refArray = []; - this._barRefArray = []; this._adjustProps(); this._dataset = this._createDataSetLayer(); } @@ -280,14 +275,6 @@ export class VerticalStackedBarChartBase extends React.Component< ); } - private _refCallback(rectElement: SVGRectElement, legendTitle: string, index: number): void { - this._refArray[index] = { index: legendTitle, refElement: rectElement }; - } - - private _barRefCallback = (barElement: SVGGElement, index: number, xAxisPoint: number | string): void => { - this._barRefArray[index] = { index: xAxisPoint, refElement: barElement }; - }; - private _onRectHover( xAxisPoint: string, point: IVSChartDataPoint, @@ -327,44 +314,36 @@ export class VerticalStackedBarChartBase extends React.Component< }); }; - private _onRectFocus(point: IVSChartDataPoint, xAxisPoint: string, color: string, refArrayIndexNumber: number): void { + private _onRectFocus(point: IVSChartDataPoint, xAxisPoint: string, color: string, ref: IRefArrayData): void { if ( this.state.isLegendSelected === false || (this.state.isLegendSelected && this.state.selectedLegendTitle === point.legend) ) { - this._refArray.forEach((obj: IRefArrayData, index: number) => { - if (obj.index === point.legend && refArrayIndexNumber === index) { - this.setState({ - refSelected: obj.refElement, - isCalloutVisible: true, - selectedLegendTitle: point.legend, - dataForHoverCard: point.data, - color: color, - xCalloutValue: point.xAxisCalloutData ? point.xAxisCalloutData : xAxisPoint, - yCalloutValue: point.yAxisCalloutData, - dataPointCalloutProps: point, - }); - } + this.setState({ + refSelected: ref.refElement, + isCalloutVisible: true, + selectedLegendTitle: point.legend, + dataForHoverCard: point.data, + color: color, + xCalloutValue: point.xAxisCalloutData ? point.xAxisCalloutData : xAxisPoint, + yCalloutValue: point.yAxisCalloutData, + dataPointCalloutProps: point, }); } } - private _onStackFocus = (xAxisPoint: string | number): void => { + private _onStackFocus = (xAxisPoint: string | number, groupRef: IRefArrayData): void => { const found = find( this._points, (sinlgePoint: { xAxisPoint: string | number; chartData: IVSChartDataPoint[] }) => sinlgePoint.xAxisPoint === xAxisPoint, ); - this._barRefArray.forEach((obj: IBarRef) => { - if (obj.index === xAxisPoint) { - this.setState({ - refSelected: obj.refElement, - isCalloutVisible: true, - YValueHover: found!.chartData, - hoverXValue: xAxisPoint, - stackCalloutProps: found!, - }); - } + this.setState({ + refSelected: groupRef.refElement, + isCalloutVisible: true, + YValueHover: found!.chartData, + hoverXValue: xAxisPoint, + stackCalloutProps: found!, }); }; @@ -374,9 +353,9 @@ export class VerticalStackedBarChartBase extends React.Component< }); }; - private _redirectToUrl(): void { + private _redirectToUrl = (): void => { this.props.href ? (window.location.href = this.props.href) : ''; - } + }; private _getYMax(dataset: IDataPoint[]) { return Math.max(d3Max(dataset, (point: IDataPoint) => point.y)!, this.props.yMaxValue || 0); @@ -391,10 +370,18 @@ export class VerticalStackedBarChartBase extends React.Component< let startingPointOfY = 0; const isCalloutForStack = this.props.isCalloutForStack || false; - const singleBar = singleChartData.chartData.map((point: IVSChartDataPoint, index: number) => { + let xPoint: number | string; + if (this._isNumeric) { + xPoint = xBarScale(singleChartData.xAxisPoint as number); + } else { + xPoint = xBarScale(indexNumber); + } + // Removing datapoints with zero data + const nonZeroBars = singleChartData.chartData.filter(point => point.data > 0); + const singleBar = nonZeroBars.map((point: IVSChartDataPoint, index: number) => { startingPointOfY = startingPointOfY + point.data; const color = point.color ? point.color : this._colors[index]; - const refArrayIndexNumber = indexNumber * singleChartData.chartData.length + index; + const ref: IRefArrayData = {}; let shouldHighlight = true; if (this.state.isLegendHovered || this.state.isLegendSelected) { @@ -405,20 +392,13 @@ export class VerticalStackedBarChartBase extends React.Component< shouldHighlight: shouldHighlight, href: this.props.href, }); - let xPoint; - if (this._isNumeric) { - xPoint = xBarScale(singleChartData.xAxisPoint as number); - } else { - xPoint = xBarScale(indexNumber); - } - const rectFocusProps = !isCalloutForStack && { 'data-is-focusable': true, 'aria-labelledby': this._calloutId, onMouseOver: this._onRectHover.bind(this, singleChartData.xAxisPoint, point), onMouseMove: this._onRectHover.bind(this, singleChartData.xAxisPoint, point), onMouseLeave: this._handleMouseOut, - onFocus: this._onRectFocus.bind(this, point, singleChartData.xAxisPoint, color, refArrayIndexNumber), + onFocus: this._onRectFocus.bind(this, point, singleChartData.xAxisPoint, color, ref), onBlur: this._handleMouseOut, onClick: this._redirectToUrl, }; @@ -431,19 +411,18 @@ export class VerticalStackedBarChartBase extends React.Component< width={this._barWidth} height={Math.max(yBarScale(point.data), 0)} fill={color} - ref={(e: SVGRectElement) => { - this._refCallback(e, point.legend, refArrayIndexNumber); - }} + ref={e => (ref.refElement = e)} {...rectFocusProps} /> ); }); + const groupRef: IRefArrayData = {}; const stackFocusProps = isCalloutForStack && { 'data-is-focusable': true, onMouseOver: this._onStackHover.bind(this, singleChartData.xAxisPoint), onMouseMove: this._onStackHover.bind(this, singleChartData.xAxisPoint), onMouseLeave: this._handleMouseOut, - onFocus: this._onStackFocus.bind(this, singleChartData.xAxisPoint), + onFocus: this._onStackFocus.bind(this, singleChartData.xAxisPoint, groupRef), onBlur: this._handleMouseOut, onClick: this._redirectToUrl, }; @@ -452,9 +431,7 @@ export class VerticalStackedBarChartBase extends React.Component< key={indexNumber} id={`${indexNumber}-singleBar`} data-is-focusable={this.props.isCalloutForStack} - ref={(gElement: SVGGElement) => { - this._barRefCallback(gElement, indexNumber, singleChartData.xAxisPoint); - }} + ref={e => (groupRef.refElement = e)} {...stackFocusProps} > {singleBar} @@ -471,10 +448,7 @@ export class VerticalStackedBarChartBase extends React.Component< const xBarScale = d3ScaleLinear() .domain(this._isRtl ? [xMax, 0] : [0, xMax]) .nice() - .range([ - this.margins.left!, - containerWidth - this.margins.right! - this._barWidth - (this._isRtl ? additionalMarginRight : 0), - ]); + .range([this.margins.left!, containerWidth - this.margins.right! - this._barWidth]); const yBarScale = d3ScaleLinear() .domain([0, yMax]) .range([0, containerHeight - this.margins.bottom! - this.margins.top!]); @@ -485,16 +459,11 @@ export class VerticalStackedBarChartBase extends React.Component< private _createStringBars = (containerHeight: number, containerWidth: number): JSX.Element[] => { const yMax = this._getYMax(this._dataset); const endpointDistance = 0.5 * ((containerWidth - this.margins.right!) / this._dataset.length); - const xBarScale = d3ScaleLinear() .domain(this._isRtl ? [this._dataset.length - 1, 0] : [0, this._dataset.length - 1]) .range([ this.margins.left! + endpointDistance - 0.5 * this._barWidth, - containerWidth - - this.margins.right! - - endpointDistance - - 0.5 * this._barWidth - - (this._isRtl ? additionalMarginRight : 0), + containerWidth - this.margins.right! - endpointDistance - 0.5 * this._barWidth, ]); const yBarScale = d3ScaleLinear() .domain([0, yMax]) diff --git a/packages/react-charting/src/utilities/utilities.ts b/packages/react-charting/src/utilities/utilities.ts index 7bcbde54489611..d44875b51975ec 100644 --- a/packages/react-charting/src/utilities/utilities.ts +++ b/packages/react-charting/src/utilities/utilities.ts @@ -46,18 +46,22 @@ export interface IWrapLabelProps { export interface IMargins { /** * left margin for the chart. + * @default 40 */ left?: number; /** * Right margin for the chart. + * @default 20 */ right?: number; /** * Top margin for the chart. + * @default 20 */ top?: number; /** * Bottom margin for the chart. + * @default 35 */ bottom?: number; } @@ -118,7 +122,6 @@ export interface IFitContainerParams { legendContainer: HTMLDivElement; container: HTMLDivElement | null | HTMLElement; } -export const additionalMarginRight: number = 20; /** * Create Numeric X axis @@ -530,7 +533,7 @@ export function domainRangeOfDateForAreaChart( }); const rStartValue = margins.left!; - const rEndValue = width - margins.right! - (isRTL ? additionalMarginRight : 0); + const rEndValue = width - margins.right!; return isRTL ? { dStartValue: lDate, dEndValue: sDate, rStartValue, rEndValue } @@ -564,7 +567,7 @@ export function domainRangeOfNumericForAreaChart( })!; const rStartValue = margins.left!; - const rEndValue = width - margins.right! - (isRTL ? additionalMarginRight : 0); + const rEndValue = width - margins.right!; return isRTL ? { dStartValue: xMax, dEndValue: xMin, rStartValue, rEndValue } @@ -584,7 +587,7 @@ export function domainRangeOfNumericForAreaChart( */ export function domainRangeOfStrForVSBC(margins: IMargins, width: number, isRTL: boolean): IDomainNRange { const rMin = margins.left!; - const rMax = width - margins.right! - (isRTL ? additionalMarginRight : 0); + const rMax = width - margins.right!; return isRTL ? { dStartValue: 0, dEndValue: 0, rStartValue: rMax, rEndValue: rMin } @@ -628,7 +631,7 @@ export function domainRangeOfVSBCNumeric( const xMax = d3Max(points, (point: IDataPoint) => point.x as number)!; // barWidth / 2 - for to get tick middle of the bar const rMax = margins.left! + barWidth / 2; - const rMin = width - margins.right! - barWidth / 2 - (isRTL ? additionalMarginRight : 0); + const rMin = width - margins.right! - barWidth / 2; return isRTL ? { dStartValue: xMax, dEndValue: xMin, rStartValue: rMax, rEndValue: rMin } : { dStartValue: xMin, dEndValue: xMax, rStartValue: rMax, rEndValue: rMin }; diff --git a/packages/react-examples/src/react-charting/VerticalStackedBarChart/VerticalStackedBarChart.Styled.Example.tsx b/packages/react-examples/src/react-charting/VerticalStackedBarChart/VerticalStackedBarChart.Styled.Example.tsx index 6c4e5ca8fe7433..e6fcccbdfeb641 100644 --- a/packages/react-examples/src/react-charting/VerticalStackedBarChart/VerticalStackedBarChart.Styled.Example.tsx +++ b/packages/react-examples/src/react-charting/VerticalStackedBarChart/VerticalStackedBarChart.Styled.Example.tsx @@ -37,7 +37,7 @@ export class VerticalStackedBarChartStyledExample extends React.Component<{}, IV const firstChartPoints: IVSChartDataPoint[] = [ { legend: 'Metadata1', data: 40, color: DefaultPalette.accent }, { legend: 'Metadata2', data: 5, color: DefaultPalette.blueMid }, - { legend: 'Metadata3', data: 15, color: DefaultPalette.blueLight }, + { legend: 'Metadata3', data: 0, color: DefaultPalette.blueLight }, ]; const secondChartPoints: IVSChartDataPoint[] = [ @@ -74,6 +74,11 @@ export class VerticalStackedBarChartStyledExample extends React.Component<{}, IV const customStyles: IVerticalStackedBarChartProps['styles'] = () => { return { + xAxis: { + selectors: { + text: { fill: 'black', fontSize: '8px' }, + }, + }, chart: { paddingBottom: '45px', }, @@ -99,6 +104,7 @@ export class VerticalStackedBarChartStyledExample extends React.Component<{}, IV height={this.state.height} width={this.state.width} yAxisTickCount={10} + // Just test link href={'www.google.com'} // eslint-disable-next-line react/jsx-no-bind styles={customStyles} @@ -109,7 +115,12 @@ export class VerticalStackedBarChartStyledExample extends React.Component<{}, IV }} // eslint-disable-next-line react/jsx-no-bind yAxisTickFormat={(x: number | string) => `${x} h`} - margins={{ left: 50 }} + margins={{ + bottom: 0, + top: 0, + left: 0, + right: 0, + }} legendProps={{ allowFocusOnLegends: true, styles: { @@ -118,8 +129,8 @@ export class VerticalStackedBarChartStyledExample extends React.Component<{}, IV }, }, }} - // eslint-disable-next-line react/jsx-no-bind - onRenderCalloutPerDataPoint={props => + // eslint-disable-next-line react/jsx-no-bind, @typescript-eslint/no-explicit-any + onRenderCalloutPerDataPoint={(props: any) => props ? (