Skip to content

Commit

Permalink
frontend: add and use charts colors (#2926)
Browse files Browse the repository at this point in the history
This PR adds the color palette to be used in charts and updates current
usages with references to the theme. A new property was added to the
theme object: `chartColors`, to hold an static array of colors.

Added default colors to the `theme.colors` object for the `Pie` and
`LinearTimeline` components, `TimeseriesChart` does not use default
colors, but their stories now use colors from `theme`.

Changed components:

### Linear Timeline
before
<img width="408" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/f1ab1b14-228e-49ea-b5c5-e93063ab774f">

after
<img width="421" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/462082ce-c11e-409b-9455-b8ff15fbe496">

Custom styled

before
<img width="415" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/a9fefc14-b7b5-4bf9-8be9-322c771c6ca8">

after
<img width="419" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/f395d04f-d927-4765-a3f6-fa41d00b3f17">

### Pie Chart
Chart now uses colors from theme
Changed labels to get colors from palette

before
<img width="325" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/2f34af8b-19de-42be-8796-a5c1e2adeea0">

after
<img width="328" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/b2e8bf9c-f5de-4603-88e5-7406d8761ce7">

### TimeseriesChart
before
<img width="917" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/38bb3a87-97c3-4fe8-952d-8b9faf56c491">

after
<img width="917" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/4105467c-b8db-4e8c-bbfd-3c82e55eeb4c">

before
<img width="895" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/386a4623-6092-4af7-87d3-d070b138187c">

after
<img width="895" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/83545c4f-416f-4a94-a14f-1c863fd5d8c1">


### Testing Performed
manual, unit tests
  • Loading branch information
jecr committed Feb 26, 2024
1 parent 2d9c174 commit 019fbeb
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 56 deletions.
18 changes: 14 additions & 4 deletions frontend/packages/core/src/Charts/linearTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
YAxis,
} from "recharts";

import { useTheme } from "../AppProvider/themes";

import { calculateDomainEdges, calculateTicks, localTimeFormatter } from "./helpers";
import type { CustomTooltipProps, LinearTimelineData, LinearTimelineStylingProps } from "./types";

