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.

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

Feat/bars improvements #1977

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ You can also check the
# Unreleased

- Features
- Centered x-axis labels
- Added y-axis labels
- Implemented Content Security Policy (CSP)
- Added a new option to the vertical axis on the line chart, allowing users to
display a dot over the line where it intercepts the x-axis ticks

Expand Down
2 changes: 1 addition & 1 deletion app/charts/area/areas-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ const useAreasState = (
marginRight: right,
});
const margins = {
top: 50 + yAxisLabelMargin,
top: 70 + yAxisLabelMargin,
right,
bottom,
left,
Expand Down
6 changes: 3 additions & 3 deletions app/charts/bar/bars-grouped-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ const useBarsGroupedState = (
});
const right = 40;
const margins = {
top: 0,
top: 55,
right,
bottom: bottom + 30,
left,
Expand Down Expand Up @@ -390,12 +390,12 @@ const useBarsGroupedState = (

const yAnchorRaw = (yScale(y) as number) + bw * 0.5;
const [xMin, xMax] = extent(xValues, (d) => d ?? 0) as [number, number];
const xAnchor = isMobile ? chartHeight : xScale((xMin + xMax) * 0.5);
const xAnchor = isMobile ? chartHeight : xScale(xMin + xMax);
const placement = isMobile
? MOBILE_TOOLTIP_PLACEMENT
: getCenteredTooltipPlacement({
chartWidth,
xAnchor: yAnchorRaw,
xAnchor,
topAnchor: !fields.segment,
});

Expand Down
6 changes: 3 additions & 3 deletions app/charts/bar/bars-stacked-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ const useBarsStackedState = (
});
const right = 40;
const margins = {
top: 0,
top: 65,
right,
bottom: bottom + 30,
left,
Expand Down Expand Up @@ -455,10 +455,10 @@ const useBarsStackedState = (
formatNumber,
});

const yAnchorRaw = (yScale(y) as number) + bw * 0.5;
const yAnchorRaw = (yScale(y) as number) + bw;
const xAnchor = isMobile
? chartHeight
: xScale(sum(xValues.map((d) => d ?? 0)) * 0.5);
: xScale(sum(xValues.map((d) => d ?? 0)));
const placement = isMobile
? MOBILE_TOOLTIP_PLACEMENT
: getCenteredTooltipPlacement({
Expand Down
2 changes: 1 addition & 1 deletion app/charts/bar/bars-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ const useBarsState = (
});
const right = 40;
const margins = {
top: 0,
top: 55,
right,
bottom: bottom + 30,
left,
Expand Down
2 changes: 1 addition & 1 deletion app/charts/column/columns-grouped-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ const useColumnsGroupedState = (
marginRight: right,
});
const margins = {
top: 50 + yAxisLabelMargin,
top: 70 + yAxisLabelMargin,
right,
bottom,
left,
Expand Down
2 changes: 1 addition & 1 deletion app/charts/column/columns-stacked-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ const useColumnsStackedState = (
marginRight: right,
});
const margins = {
top: 50 + yAxisLabelMargin,
top: 70 + yAxisLabelMargin,
right,
bottom,
left,
Expand Down
2 changes: 1 addition & 1 deletion app/charts/column/columns-state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ const useColumnsState = (
marginRight: right,
});
const margins = {
top: 50 + yAxisLabelMargin,
top: 70 + yAxisLabelMargin,
right,
bottom,
left,
Expand Down
41 changes: 31 additions & 10 deletions app/charts/shared/axis-height-band.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,38 @@
renderContainer,
} from "@/charts/shared/rendering-utils";
import { useChartTheme } from "@/charts/shared/use-chart-theme";
import { OpenMetadataPanelWrapper } from "@/components/metadata-panel";
import { useTimeFormatUnit } from "@/formatters";
import { useTransitionStore } from "@/stores/transition";
import { getTextWidth } from "@/utils/get-text-width";

export const AxisHeightBand = () => {
const ref = useRef<SVGGElement>(null);
const state = useChartState() as BarsState;
const { xScale, getYLabel, yTimeUnit, yScale, bounds } = state;
const { xScale, getYLabel, yTimeUnit, yScale, bounds, yDimension } = state;
const enableTransition = useTransitionStore((state) => state.enable);
const transitionDuration = useTransitionStore((state) => state.duration);
const formatDate = useTimeFormatUnit();
const { chartHeight, margins } = bounds;
const { labelColor, gridColor, labelFontSize, fontFamily, domainColor } =
useChartTheme();
const {
labelColor,
axisLabelFontSize,
gridColor,
labelFontSize,
fontFamily,
domainColor,
} = useChartTheme();

const fontSize =
yScale.bandwidth() > labelFontSize ? labelFontSize : yScale.bandwidth();

const labelLength = getTextWidth(yDimension.label, {
fontSize: Math.max(labelFontSize, fontSize),
});

useEffect(() => {
if (ref.current) {
const hasNegativeValues = xScale.domain()[0] < 0;
const fontSize =
yScale.bandwidth() > labelFontSize ? labelFontSize : yScale.bandwidth();
const axis = axisLeft(yScale)
.tickSizeOuter(0)
.tickSizeInner(hasNegativeValues ? -chartHeight : 6);
Expand Down Expand Up @@ -60,7 +73,7 @@
.attr("fill", labelColor)
.attr("text-anchor", "unset");
}
}, [

Check warning on line 76 in app/charts/shared/axis-height-band.tsx

View workflow job for this annotation

GitHub Actions / base-checks (lint)

React Hook useEffect has a missing dependency: 'fontSize'. Either include it or remove the dependency array
chartHeight,
domainColor,
enableTransition,
Expand All @@ -78,7 +91,18 @@
yScale,
]);

return <g ref={ref} />;
return (
<>
<g ref={ref} />
<foreignObject x={0} y={25} width={labelLength} height={20}>
<OpenMetadataPanelWrapper>
<span style={{ fontSize: `${axisLabelFontSize}px` }}>
{yDimension.label}
</span>
</OpenMetadataPanelWrapper>
</foreignObject>
</>
);
};

export const AxisHeightBandDomain = () => {
Expand Down Expand Up @@ -107,10 +131,7 @@
g.selectAll(".tick line").remove();
g.selectAll(".tick text").remove();
g.select(".domain")
.attr(
"transform",
`translate(0, -${chartHeight - (yScale(minY) || 0)})`
)
.attr("transform", `translate(0, 0)`)
.attr("stroke", domainColor);
}
}, [
Expand Down
1 change: 1 addition & 0 deletions app/charts/shared/axis-height-linear.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const AxisHeightLinear = () => {
<foreignObject
width={Math.min(axisTitleWidth, state.bounds.chartWidth)}
height={height}
y={30}
style={{ display: "flex" }}
>
<OpenMetadataPanelWrapper component={state.yMeasure}>
Expand Down
4 changes: 2 additions & 2 deletions app/charts/shared/axis-width-linear.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const AxisWidthLinear = () => {
xScale,
]);

const { height } = useAxisLabelHeightOffset({
const { height, labelWidth } = useAxisLabelHeightOffset({
label: xAxisLabel,
width: chartWidth,
marginLeft: margins.left,
Expand All @@ -96,7 +96,7 @@ export const AxisWidthLinear = () => {
return (
<>
<foreignObject
x={margins.left}
x={margins.left + chartWidth / 2 - labelWidth / 2}
y={margins.top + chartHeight + 34}
width={chartWidth}
height={height}
Expand Down
1 change: 1 addition & 0 deletions app/charts/shared/chart-dimensions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -198,5 +198,6 @@ export const useAxisLabelHeightOffset = ({
return {
height: fontSize * LINE_HEIGHT * lines,
offset: fontSize * LINE_HEIGHT * (lines - 1),
labelWidth,
};
};
2 changes: 1 addition & 1 deletion app/charts/shared/containers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export const ChartSvg = ({ children }: { children: ReactNode }) => {
<foreignObject
{...DISABLE_SCREENSHOT_ATTR}
width={width - margins.right}
y={20}
y={0}
height="26"
style={{ display: "flex", textAlign: "right" }}
>
Expand Down
Loading