Skip to content

Commit

Permalink
chore(charts): refactor how skeleton theme is applied
Browse files Browse the repository at this point in the history
  • Loading branch information
dlabrecq committed May 8, 2024
1 parent 2d19e26 commit 0efcae1
Show file tree
Hide file tree
Showing 44 changed files with 928 additions and 755 deletions.
9 changes: 4 additions & 5 deletions packages/react-charts/src/components/Chart/Chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,7 @@ export const Chart: React.FunctionComponent<ChartProps> = ({
...(name && { name: `${name}-${(legendComponent as any).type.displayName}` }),
orientation: legendOrientation,
theme,
themeColor,
...(legendDirection === 'rtl' && {
dataComponent: legendComponent.props.dataComponent ? (
React.cloneElement(legendComponent.props.dataComponent, { transform: `translate(${legendXOffset})` })
Expand All @@ -552,11 +553,7 @@ export const Chart: React.FunctionComponent<ChartProps> = ({
labelComponent: legendComponent.props.labelComponent ? (
React.cloneElement(legendComponent.props.labelComponent, { direction: 'rtl', dx: legendXOffset - 30 })
) : (
<ChartLabel
direction="rtl"
dx={legendXOffset - 30}
backgroundStyle={theme.skeleton ? theme.skeleton.backgroundStyle : undefined} // override backgroundStyle
/>
<ChartLabel direction="rtl" dx={legendXOffset - 30} />
)
}),
...legendComponent.props
Expand Down Expand Up @@ -603,6 +600,7 @@ export const Chart: React.FunctionComponent<ChartProps> = ({
padding: defaultPadding,
position: legendPosition,
theme,
themeColor,
width,
...(defaultPatternScale && { patternScale: defaultPatternScale })
});
Expand All @@ -621,6 +619,7 @@ export const Chart: React.FunctionComponent<ChartProps> = ({
name: `${name}-${(child as any).type.displayName}-${index}`
}),
theme,
themeColor,
...childProps,
...((child as any).type.displayName === 'ChartPie' && {
data: mergePatternData(childProps.data, defaultPatternScale)
Expand Down
8 changes: 5 additions & 3 deletions packages/react-charts/src/components/ChartAxis/ChartAxis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { VictoryAxis, VictoryAxisProps, VictoryAxisTTargetType } from 'victory-a
import { ChartContainer } from '../ChartContainer/ChartContainer';
import { ChartLabel } from '../ChartLabel/ChartLabel';
import { ChartThemeDefinition } from '../ChartTheme/ChartTheme';
import { getTheme } from '../ChartUtils/chart-theme';
import { getComponentTheme, getTheme } from '../ChartUtils/chart-theme';
import { getAxisTheme } from '../ChartUtils/chart-theme-types';

/**
Expand Down Expand Up @@ -451,6 +451,8 @@ export const ChartAxis: React.FunctionComponent<ChartAxisProps> = ({
theme = getTheme(themeColor),
...rest
}: ChartAxisProps) => {
const componentTheme = getComponentTheme(themeColor);

// Clone so users can override container props
const container = React.cloneElement(containerComponent, {
theme,
Expand All @@ -463,7 +465,7 @@ export const ChartAxis: React.FunctionComponent<ChartAxisProps> = ({
id: () => `${name}-${(axisLabelComponent as any).type.displayName}`
}),
...axisLabelComponent.props,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});

const getTickLabelComponent = () =>
Expand All @@ -472,7 +474,7 @@ export const ChartAxis: React.FunctionComponent<ChartAxisProps> = ({
id: (props: any) => `${name}-${(tickLabelComponent as any).type.displayName}-${props.index}`
}),
...tickLabelComponent.props,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});

// Note: containerComponent is required for theme
Expand Down
10 changes: 2 additions & 8 deletions packages/react-charts/src/components/ChartBullet/ChartBullet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,6 @@ export const ChartBullet: React.FunctionComponent<ChartBulletProps> = ({
standalone: false,
subTitle: groupSubTitle,
title: groupTitle,
theme,
themeColor,
width,
...groupTitleComponent.props
Expand Down Expand Up @@ -719,15 +718,10 @@ export const ChartBullet: React.FunctionComponent<ChartBulletProps> = ({
labelComponent: legendComponent.props.labelComponent ? (
React.cloneElement(legendComponent.props.labelComponent, {
direction: 'rtl',
dx: legendXOffset - 30,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
dx: legendXOffset - 30
})
) : (
<ChartLabel
direction="rtl"
dx={legendXOffset - 30}
{...(theme.skeleton && theme.skeleton)} // override backgroundStyle
/>
<ChartLabel direction="rtl" dx={legendXOffset - 30} />
)
}),
...legendComponent.props
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ export const ChartBulletComparativeErrorMeasure: React.FunctionComponent<ChartBu
padding,
standalone: false,
theme,
themeColor,
width,
y,
...measureComponent.props
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { ChartBulletStyles } from '../ChartTheme/ChartStyles';
import { getLabelTextSize, getBulletLabelX, getBulletLabelY } from '../ChartUtils/chart-label';
import { getPaddingForSide } from '../ChartUtils/chart-padding';
import { getBulletGroupTitleTheme } from '../ChartUtils/chart-theme-types';
import { getComponentTheme } from '../ChartUtils/chart-theme';

/**
* ChartBulletGroupTitle renders a group title.
Expand Down Expand Up @@ -165,6 +166,7 @@ export const ChartBulletGroupTitle: React.FunctionComponent<ChartBulletGroupTitl

// Returns title
const getTitle = () => {
const componentTheme = getComponentTheme(themeColor);
const titleProps = titleComponent ? titleComponent.props : {};
const showBoth = title && subTitle;
return React.cloneElement(titleComponent, {
Expand All @@ -184,7 +186,7 @@ export const ChartBulletGroupTitle: React.FunctionComponent<ChartBulletGroupTitl
labelPosition: 'top'
}),
...titleProps,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ export const ChartBulletPrimaryDotMeasure: React.FunctionComponent<ChartBulletPr
}
},
theme,
themeColor,
width,
...measureComponent.props
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ export const ChartBulletPrimarySegmentedMeasure: React.FunctionComponent<ChartBu
}
},
theme,
themeColor,
width,
...measureComponent.props
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export const ChartBulletQualitativeRange: React.FunctionComponent<ChartBulletQua
}
},
theme,
themeColor,
width,
...measureComponent.props
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { ChartThemeDefinition } from '../ChartTheme/ChartTheme';
import { getBulletLabelX, getBulletLabelY } from '../ChartUtils/chart-label';
import { getPaddingForSide } from '../ChartUtils/chart-padding';
import { getBulletTheme } from '../ChartUtils/chart-theme-types';
import { getComponentTheme } from '../ChartUtils/chart-theme';

/**
* ChartBulletTitle renders the bullet chart title.
Expand Down Expand Up @@ -159,6 +160,7 @@ export const ChartBulletTitle: React.FunctionComponent<ChartBulletTitleProps> =

// Returns title
const getTitle = () => {
const componentTheme = getComponentTheme(themeColor);
const showBoth = title && subTitle;

let labelPosition: 'bottom' | 'left' | 'top-left' = horizontal ? 'left' : 'bottom';
Expand Down Expand Up @@ -216,7 +218,7 @@ export const ChartBulletTitle: React.FunctionComponent<ChartBulletTitleProps> =
labelPosition
}),
...titleComponent.props,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from 'victory-cursor-container';
import { ChartLabel } from '../ChartLabel/ChartLabel';
import { ChartThemeDefinition } from '../ChartTheme/ChartTheme';
import { getTheme } from '../ChartUtils/chart-theme';
import { getComponentTheme, getTheme } from '../ChartUtils/chart-theme';
import { getClassName } from '../ChartUtils/chart-helpers';

/**
Expand Down Expand Up @@ -210,11 +210,12 @@ export const ChartCursorContainer: React.FunctionComponent<ChartCursorContainerP
cursorLabelComponent = <ChartLabel />, // Note that Victory provides its own label component here
...rest
}: ChartCursorContainerProps) => {
const componentTheme = getComponentTheme(themeColor);
const chartClassName = getClassName({ className });
const chartCursorLabelComponent = React.cloneElement(cursorLabelComponent, {
theme,
...cursorLabelComponent.props,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});

// Clone so users can override cursor container props
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { ChartCommonStyles } from '../ChartTheme/ChartStyles';
import { ChartThemeDefinition } from '../ChartTheme/ChartTheme';
import { getPaddingForSide } from '../ChartUtils/chart-padding';
import { getPieLabelX, getPieLabelY } from '../ChartUtils/chart-label';
import { getComponentTheme } from '../ChartUtils/chart-theme';

interface ChartDonutSubTitleInterface {
dy?: number;
Expand Down Expand Up @@ -596,6 +597,8 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({
width = theme.pie.width,
...rest
}: ChartDonutProps) => {
const componentTheme = getComponentTheme(themeColor);

const defaultPadding = {
bottom: getPaddingForSide('bottom', padding, theme.pie.padding),
left: getPaddingForSide('left', padding, theme.pie.padding),
Expand Down Expand Up @@ -660,7 +663,7 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({
width
}),
...subTitleProps,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});
};

Expand Down Expand Up @@ -694,7 +697,7 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({
width
}),
...titleProps,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});
};

Expand All @@ -712,6 +715,7 @@ export const ChartDonut: React.FunctionComponent<ChartDonutProps> = ({
radius={chartRadius > 0 ? chartRadius : 0}
standalone={false}
theme={theme}
themeColor={themeColor}
width={width}
{...rest}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ export const ChartDonutThreshold: React.FunctionComponent<ChartDonutThresholdPro
standalone: false,
subTitlePosition: childProps.subTitlePosition || subTitlePosition,
theme: dynamicTheme,
themeColor,
width,
...childProps
});
Expand All @@ -546,6 +547,7 @@ export const ChartDonutThreshold: React.FunctionComponent<ChartDonutThresholdPro
padding={defaultPadding}
standalone={false}
theme={theme}
themeColor={themeColor}
width={width}
{...rest}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,7 @@ export const ChartDonutUtilization: React.FunctionComponent<ChartDonutUtilizatio
padding={padding}
standalone={false}
theme={getThresholdTheme()}
themeColor={themeColor}
width={width}
{...rest}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@ export const ChartGroup: React.FunctionComponent<ChartGroupProps> = ({
<VictoryGroup colorScale={colorScale} containerComponent={container} theme={theme} {...rest}>
{renderChildrenWithPatterns({
children,
patternScale: defaultPatternScale
patternScale: defaultPatternScale,
themeColor
})}
{isPatternDefs && getPatternDefs({ patternId, colorScale: defaultColorScale })}
</VictoryGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ChartContainer } from '../ChartContainer/ChartContainer';
import { ChartLabel } from '../ChartLabel/ChartLabel';
import { ChartPoint } from '../ChartPoint/ChartPoint';
import { ChartThemeDefinition } from '../ChartTheme/ChartTheme';
import { getTheme } from '../ChartUtils/chart-theme';
import { getComponentTheme, getTheme } from '../ChartUtils/chart-theme';

/**
* ChartLegend renders a chart legend component.
Expand Down Expand Up @@ -317,6 +317,8 @@ export const ChartLegend: React.FunctionComponent<ChartLegendProps> = ({
theme = getTheme(themeColor),
...rest
}: ChartLegendProps) => {
const componentTheme = getComponentTheme(themeColor);

// Merge pattern IDs with `style.data.fill` property
const getDefaultStyle = () => {
if (!patternScale) {
Expand Down Expand Up @@ -351,15 +353,15 @@ export const ChartLegend: React.FunctionComponent<ChartLegendProps> = ({
React.cloneElement(labelComponent, {
...(name && { id: (props: any) => `${name}-${(labelComponent as any).type.displayName}-${props.index}` }),
...labelComponent.props,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});

const getTitleComponent = () =>
React.cloneElement(titleComponent, {
// Victory doesn't appear to call the id function here, but it's valid for label components
...(name && { id: () => `${name}-${(titleComponent as any).type.displayName}` }),
...titleComponent.props,
...(theme.skeleton && theme.skeleton) // override backgroundStyle
...(componentTheme?.label && componentTheme.label) // override backgroundStyle
});

// Note: containerComponent is required for theme
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ export const ChartLegendTooltip: React.FunctionComponent<ChartLegendTooltipProps
}),
text,
theme,
themeColor,
width,
...rest
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ export const ChartLegendTooltipContent: React.FunctionComponent<ChartLegendToolt
patternScale,
standalone: false,
theme,
themeColor,
x: getX() + legendOffsetX + Helpers.evaluateProp(dx, undefined),
y: getY() + legendOffsetY + Helpers.evaluateProp(dy, undefined),
...legendProps
Expand Down
1 change: 1 addition & 0 deletions packages/react-charts/src/components/ChartPie/ChartPie.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ export const ChartPie: React.FunctionComponent<ChartPieProps> = ({
key: 'pf-chart-pie-legend',
orientation: legendOrientation,
theme,
themeColor,
...(legendDirection === 'rtl' && {
dataComponent: legendComponent.props.dataComponent ? (
React.cloneElement(legendComponent.props.dataComponent, { transform: `translate(${legendXOffset})` })
Expand Down
41 changes: 38 additions & 3 deletions packages/react-charts/src/components/ChartTheme/ChartTheme.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
import { VictoryThemeDefinition } from 'victory-core';

// Note: Victory incorrectly typed ThemeBaseProps.padding as number instead of PaddingProps
export interface ChartThemeDefinitionInterface extends VictoryThemeDefinition {
skeleton?: {
/**
* Chart component theme definition
* @private
* @beta
*/
export interface ChartComponentThemeDefinitionInterface {
axis?: VictoryThemeDefinition;
bullet?: VictoryThemeDefinition;
bulletComparativeErrorMeasure?: VictoryThemeDefinition;
bulletComparativeMeasure?: VictoryThemeDefinition;
bulletComparativeWarningMeasure: VictoryThemeDefinition;
bulletGroupTitle?: VictoryThemeDefinition;
bulletPrimaryDotMeasure?: VictoryThemeDefinition;
bulletPrimaryNegativeMeasure?: VictoryThemeDefinition;
bulletPrimarySegmentedMeasure?: VictoryThemeDefinition;
bulletQualitativeRange?: VictoryThemeDefinition;
donut?: VictoryThemeDefinition;
donutThresholdDynamic?: VictoryThemeDefinition;
donutThresholdStatic?: VictoryThemeDefinition;
donutUtilization?: VictoryThemeDefinition;
label?: {
backgroundStyle?: {
fill?: string;
};
Expand All @@ -11,10 +29,27 @@ export interface ChartThemeDefinitionInterface extends VictoryThemeDefinition {
stroke?: string;
};
};
threshold?: VictoryThemeDefinition;
}

/**
* Chart theme definition
*
* Note: Victory incorrectly typed ThemeBaseProps.padding as number instead of PaddingProps
*
* @public
*/
export interface ChartThemeDefinitionInterface extends VictoryThemeDefinition {}

/**
* Chart theme definition
* @public
*/
export type ChartThemeDefinition = ChartThemeDefinitionInterface;

/**
* Chart component theme definition
* @private
* @beta
*/
export type ChartComponentThemeDefinition = ChartComponentThemeDefinitionInterface;
Loading

0 comments on commit 0efcae1

Please sign in to comment.