Expand Down Expand Up @@ -46,6 +48,8 @@ const LinearTimeline = ({
tooltipFormatterFunc = null,
stylingProps = {},
}: LinearTimelineProps) => {
const theme = useTheme();

const combinedData = Object.keys(data).reduce((acc, lane) => {
return [...acc, ...data[lane].points];
}, []);
Expand Down Expand Up @@ -86,8 +90,11 @@ const LinearTimeline = ({
return (
<div
style={{
backgroundColor: stylingProps?.tooltipBackgroundColor,
color: stylingProps?.tooltipTextColor,
backgroundColor:
stylingProps?.tooltipBackgroundColor ||
theme.colors.charts.linearTimeline.tooltipBackgroundColor,
color:
stylingProps?.tooltipTextColor || theme.colors.charts.linearTimeline.tooltipTextColor,
}}
>
{localTimeFormatter(payload[0].value)}
Expand All @@ -102,8 +109,11 @@ const LinearTimeline = ({
<ResponsiveContainer width="100%" height="100%">
<ScatterChart>
<CartesianGrid
fill={stylingProps?.gridBackgroundColor ?? "black"}
stroke={stylingProps?.gridStroke ?? "white"}
fill={
stylingProps?.gridBackgroundColor ??
theme.colors.charts.linearTimeline.gridBackgroundColor
}
stroke={stylingProps?.gridStroke ?? theme.colors.charts.linearTimeline.gridStroke}
/>
<XAxis
dataKey={xAxisDataKey}
Expand Down
45 changes: 23 additions & 22 deletions frontend/packages/core/src/Charts/pie.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import React, { PureComponent } from "react";
import { ThemeContext } from "@emotion/react";
import type { Theme } from "@mui/material";
import {
Cell,
Label,
Expand All @@ -10,6 +12,8 @@ import {
Tooltip,
} from "recharts";

import styled from "../styled";

import type { PieChartData } from "./types";

export interface PieChartProps {
Expand Down Expand Up @@ -90,22 +94,13 @@ interface PieChartState {
activeIndex?: number;
}

const DEFAULT_COLORS = [
"#651FFF",
"#FF4081",
"#0091EA",
"#00695C",
"#9E9D24",
"#880E4F",
"#01579B",
"#F4511E",
"#009688",
"#C2185B",
"#1A237E",
"#7C4DFF",
"#88451D",
"#AA00FF",
];
const ChartLabelPrimary = styled("text")(({ theme }: { theme: Theme }) => ({
fill: theme.colors.charts.pie.labelPrimary,
}));

const ChartLabelSecondary = styled("text")(({ theme }: { theme: Theme }) => ({
fill: theme.colors.charts.pie.labelSecondary,
}));

const renderActiveShape = (props, options) => {
const RADIAN = Math.PI / 180;
Expand Down Expand Up @@ -161,12 +156,12 @@ const renderActiveShape = (props, options) => {
/>
<path d={`M${sx},${sy}L${mx},${my}L${ex},${ey}`} stroke={fill} fill="none" />
<circle cx={ex} cy={ey} r={2} fill={fill} stroke="none" />
<text x={ex + (cos >= 0 ? 1 : -1) * 12} y={ey} textAnchor={textAnchor} fill="#333">
<ChartLabelPrimary x={ex + (cos >= 0 ? 1 : -1) * 12} y={ey} textAnchor={textAnchor}>
{payload.name}
</text>
<text x={ex + (cos >= 0 ? 1 : -1) * 12} y={ey} dy={18} textAnchor={textAnchor} fill="#999">
</ChartLabelPrimary>
<ChartLabelSecondary x={ex + (cos >= 0 ? 1 : -1) * 12} y={ey} dy={18} textAnchor={textAnchor}>
{`${value} (${(percent * 100).toFixed(2)}%)`}
</text>
</ChartLabelSecondary>
</g>
);
};
Expand Down Expand Up @@ -219,6 +214,8 @@ class PieChart extends PureComponent<PieChartProps, PieChartState> {
tooltip,
} = this.props;

const { colors } = this.context;

const chartOptions = {
activeTooltip: typeof activeTooltip === "boolean" ? activeTooltip : true,
activeTooltipOptions: typeof activeTooltip !== "boolean" ? { ...activeTooltip } : {},
Expand Down Expand Up @@ -270,7 +267,7 @@ class PieChart extends PureComponent<PieChartProps, PieChartState> {
>
<Pie
data={data}
fill={DEFAULT_COLORS[0]}
fill={colors.charts.common.data[0]}
dataKey="value"
onMouseEnter={this.onPieEnter}
{...chartOptions.dimensions}
Expand All @@ -280,7 +277,9 @@ class PieChart extends PureComponent<PieChartProps, PieChartState> {
<Cell
// eslint-disable-next-line react/no-array-index-key
key={`cell-${index}`}
fill={entry.color ?? DEFAULT_COLORS[index % DEFAULT_COLORS.length]}
fill={
entry.color ?? colors.charts.common.data[index % colors.charts.common.data.length]
}
/>
))}
{centerLabel && <Label content={<CenterLabel options={centerLabel} />} />}
Expand All @@ -301,4 +300,6 @@ class PieChart extends PureComponent<PieChartProps, PieChartState> {
}
}

PieChart.contextType = ThemeContext;

export { PieChart };
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react";
import type { Meta } from "@storybook/react";

import { useTheme } from "../../AppProvider/themes";
import styled from "../../styled";
import LinearTimeline from "../linearTimeline";
import type { LinearTimelineData } from "../types";
Expand All @@ -16,9 +17,18 @@ const ChartContainer = styled("div")({
});

export const Primary = () => {
const theme = useTheme();
const mockData: LinearTimelineData = {
deploys: { points: [{ timestamp: 1588884888 }], shape: "cross", color: "purple" },
"k8s events": { points: [{ timestamp: 1588985888 }], shape: "triangle", color: "blue" },
deploys: {
points: [{ timestamp: 1588884888 }],
shape: "cross",
color: theme.colors.charts.common.data[1],
},
"k8s events": {
points: [{ timestamp: 1588985888 }],
shape: "triangle",
color: theme.colors.charts.common.data[6],
},
explosions: {
points: [
{ timestamp: 1588788888 },
Expand All @@ -36,7 +46,7 @@ export const Primary = () => {
{ timestamp: 1589807088 },
],
shape: "star",
color: "green",
color: theme.colors.charts.common.data[2],
},
};
return (
Expand All @@ -47,9 +57,18 @@ export const Primary = () => {
};

export const ColoredChart = () => {
const theme = useTheme();
const mockData: LinearTimelineData = {
deploys: { points: [{ timestamp: 1588884888 }], shape: "cross", color: "purple" },
"k8s events": { points: [{ timestamp: 1588985888 }], shape: "triangle", color: "blue" },
deploys: {
points: [{ timestamp: 1588884888 }],
shape: "cross",
color: theme.colors.red[500],
},
"k8s events": {
points: [{ timestamp: 1588985888 }],
shape: "triangle",
color: theme.colors.amber[500],
},
explosions: {
points: [
{ timestamp: 1588788888 },
Expand All @@ -67,7 +86,7 @@ export const ColoredChart = () => {
{ timestamp: 1589807088 },
],
shape: "star",
color: "black",
color: theme.colors.green[500],
},
};
return (
Expand All @@ -76,11 +95,11 @@ export const ColoredChart = () => {
data={mockData}
xAxisDataKey="timestamp"
stylingProps={{
xAxisStroke: "red",
tooltipBackgroundColor: "blue",
tooltipTextColor: "white",
gridBackgroundColor: "green",
gridStroke: "yellow",
xAxisStroke: theme.colors.blue[500],
tooltipBackgroundColor: theme.colors.blue[200],
tooltipTextColor: theme.colors.blue[900],
gridBackgroundColor: theme.colors.blue[100],
gridStroke: theme.colors.blue[300],
}}
/>
</ChartContainer>
Expand Down
30 changes: 18 additions & 12 deletions frontend/packages/core/src/Charts/stories/timeseries.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from "react";
import { alpha } from "@mui/system";
import type { Meta } from "@storybook/react";

import { useTheme } from "../../AppProvider/themes";
import styled from "../../styled";
import { dateTimeFormatter, isoTimeFormatter } from "../helpers";
import TimeseriesChart from "../timeseries";
Expand All @@ -17,6 +19,7 @@ const ChartContainer = styled("div")({
});

export const SingleDataLine = () => {
const theme = useTheme();
const mockDataSingleLine = [
{
lineName: "line1",
Expand Down Expand Up @@ -53,7 +56,7 @@ export const SingleDataLine = () => {
const mockLines = [
{
dataKey: "value",
color: "red",
color: theme.colors.red[500],
animationDuration: 0,
},
];
Expand Down Expand Up @@ -94,6 +97,7 @@ export const SingleDataLine = () => {
};

export const MultiLine = () => {
const theme = useTheme();
const mockDataMultiLine = [
{
lineName: "line1",
Expand Down Expand Up @@ -130,12 +134,12 @@ export const MultiLine = () => {
const mockMultipleLines = [
{
dataKey: "value",
color: "red",
color: theme.colors.red[500],
animationDuration: 0,
},
{
dataKey: "value2",
color: "blue",
color: theme.colors.blue[500],
},
];

Expand All @@ -160,6 +164,7 @@ export const MultiLine = () => {
* other options.
*/
export const ReferenceLinesAndThingsDisabled = () => {
const theme = useTheme();
const mockData = [
{
lineName: "line1",
Expand All @@ -181,7 +186,7 @@ export const ReferenceLinesAndThingsDisabled = () => {
const mockLines = [
{
dataKey: "value",
color: "purple",
color: theme.colors.charts.common.data[0],
animationDuration: 2000,
},
];
Expand All @@ -191,13 +196,13 @@ export const ReferenceLinesAndThingsDisabled = () => {
axis: "x",
coordinate: 1546300850000,
label: "ref line 1",
color: "green",
color: theme.colors.green[500],
},
{
axis: "y",
coordinate: 17,
label: "ref line 2",
color: "red",
color: theme.colors.red[500],
},
];

Expand All @@ -212,14 +217,15 @@ export const ReferenceLinesAndThingsDisabled = () => {
drawDots={false}
legend={false}
grid={false}
tickFormatterFunc={null}
tickFormatterFunc={v => `${v}`}
xDomainSpread={null}
/>
</ChartContainer>
);
};

export const StyledChart = () => {
const theme = useTheme();
const mockDataSingleLine = [
{
lineName: "line1",
Expand Down Expand Up @@ -256,7 +262,7 @@ export const StyledChart = () => {
const mockLines = [
{
dataKey: "value",
color: "red",
color: theme.colors.red[500],
animationDuration: 0,
},
];
Expand All @@ -272,10 +278,10 @@ export const StyledChart = () => {
yDomainSpread={0.3}
regularIntervalTicks
stylingProps={{
gridBackgroundColor: "pink",
gridStroke: "blue",
xAxisStroke: "green",
yAxisStroke: "orange",
gridBackgroundColor: alpha(theme.colors.red[600], 0.4),
gridStroke: theme.colors.blue[700],
xAxisStroke: theme.colors.green[500],
yAxisStroke: theme.colors.amber[500],
}}
/>
</ChartContainer>
Expand Down
8 changes: 7 additions & 1 deletion frontend/packages/core/src/Charts/tests/pie.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { render } from "@testing-library/react";

import "@testing-library/jest-dom";

import { ThemeProvider } from "../../Theme";
import { PieChart } from "../pie";
import type { PieChartData } from "../types";

Expand All @@ -28,7 +29,12 @@ jest.mock("recharts", () => {
};
});

const setup = props => render(<PieChart data={mockData} {...props} />);
const setup = props =>
render(
<ThemeProvider>
<PieChart data={mockData} {...props} />
</ThemeProvider>
);

test("renders correctly", () => {
const { container } = setup({});
Expand Down
Loading

0 comments on commit 019fbeb

Please sign in to comment.