Skip to content

Commit

Permalink
[IMP] chart: harmonize chart titles & paddings
Browse files Browse the repository at this point in the history
This commit harmonizes the chart titles and paddings across all
chart types.

Some things to note:
- gauge chart matches title/padding sizes with our chartJS charts. The
scorecards do their own thing.
- there is now only 2 font colors in the charts: the normal font color
and the muted font color (+ their high contrast variants). Titles
are now using the muted font color.
- muted font color is `#666666`, which is ChartJs default color (
`Chart.defaults.color`)

closes #5194

Task: 4316044
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
  • Loading branch information
hokolomopo committed Dec 2, 2024
1 parent ac37929 commit 9ef03b3
Show file tree
Hide file tree
Showing 21 changed files with 283 additions and 331 deletions.
9 changes: 4 additions & 5 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ export const ALERT_INFO_BORDER = "#98DBE2";
export const ALERT_INFO_TEXT_COLOR = "#09414A";
export const BADGE_SELECTED_COLOR = "#E6F2F3";

export const DEFAULT_CHART_PADDING = 20;
export const DEFAULT_CHART_FONT_SIZE = 22;

export const SCORECARD_GAUGE_CHART_PADDING = 10;
export const SCORECARD_GAUGE_CHART_FONT_SIZE = 14;
export const CHART_PADDING = 20;
export const CHART_PADDING_BOTTOM = 10;
export const CHART_PADDING_TOP = 15;
export const CHART_TITLE_FONT_SIZE = 16;

