Skip to content

[WEB-4855] refactor: bar chart improvements #7731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JayashTripathy wants to merge 3 commits into from
Closed

[WEB-4855] refactor: bar chart improvements #7731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
67887ef
🚧 WIP: Introduced customTicks prop in BarChart for flexible tick rend…
JayashTripathy Sep 4, 2025
d063573
✨ feat: added customTicks to axis charts for flexible tick rendering
JayashTripathy Sep 7, 2025
aa6b71a
Merge branch 'preview' of https://github.com/makeplane/plane into ref…
JayashTripathy Sep 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions packages/propel/src/charts/area-chart/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const AreaChart = React.memo(<K extends string, T extends string>(props:
x: undefined,
y: 10,
},
customTicks,
showTooltip = true,
comparisonLine,
} = props;
Expand Down Expand Up @@ -114,7 +115,10 @@ export const AreaChart = React.memo(<K extends string, T extends string>(props:
<CartesianGrid stroke="rgba(var(--color-border-100), 0.8)" vertical={false} />
<XAxis
dataKey={xAxis.key}
tick={(props) => <CustomXAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} />;
}}
tickLine={false}
axisLine={false}
label={
Expand All @@ -140,7 +144,10 @@ export const AreaChart = React.memo(<K extends string, T extends string>(props:
className: AXIS_LABEL_CLASSNAME,
}
}
tick={(props) => <CustomYAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.y || CustomYAxisTick;
return <TickComponent {...props} />;
}}
tickCount={tickCount.y}
allowDecimals={!!yAxis.allowDecimals}
/>
Expand Down
7 changes: 5 additions & 2 deletions packages/propel/src/charts/bar-chart/bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const BAR_TOP_BORDER_RADIUS = 4; // Border radius for the top of bars
const BAR_BOTTOM_BORDER_RADIUS = 4; // Border radius for the bottom of bars
const DEFAULT_LOLLIPOP_LINE_WIDTH = 2; // Width of lollipop stick
const DEFAULT_LOLLIPOP_CIRCLE_RADIUS = 8; // Radius of lollipop circle
const DEFAULT_BAR_FILL_COLOR = "#6B7280"; // Default color when fill is a function - using a neutral gray from design system

