Skip to content
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

Jump to bottom

withBarValueLabel not displaying in correct positions in BarChart - Issue #6991 #7011

Closed
wants to merge 2 commits into from
Closed

withBarValueLabel not displaying in correct positions in BarChart - Issue #6991 #7011

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4c46b4
withBarValueLabel not displaying in correct positions in BarChart
fsd-niraj Oct 22, 2024
a6a4453
Merge branch 'master' into master
fsd-niraj Oct 27, 2024
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
27 changes: 20 additions & 7 deletions packages/@mantine/charts/src/BarChart/BarChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ export type BarChartCssVariables = {

export interface BarChartProps
extends BoxProps,
GridChartBaseProps,
StylesApiProps<BarChartFactory>,
ElementProps<'div'> {
GridChartBaseProps,
StylesApiProps<BarChartFactory>,
ElementProps<'div'> {
/** Data used to display chart. */
data: Record<string, any>[];

Expand All @@ -81,8 +81,8 @@ export interface BarChartProps

/** Props passed down to recharts `Bar` component */
barProps?:
| ((series: BarChartSeries) => Partial<Omit<BarProps, 'ref'>>)
| Partial<Omit<BarProps, 'ref'>>;
| ((series: BarChartSeries) => Partial<Omit<BarProps, 'ref'>>)
| Partial<Omit<BarProps, 'ref'>>;

/** Determines whether a label with bar value should be displayed on top of each bar, incompatible with `type="stacked"` and `type="percent"`, `false` by default */
withBarValueLabel?: boolean;
Expand Down Expand Up @@ -130,10 +130,23 @@ export function BarLabel({
parentViewBox,
...others
}: Record<string, any>) {

function labelCoordinates(): { dx: number, dy: number } {
let coord: { dx: number, dy: number } = { dx: 0, dy: 0 }
if (others.props.h < 300) {
coord = { ...coord, dx: Math.ceil(others?.width + 10) }
} else {
coord.dx = Math.ceil(others?.width - others?.viewBox?.x / 2)
}
coord = { ...coord, dy: Math.ceil(others.props.h / 15) }
return coord;
}
Comment on lines +134 to +143
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO: I think this way is much simple, WDYT? I prefer not using let in this case.

Suggested change
function labelCoordinates(): { dx: number, dy: number } {
let coord: { dx: number, dy: number } = { dx: 0, dy: 0 }
if (others.props.h < 300) {
coord = { ...coord, dx: Math.ceil(others?.width + 10) }
} else {
coord.dx = Math.ceil(others?.width - others?.viewBox?.x / 2)
}
coord = { ...coord, dy: Math.ceil(others.props.h / 15) }
return coord;
}
function labelCoordinates(): { dx: number, dy: number } {
return {
dx: others.props.h < 300
? Math.ceil(others?.width + 10)
: Math.ceil(others?.width - others?.viewBox?.x / 2),
dy: Math.ceil(others.props.h / 15)
};
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That looks good too.
I tried not to use ternary operators just in case we need to add more conditions in future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since the code is not complicated yet, I think we should consider it only when additional conditions arise!


return (
<text
{...others}
dy={-10}
dx={labelCoordinates().dx}
dy={labelCoordinates().dy}
fontSize={12}
fill="var(--chart-text-color, var(--mantine-color-dimmed))"
textAnchor="center"
Expand Down Expand Up @@ -260,7 +273,7 @@ export const BarChart = factory<BarChartFactory>((_props, ref) => {
fillOpacity={dimmed ? 0.1 : fillOpacity}
strokeOpacity={dimmed ? 0.2 : 0}
stackId={stacked ? 'stack' : item.stackId || undefined}
label={withBarValueLabel ? <BarLabel valueFormatter={valueFormatter} /> : undefined}
label={withBarValueLabel ? <BarLabel valueFormatter={valueFormatter} props={_props} /> : undefined}
yAxisId={item.yAxisId || 'left'}
minPointSize={minBarSize}
{...(typeof barProps === 'function' ? barProps(item) : barProps)}
Expand Down
Loading