// Color picker defaults as upper case HEX to match `toHex`helper
export const COLOR_PICKER_DEFAULTS: Color[] = [
Expand Down
4 changes: 2 additions & 2 deletions src/helpers/figures/charts/bar_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ import { CHART_COMMON_OPTIONS, truncateLabel } from "./chart_ui_common";
import {
getBarChartData,
getBarChartDatasets,
getBarChartLayout,
getBarChartLegend,
getBarChartScales,
getBarChartTooltip,
getChartLayout,
getChartShowValues,
getChartTitle,
} from "./runtime";
Expand Down Expand Up @@ -234,7 +234,7 @@ export function createBarChartRuntime(chart: BarChart, getters: Getters): BarCha
options: {
...CHART_COMMON_OPTIONS,
indexAxis: chart.horizontal ? "y" : "x",
layout: getBarChartLayout(definition),
layout: getChartLayout(definition),
scales: getBarChartScales(definition, chartData),
plugins: {
title: getChartTitle(definition),
Expand Down
7 changes: 7 additions & 0 deletions src/helpers/figures/charts/chart_common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,13 @@ export function chartFontColor(backgroundColor: Color | undefined): Color {
return relativeLuminance(backgroundColor) < 0.3 ? "#FFFFFF" : "#000000";
}

export function chartMutedFontColor(backgroundColor: Color | undefined): Color {
if (!backgroundColor) {
return "#666666";
}
return relativeLuminance(backgroundColor) < 0.3 ? "#C8C8C8" : "#666666";
}

export function checkDataset(definition: ChartWithDataSetDefinition): CommandResult {
if (definition.dataSets) {
const invalidRanges =
Expand Down
4 changes: 2 additions & 2 deletions src/helpers/figures/charts/combo_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ import {
import { CHART_COMMON_OPTIONS, truncateLabel } from "./chart_ui_common";
import {
getBarChartData,
getBarChartLayout,
getBarChartScales,
getBarChartTooltip,
getChartLayout,
getChartShowValues,
getChartTitle,
getComboChartDatasets,
Expand Down Expand Up @@ -236,7 +236,7 @@ export function createComboChartRuntime(chart: ComboChart, getters: Getters): Co
},
options: {
...CHART_COMMON_OPTIONS,
layout: getBarChartLayout(definition),
layout: getChartLayout(definition),
scales: getBarChartScales(definition, chartData),
plugins: {
title: getChartTitle(definition),
Expand Down
33 changes: 11 additions & 22 deletions src/helpers/figures/charts/gauge_chart_rendering.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import {
CHART_PADDING,
CHART_PADDING_TOP,
CHART_TITLE_FONT_SIZE,
DEFAULT_FONT,
SCORECARD_GAUGE_CHART_FONT_SIZE,
SCORECARD_GAUGE_CHART_PADDING,
} from "../../../constants";
import { Color, PixelPosition, Rect } from "../../../types";
import { GaugeChartRuntime } from "../../../types/chart";
import { relativeLuminance } from "../../color";
import { clip } from "../../misc";
import {
computeTextDimension,
computeTextWidth,
getDefaultContextFont,
getFontSizeMatchingWidth,
} from "../../text_helper";
import { chartMutedFontColor } from "./chart_common";

export const GAUGE_PADDING_SIDE = 30;
export const GAUGE_PADDING_TOP = 10;
Expand All @@ -21,15 +22,9 @@ export const GAUGE_LABELS_FONT_SIZE = 12;
export const GAUGE_DEFAULT_VALUE_FONT_SIZE = 80;

const GAUGE_BACKGROUND_COLOR = "#F3F2F1";
export const GAUGE_TEXT_COLOR = "#666666";
export const GAUGE_TEXT_COLOR_HIGH_CONTRAST = "#C8C8C8";
const GAUGE_INFLECTION_MARKER_COLOR = "#666666aa";
const GAUGE_INFLECTION_LABEL_BOTTOM_MARGIN = 6;

export const GAUGE_TITLE_SECTION_HEIGHT = 25;
export const GAUGE_TITLE_FONT_SIZE = SCORECARD_GAUGE_CHART_FONT_SIZE;
export const GAUGE_TITLE_PADDING_LEFT = SCORECARD_GAUGE_CHART_PADDING;
export const GAUGE_TITLE_PADDING_TOP = SCORECARD_GAUGE_CHART_PADDING;

interface RenderingParams {
width: number;
Expand Down Expand Up @@ -144,7 +139,7 @@ function drawInflectionValues(ctx: CanvasRenderingContext2D, config: RenderingPa
ctx.rotate(Math.PI / 2 - inflectionValue.rotation);

ctx.lineWidth = 2;
ctx.strokeStyle = GAUGE_INFLECTION_MARKER_COLOR;
ctx.strokeStyle = chartMutedFontColor(config.backgroundColor) + "aa";
ctx.beginPath();
ctx.moveTo(0, -(height - config.gauge.arcWidth));
ctx.lineTo(0, -height - 3);
Expand Down Expand Up @@ -217,7 +212,7 @@ export function getGaugeRenderingConfig(
y: gaugeRect.y + gaugeRect.height + GAUGE_LABELS_FONT_SIZE,
};

const textColor = getContrastedTextColor(runtime.background);
const textColor = chartMutedFontColor(runtime.background);

const inflectionValues = getInflectionValues(runtime, gaugeRect, textColor, ctx);

Expand All @@ -228,20 +223,20 @@ export function getGaugeRenderingConfig(
({ width: titleWidth, height: titleHeight } = computeTextDimension(
ctx,
runtime.title.text,
{ ...runtime.title, fontSize: GAUGE_TITLE_FONT_SIZE },
{ ...runtime.title, fontSize: CHART_TITLE_FONT_SIZE },
"px"
));
}
switch (runtime.title.align) {
case "right":
x = boundingRect.width - titleWidth - GAUGE_TITLE_PADDING_LEFT;
x = boundingRect.width - titleWidth - CHART_PADDING;
break;
case "center":
x = (boundingRect.width - titleWidth) / 2;
break;
case "left":
default:
x = GAUGE_TITLE_PADDING_LEFT;
x = CHART_PADDING;
break;
}

Expand All @@ -250,10 +245,10 @@ export function getGaugeRenderingConfig(
height: boundingRect.height,
title: {
label: runtime.title.text ?? "",
fontSize: GAUGE_TITLE_FONT_SIZE,
fontSize: CHART_TITLE_FONT_SIZE,
textPosition: {
x,
y: GAUGE_TITLE_PADDING_TOP + titleHeight / 2,
y: CHART_PADDING_TOP + titleHeight / 2,
},
color: runtime.title.color ?? textColor,
bold: runtime.title.bold,
Expand Down Expand Up @@ -384,12 +379,6 @@ function getGaugeColor(runtime: GaugeChartRuntime): Color {
return runtime.colors.at(-1)!;
}

function getContrastedTextColor(backgroundColor: Color) {
return relativeLuminance(backgroundColor) > 0.3
? GAUGE_TEXT_COLOR
: GAUGE_TEXT_COLOR_HIGH_CONTRAST;
}

function getSegmentsOfRectangle(rectangle: UnalignedRectangle): Segment[] {
return [
{ start: rectangle.topLeft, end: rectangle.topRight },
Expand Down
4 changes: 2 additions & 2 deletions src/helpers/figures/charts/line_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ import {
} from "./chart_common";
import { CHART_COMMON_OPTIONS, truncateLabel } from "./chart_ui_common";
import {
getChartLayout,
getChartShowValues,
getChartTitle,
getLineChartData,
getLineChartDatasets,
getLineChartLayout,
getLineChartLegend,
getLineChartScales,
getLineChartTooltip,
Expand Down Expand Up @@ -244,7 +244,7 @@ export function createLineChartRuntime(chart: LineChart, getters: Getters): Char
},
options: {
...CHART_COMMON_OPTIONS,
layout: getLineChartLayout(definition),
layout: getChartLayout(definition),
scales: getLineChartScales(definition, chartData),
plugins: {
title: getChartTitle(definition),
Expand Down
4 changes: 2 additions & 2 deletions src/helpers/figures/charts/pie_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ import {
} from "./chart_common";
import { CHART_COMMON_OPTIONS, truncateLabel } from "./chart_ui_common";
import {
getChartLayout,
getChartShowValues,
getChartTitle,
getPieChartData,
getPieChartDatasets,
getPieChartLayout,
getPieChartLegend,
getPieChartTooltip,
} from "./runtime";
Expand Down Expand Up @@ -207,7 +207,7 @@ export function createPieChartRuntime(chart: PieChart, getters: Getters): PieCha
},
options: {
...CHART_COMMON_OPTIONS,
layout: getPieChartLayout(definition),
layout: getChartLayout(definition),
plugins: {
title: getChartTitle(definition),
legend: getPieChartLegend(definition, chartData),
Expand Down
4 changes: 2 additions & 2 deletions src/helpers/figures/charts/pyramid_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ import {
import { CHART_COMMON_OPTIONS, truncateLabel } from "./chart_ui_common";
import {
getBarChartDatasets,
getBarChartLayout,
getBarChartLegend,
getChartLayout,
getChartShowValues,
getChartTitle,
getPyramidChartData,
Expand Down Expand Up @@ -210,7 +210,7 @@ export function createPyramidChartRuntime(
options: {
...CHART_COMMON_OPTIONS,
indexAxis: "y",
layout: getBarChartLayout(definition),
layout: getChartLayout(definition),
scales: getPyramidChartScales(definition, chartData),
plugins: {
title: getChartTitle(definition),
Expand Down
4 changes: 2 additions & 2 deletions src/helpers/figures/charts/radar_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
} from "./chart_common";
import { CHART_COMMON_OPTIONS, truncateLabel } from "./chart_ui_common";
import {
getBarChartLayout,
getChartLayout,
getChartShowValues,
getChartTitle,
getRadarChartData,
Expand Down Expand Up @@ -228,7 +228,7 @@ export function createRadarChartRuntime(chart: RadarChart, getters: Getters): Ra
},
options: {
...CHART_COMMON_OPTIONS,
layout: getBarChartLayout(definition),
layout: getChartLayout(definition),
scales: getRadarChartScales(definition, chartData),
plugins: {
title: getChartTitle(definition),
Expand Down
76 changes: 7 additions & 69 deletions src/helpers/figures/charts/runtime/chartjs_layout.ts
Original file line number Diff line number Diff line change
@@ -1,80 +1,18 @@
import { ChartOptions } from "chart.js";
import { DEFAULT_CHART_PADDING } from "../../../../constants";
import {
BarChartDefinition,
ChartWithDataSetDefinition,
GenericDefinition,
LineChartDefinition,
PieChartDefinition,
WaterfallChartDefinition,
} from "../../../../types/chart";
import { CHART_PADDING, CHART_PADDING_BOTTOM, CHART_PADDING_TOP } from "../../../../constants";
import { ChartWithDataSetDefinition, GenericDefinition } from "../../../../types/chart";

type ChartLayout = ChartOptions["layout"];

export function getCommonChartLayout(
export function getChartLayout(
definition: GenericDefinition<ChartWithDataSetDefinition>
): ChartLayout {
// TODO FIXME: this is unused ATM. All the charts should probably use this instead oh whatever padding they are using now
// also look into how DEFAULT_CHART_PADDING is used in scorecards, it look strange
return {
padding: {
left: DEFAULT_CHART_PADDING,
right: DEFAULT_CHART_PADDING,
top: definition.title?.text ? DEFAULT_CHART_PADDING / 2 : DEFAULT_CHART_PADDING + 5,
bottom: DEFAULT_CHART_PADDING,
left: CHART_PADDING,
right: CHART_PADDING,
top: CHART_PADDING_TOP,
bottom: CHART_PADDING_BOTTOM,
},
};
}

export function getBarChartLayout(definition: GenericDefinition<BarChartDefinition>): ChartLayout {
return {
padding: computeChartPadding({
displayTitle: !!definition.title?.text,
displayLegend: definition.legendPosition === "top",
}),
};
}

export function getLineChartLayout(
definition: GenericDefinition<LineChartDefinition>
): ChartLayout {
return {
padding: computeChartPadding({
displayTitle: !!definition.title?.text,
displayLegend: definition.legendPosition === "top",
}),
};
}

export function getPieChartLayout(definition: PieChartDefinition): ChartLayout {
return {
padding: { left: 20, right: 20, top: definition.title ? 10 : 25, bottom: 10 },
};
}

export function getWaterfallChartLayout(definition: WaterfallChartDefinition): ChartLayout {
return {
padding: { left: 20, right: 20, top: definition.title ? 10 : 25, bottom: 10 },
};
}

function computeChartPadding({
displayTitle,
displayLegend,
}: {
displayTitle: boolean;
displayLegend: boolean;
}): {
top: number;
bottom: number;
left: number;
right: number;
} {
let top = 25;
if (displayTitle) {
top = 0;
} else if (displayLegend) {
top = 10;
}
return { left: 20, right: 20, top, bottom: 10 };
}
13 changes: 9 additions & 4 deletions src/helpers/figures/charts/runtime/chartjs_title.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
import { TitleOptions } from "chart.js";
import { _DeepPartialObject } from "chart.js/dist/types/utils";
import { DEFAULT_CHART_FONT_SIZE } from "../../../../constants";
import { CHART_PADDING, CHART_TITLE_FONT_SIZE } from "../../../../constants";
import { _t } from "../../../../translation";
import { ChartWithDataSetDefinition } from "../../../../types/chart";
import { chartFontColor } from "../chart_common";
import { chartMutedFontColor } from "../chart_common";

export function getChartTitle(
definition: ChartWithDataSetDefinition
): _DeepPartialObject<TitleOptions> {
const chartTitle = definition.title;
const fontColor = chartFontColor(definition.background);
const fontColor = chartMutedFontColor(definition.background);
return {
display: !!chartTitle.text,
text: _t(chartTitle.text!),
color: chartTitle?.color ?? fontColor,
align:
chartTitle.align === "center" ? "center" : chartTitle.align === "right" ? "end" : "start",
font: {
size: DEFAULT_CHART_FONT_SIZE,
size: CHART_TITLE_FONT_SIZE,
weight: chartTitle.bold ? "bold" : "normal",
style: chartTitle.italic ? "italic" : "normal",
},
padding: {
// Disable title top/left/right padding to use the chart padding instead.
// The legend already has a top padding, so bottom padding is useless for the title there.
bottom: definition.legendPosition === "top" ? 0 : CHART_PADDING,
},
};
}
Loading

0 comments on commit 9ef03b3

Please sign in to comment.