// Types
interface TShapeProps {
Expand Down Expand Up @@ -108,7 +109,7 @@ const CustomBar = React.memo((props: TBarProps) => {
<path
d={getBarPath(x, y, width, height, topBorderRadius, bottomBorderRadius)}
className="transition-opacity duration-200"
fill={typeof fill === "function" ? fill(payload) : fill}
fill={typeof fill === "function" ? DEFAULT_BAR_FILL_COLOR : fill}
opacity={opacity}
/>
Comment on lines 110 to 114
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Bug: functional fills are ignored; bars won’t use per-datapoint color.

When fill is a function, this should call fill(payload), not a static default.

-        fill={typeof fill === "function" ? DEFAULT_BAR_FILL_COLOR : fill}
+        fill={typeof fill === "function" ? fill(payload) : fill}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
d={getBarPath(x, y, width, height, topBorderRadius, bottomBorderRadius)}
className="transition-opacity duration-200"
fill={typeof fill === "function" ? fill(payload) : fill}
fill={typeof fill === "function" ? DEFAULT_BAR_FILL_COLOR : fill}
opacity={opacity}
/>
d={getBarPath(x, y, width, height, topBorderRadius, bottomBorderRadius)}
className="transition-opacity duration-200"
fill={typeof fill === "function" ? fill(payload) : fill}
opacity={opacity}
/>
🤖 Prompt for AI Agents 
In packages/propel/src/charts/bar-chart/bar.tsx around lines 110 to 114, the
current fill prop uses a static default when fill is a function; update it so
when fill is a function it is invoked with the bar's payload (e.g.
fill(payload)) and its return value is used as the fill; keep the existing
behavior of using the provided non-function fill value unchanged and fallback to
DEFAULT_BAR_FILL_COLOR only if the function returns a falsy/undefined value.

{showText && (
Expand Down Expand Up @@ -139,7 +140,7 @@ const CustomBarLollipop = React.memo((props: TBarProps) => {
cx={x + width / 2}
cy={y}
r={DEFAULT_LOLLIPOP_CIRCLE_RADIUS}
fill={typeof fill === "function" ? fill(payload) : fill}
fill={typeof fill === "function" ? DEFAULT_BAR_FILL_COLOR : fill}
stroke="none"
/>
Comment on lines 140 to 145
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Circle color inconsistent with lollipop stroke.

Circle should match computed stroke color when fill is a function.

-        fill={typeof fill === "function" ? DEFAULT_BAR_FILL_COLOR : fill}
+        fill={typeof fill === "function" ? fill(payload) : fill}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cx={x + width / 2}
cy={y}
r={DEFAULT_LOLLIPOP_CIRCLE_RADIUS}
fill={typeof fill === "function" ? fill(payload) : fill}
fill={typeof fill === "function" ? DEFAULT_BAR_FILL_COLOR : fill}
stroke="none"
/>
cx={x + width / 2}
cy={y}
r={DEFAULT_LOLLIPOP_CIRCLE_RADIUS}
fill={typeof fill === "function" ? fill(payload) : fill}
stroke="none"
/>
🤖 Prompt for AI Agents 
In packages/propel/src/charts/bar-chart/bar.tsx around lines 140 to 145, the
circle's fill uses DEFAULT_BAR_FILL_COLOR when fill is a function which makes it
inconsistent with the lollipop stroke; instead compute the stroke color exactly
the same way the stroke prop is computed (e.g., call fill(...) or the stroke
color resolver used for the lollipop) and set the circle's fill to that computed
stroke color when fill is a function, otherwise keep using the provided fill
value; ensure the fallback behavior for non-function fills remains unchanged.

{showPercentage && (
Expand Down Expand Up @@ -176,6 +177,8 @@ const createShapeVariant =
);
};

export { DEFAULT_BAR_FILL_COLOR };

export const barShapeVariants: Record<
TBarChartShapeVariant,
(props: TShapeProps, bar: TBarItem<string>, stackKeys: string[]) => React.ReactNode
Expand Down
14 changes: 11 additions & 3 deletions packages/propel/src/charts/bar-chart/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { TBarChartProps } from "@plane/types";
import { getLegendProps } from "../components/legend";
import { CustomXAxisTick, CustomYAxisTick } from "../components/tick";
import { CustomTooltip } from "../components/tooltip";
import { barShapeVariants } from "./bar";
import { barShapeVariants, DEFAULT_BAR_FILL_COLOR } from "./bar";

export const BarChart = React.memo(<K extends string, T extends string>(props: TBarChartProps<K, T>) => {
const {
Expand All @@ -35,6 +35,7 @@ export const BarChart = React.memo(<K extends string, T extends string>(props: T
x: undefined,
y: 10,
},
customTicks,
showTooltip = true,
customTooltipContent,
} = props;
Expand Down Expand Up @@ -65,6 +66,7 @@ export const BarChart = React.memo(<K extends string, T extends string>(props: T
key={bar.key}
dataKey={bar.key}
stackId={bar.stackId}
fill={typeof bar.fill === "function" ? DEFAULT_BAR_FILL_COLOR : bar.fill}
opacity={!!activeLegend && activeLegend !== bar.key ? 0.1 : 1}
shape={(shapeProps: any) => {
const shapeVariant = barShapeVariants[bar.shapeVariant ?? "bar"];
Expand Down Expand Up @@ -96,7 +98,10 @@ export const BarChart = React.memo(<K extends string, T extends string>(props: T
<CartesianGrid stroke="rgba(var(--color-border-100), 0.8)" vertical={false} />
<XAxis
dataKey={xAxis.key}
tick={(props) => <CustomXAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} />;
}}
Comment on lines +101 to +104
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Regression: X-axis tick formatter (getLabel) dropped.

Same issue as LineChart; pass through xAxis.getLabel.

-            tick={(props) => {
-              const TickComponent = customTicks?.x || CustomXAxisTick;
-              return <TickComponent {...props} />;
-            }}
+            tick={(props) => {
+              const TickComponent = customTicks?.x || CustomXAxisTick;
+              return <TickComponent {...props} getLabel={xAxis.getLabel} />;
+            }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} />;
}}
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} getLabel={xAxis.getLabel} />;
}}
🤖 Prompt for AI Agents 
In packages/propel/src/charts/bar-chart/root.tsx around lines 101 to 104, the
X-axis TickComponent is rendered without forwarding the axis label formatter
(xAxis.getLabel), which regresses behavior; update the tick render to pass
through the xAxis.getLabel (e.g. include getLabel: xAxis.getLabel in the props
spread passed to TickComponent) so custom and default ticks receive the label
formatter while keeping all existing props intact.

tickLine={false}
axisLine={false}
label={{
Expand All @@ -118,7 +123,10 @@ export const BarChart = React.memo(<K extends string, T extends string>(props: T
dx: yAxis.dx ?? -16,
className: AXIS_LABEL_CLASSNAME,
}}
tick={(props) => <CustomYAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.y || CustomYAxisTick;
return <TickComponent {...props} />;
}}
tickCount={tickCount.y}
allowDecimals={!!yAxis.allowDecimals}
/>
Expand Down
11 changes: 9 additions & 2 deletions packages/propel/src/charts/line-chart/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const LineChart = React.memo(<K extends string, T extends string>(props:
x: undefined,
y: 10,
},
customTicks,
legend,
showTooltip = true,
} = props;
Expand Down Expand Up @@ -100,7 +101,10 @@ export const LineChart = React.memo(<K extends string, T extends string>(props:
<CartesianGrid stroke="rgba(var(--color-border-100), 0.8)" vertical={false} />
<XAxis
dataKey={xAxis.key}
tick={(props) => <CustomXAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} />;
}}
Comment on lines +104 to +107
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Regression: X-axis tick formatter (getLabel) no longer applied.

CustomXAxisTick expects getLabel; without passing it, labels won’t be formatted.

Apply:

-            tick={(props) => {
-              const TickComponent = customTicks?.x || CustomXAxisTick;
-              return <TickComponent {...props} />;
-            }}
+            tick={(props) => {
+              const TickComponent = customTicks?.x || CustomXAxisTick;
+              return <TickComponent {...props} getLabel={xAxis.getLabel} />;
+            }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} />;
}}
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} getLabel={xAxis.getLabel} />;
}}
🤖 Prompt for AI Agents 
In packages/propel/src/charts/line-chart/root.tsx around lines 104 to 107, the
tick renderer calls the TickComponent but does not pass the getLabel formatter
through, so CustomXAxisTick never receives getLabel; update the tick callback to
pass getLabel (preferably from customTicks.x if present, otherwise from the
current component props) along with the existing props so the TickComponent
receives and applies the label formatter.

tickLine={false}
axisLine={false}
label={
Expand All @@ -126,7 +130,10 @@ export const LineChart = React.memo(<K extends string, T extends string>(props:
className: AXIS_LABEL_CLASSNAME,
}
}
tick={(props) => <CustomYAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.y || CustomYAxisTick;
return <TickComponent {...props} />;
}}
tickCount={tickCount.y}
allowDecimals={!!yAxis.allowDecimals}
/>
Expand Down
11 changes: 9 additions & 2 deletions packages/propel/src/charts/scatter-chart/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const ScatterChart = React.memo(<K extends string, T extends string>(prop
xAxis,
yAxis,
className,
customTicks,
tickCount = {
x: undefined,
y: 10,
Expand Down Expand Up @@ -85,7 +86,10 @@ export const ScatterChart = React.memo(<K extends string, T extends string>(prop
<CartesianGrid stroke="rgba(var(--color-border-100), 0.8)" vertical={false} />
<XAxis
dataKey={xAxis.key}
tick={(props) => <CustomXAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.x || CustomXAxisTick;
return <TickComponent {...props} />;
}}
tickLine={false}
axisLine={false}
label={
Expand All @@ -111,7 +115,10 @@ export const ScatterChart = React.memo(<K extends string, T extends string>(prop
className: AXIS_LABEL_CLASSNAME,
}
}
tick={(props) => <CustomYAxisTick {...props} />}
tick={(props) => {
const TickComponent = customTicks?.y || CustomYAxisTick;
return <TickComponent {...props} />;
}}
tickCount={tickCount.y}
allowDecimals={!!yAxis.allowDecimals}
/>
Expand Down
32 changes: 20 additions & 12 deletions packages/types/src/charts/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,17 @@ export type TChartData<K extends string, T extends string> = {
[key in K]: string | number;
} & Record<T, any>;

export type TChartProps<K extends string, T extends string> = {
export type TBaseChartProps<K extends string, T extends string> = {
data: TChartData<K, T>[];
className?: string;
legend?: TChartLegend;
margin?: TChartMargin;
showTooltip?: boolean;
customTooltipContent?: (props: { active?: boolean; label: string; payload: any }) => React.ReactNode;
};

// Props specific to charts with X and Y axes
export type TAxisChartProps<K extends string, T extends string> = TBaseChartProps<K, T> & {
xAxis: {
key: keyof TChartData<K, T>;
label?: string;
Expand All @@ -38,15 +47,14 @@ export type TChartProps<K extends string, T extends string> = {
offset?: number;
dx?: number;
};
className?: string;
legend?: TChartLegend;
margin?: TChartMargin;
tickCount?: {
x?: number;
y?: number;
};
showTooltip?: boolean;
customTooltipContent?: (props: { active?: boolean; label: string; payload: any }) => React.ReactNode;
customTicks?: {
x?: React.ComponentType<any>;
y?: React.ComponentType<any>;
};
};

// ============================================================
Expand All @@ -67,7 +75,7 @@ export type TBarItem<T extends string> = {
shapeVariant?: TBarChartShapeVariant;
};

export type TBarChartProps<K extends string, T extends string> = TChartProps<K, T> & {
export type TBarChartProps<K extends string, T extends string> = TAxisChartProps<K, T> & {
bars: TBarItem<T>[];
barSize?: number;
};
Expand All @@ -87,7 +95,7 @@ export type TLineItem<T extends string> = {
style?: Record<string, string | number>;
};

export type TLineChartProps<K extends string, T extends string> = TChartProps<K, T> & {
export type TLineChartProps<K extends string, T extends string> = TAxisChartProps<K, T> & {
lines: TLineItem<T>[];
};

Expand All @@ -102,7 +110,7 @@ export type TScatterPointItem<T extends string> = {
stroke: string;
};

export type TScatterChartProps<K extends string, T extends string> = TChartProps<K, T> & {
export type TScatterChartProps<K extends string, T extends string> = TAxisChartProps<K, T> & {
scatterPoints: TScatterPointItem<T>[];
};

Expand All @@ -123,7 +131,7 @@ export type TAreaItem<T extends string> = {
style?: Record<string, string | number>;
};

export type TAreaChartProps<K extends string, T extends string> = TChartProps<K, T> & {
export type TAreaChartProps<K extends string, T extends string> = TAxisChartProps<K, T> & {
areas: TAreaItem<T>[];
comparisonLine?: {
dashedLine: boolean;
Expand All @@ -141,7 +149,7 @@ export type TCellItem<T extends string> = {
};

export type TPieChartProps<K extends string, T extends string> = Pick<
TChartProps<K, T>,
TBaseChartProps<K, T>,
"className" | "data" | "showTooltip" | "legend" | "margin"
> & {
dataKey: T;
Expand Down Expand Up @@ -223,7 +231,7 @@ export type TRadarItem<T extends string> = {
};

export type TRadarChartProps<K extends string, T extends string> = Pick<
TChartProps<K, T>,
TBaseChartProps<K, T>,
"className" | "showTooltip" | "margin" | "data" | "legend"
> & {
dataKey: T;
Expand Down
Loading