Skip to content

Commit

Permalink
fix(charts): add individual padding vars for donut charts
Browse files Browse the repository at this point in the history 
Fixes #2528
  • Loading branch information
@dlabrecq
dlabrecq committed Jul 17, 2019
1 parent 7ae5b17 commit 6174c09
Show file tree
Hide file tree
Showing 15 changed files with 173 additions and 1,495 deletions.
10 changes: 6 additions & 4 deletions packages/patternfly-4/react-charts/src/components/Chart/Chart.tsx
View file
Expand Up @@ -23,8 +23,7 @@ import {
ChartLegendWrapper
} from "../ChartLegend";
import { ChartCommonStyles, ChartThemeDefinition } from '../ChartTheme';
import { getTheme } from '../ChartUtils';
import { getPaddingForSide } from '../ChartUtils/chart-padding';
import { getPaddingForSide, getTheme } from '../ChartUtils';

/**
* See https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/victory/index.d.ts
Expand Down Expand Up @@ -169,6 +168,8 @@ export interface ChartProps extends VictoryChartProps {
* Because Victory renders responsive containers, the width and height props do not determine the width and
* height of the chart in number of pixels, but instead define an aspect ratio for the chart. The exact number of
* pixels will depend on the size of the container the chart is rendered into.
*
* Typically, the parent container is set to the same width in order to maintain the aspect ratio.
*/
height?: number;
/**
Expand Down Expand Up @@ -347,6 +348,8 @@ export interface ChartProps extends VictoryChartProps {
* Because Victory renders responsive containers, the width and height props do not determine the width and
* height of the chart in number of pixels, but instead define an aspect ratio for the chart. The exact number of
* pixels will depend on the size of the container the chart is rendered into.
*
* Typically, the parent container is set to the same width in order to maintain the aspect ratio.
*/
width?: number;
}
Expand All @@ -372,7 +375,6 @@ export const Chart: React.FunctionComponent<ChartProps> = ({
width = theme.chart.width,
...rest
}: ChartProps) => {

const defaultPadding = {
bottom: getPaddingForSide('bottom', padding, theme.chart.padding),
left: getPaddingForSide('left', padding, theme.chart.padding),
Expand Down Expand Up @@ -405,7 +407,7 @@ export const Chart: React.FunctionComponent<ChartProps> = ({
return null;
}
let dx = 0;
let dy = defaultPadding.top || 0;
let dy = defaultPadding.top;
if (legendPosition === ChartLegendPosition.bottom) {
dy += ChartCommonStyles.legend.margin;
} else if (legendPosition === ChartLegendPosition.bottomLeft) {
Expand Down
52 changes: 30 additions & 22 deletions packages/patternfly-4/react-charts/src/components/ChartDonut/ChartDonut.tsx
View file
Expand Up @@ -16,8 +16,7 @@ import { ChartContainer } from '../ChartContainer';
import { ChartLabel } from '../ChartLabel';
import { ChartPie, ChartPieLegendPosition, ChartPieProps } from '../ChartPie';
import { ChartCommonStyles, ChartDonutStyles, ChartThemeDefinition } from '../ChartTheme';
import { ChartTooltip } from '../ChartTooltip';
import { getLabelX, getLabelY } from "../ChartUtils";
import { getLabelX, getLabelY } from '../ChartUtils';

export enum ChartDonutLabelPosition {
centroid = 'centroid',
Expand Down Expand Up @@ -130,8 +129,7 @@ export interface ChartDonutProps extends ChartPieProps {
*/
dataComponent?: React.ReactElement<any>;
/**
* Specifies the height of the donut chart. This value should be given as a
* number of pixels.
* Specifies the height of the donut chart. This value should be given as a number of pixels.
*
* Because Victory renders responsive containers, the width and height props do not determine the width and
* height of the chart in number of pixels, but instead define an aspect ratio for the chart. The exact number of
Expand All @@ -144,7 +142,7 @@ export interface ChartDonutProps extends ChartPieProps {
* legends within the same SVG. However, donutHeight (not height) may need to be set in order to adjust the donut
* height.
*
* The innerRadius may also need to be set when changing the donut size.
* Note: innerRadius may need to be set when using this property.
*/
donutHeight?: number;
/**
Expand All @@ -156,8 +154,7 @@ export interface ChartDonutProps extends ChartPieProps {
*/
donutDy?: number;
/**
* Specifies the width of the donut chart. This value should be given as a
* number of pixels.
* Specifies the width of the donut chart. This value should be given as a number of pixels.
*
* Because Victory renders responsive containers, the width and height props do not determine the width and
* height of the chart in number of pixels, but instead define an aspect ratio for the chart. The exact number of
Expand All @@ -169,7 +166,7 @@ export interface ChartDonutProps extends ChartPieProps {
* By default, donutWidth is the min. of either height or width. This covers most use cases in order to accommodate
* legends within the same SVG. However, donutWidth (not width) may need to be set in order to adjust the donut width.
*
* The innerRadius may also need to be set when changing the donut size.
* Note: innerRadius may need to be set when using this property.
*/
donutWidth?: number;
/**
Expand Down Expand Up @@ -241,7 +238,13 @@ export interface ChartDonutProps extends ChartPieProps {
* height of the chart in number of pixels, but instead define an aspect ratio for the chart. The exact number of
* pixels will depend on the size of the container the chart is rendered into.
*
* Note: innerRadius may need to be set when using this property.
* Note: When adding a legend, height (the overall SVG height) may need to be larger than donutHeight (the donut size)
* in order to accommodate the extra legend.
*
* By default, donutHeight is the min. of either height or width. This covers most use cases in order to accommodate
* legends within the same SVG. However, donutHeight (not height) may need to be set in order to adjust the donut height.
*
* Typically, the parent container is set to the same width in order to maintain the aspect ratio.
*/
height?: number;
/**
Expand Down Expand Up @@ -339,12 +342,13 @@ export interface ChartDonutProps extends ChartPieProps {
* the edge of the chart and any rendered child components. This prop can be given
* as a number or as an object with padding specified for top, bottom, left
* and right.
*
* Note: innerRadius may need to be set when using this property.
*/
padding?: PaddingProps;
/**
* Specifies the radius of the chart. If this property is not provided it is computed
* from width, height, and padding props
*
*/
radius?: number;
/**
Expand Down Expand Up @@ -436,14 +440,19 @@ export interface ChartDonutProps extends ChartPieProps {
*/
titleComponent?: React.ReactElement<any>;
/**
* Specifies the width of the svg viewBox of the chart container. This value should be given as a
* number of pixels.
* Specifies the width of the svg viewBox of the chart container. This value should be given as a number of pixels.
*
* Because Victory renders responsive containers, the width and height props do not determine the width and
* height of the chart in number of pixels, but instead define an aspect ratio for the chart. The exact number of
* pixels will depend on the size of the container the chart is rendered into.
*
* Note: innerRadius may need to be set when using this property.
* Note: When adding a legend, width (the overall SVG width) may need to be larger than donutWidth (the donut size)
* in order to accommodate the extra legend.
*
* By default, donutWidth is the min. of either height or width. This covers most use cases in order to accommodate
* legends within the same SVG. However, donutWidth (not width) may need to be set in order to adjust the donut width.
*
* Typically, the parent container is set to the same width in order to maintain the aspect ratio.
*/
width?: number;
/**
Expand Down Expand Up @@ -489,15 +498,15 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({

// destructure last
theme = getDonutTheme(themeColor, themeVariant),

capHeight = 1.1,
height = theme.pie.height,
width = theme.pie.width,
donutHeight = Math.min(height, width),
donutWidth = Math.min(height, width, donutHeight),
donutWidth = Math.min(height, width),
innerRadius = (Math.min(donutHeight, donutWidth) - 34) / 2,
...rest
}: ChartDonutProps) => {
const donutSize = Math.min(donutHeight, donutWidth);

// Returns subtitle
const getSubTitle = () => {
Expand All @@ -511,14 +520,14 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({
textAnchor: subTitlePosition === 'right' ? 'start' : 'middle',
verticalAnchor: 'middle',
x: getLabelX({
chartWidth: donutWidth,
chartWidth: donutSize,
dx: subTitleDx,
labelPosition: subTitlePosition,
legendPosition,
svgWidth: width
}),
y: getLabelY({
chartHeight: donutHeight,
chartHeight: donutSize,
dy: subTitleDy,
labelPosition: subTitlePosition
}),
Expand All @@ -540,14 +549,14 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({
textAnchor: 'middle',
verticalAnchor: 'middle',
x: getLabelX({
chartWidth: donutWidth,
chartWidth: donutSize,
dx: donutDx,
labelPosition: 'center',
legendPosition,
svgWidth: width
}),
y: getLabelY({
chartHeight: donutHeight,
chartHeight: donutSize,
dy: donutDy,
labelPosition: 'center'
}),
Expand All @@ -559,12 +568,11 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({
<ChartPie
height={height}
innerRadius={innerRadius > 0 ? innerRadius : 0}
labelComponent={<ChartTooltip theme={theme} />}
legendPosition={legendPosition}
pieDx={donutDx}
pieDy={donutDy}
pieHeight={donutHeight}
pieWidth={donutWidth}
pieHeight={donutSize}
pieWidth={donutSize}
standalone={false}
theme={theme}
width={width}
Expand Down

0 comments on commit 6174c09

Please sign in to comment.