diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c133ce85..fbcb0e080 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,16 @@ follow semantic versioning; release dates are ISO 8601. ### Fixed +- **A bar chart's value labels get the halo the style asks for.** The halo chip exists + so a grid line stops running through the digits. `BarChartLayout` resolved the colour + from the style and then passed `null` for every value label except a stacked total — + a hardcoded argument forty lines from a sibling call that passed the real one. The + result read as a feature that does nothing: the style accepted a halo, the layout + computed it, and the label was drawn without it. Line charts and donuts had it all + along, which is why the gap survived since 1.8.0 — where the release notes already + promised the chip for a label sitting at a "marker **or bar top**". Two arguments, and + the six committed previews that carry a bar chart with outside labels are re-rendered + with their grid lines cleanly interrupted. - **The example catalogue renders the weights it declares.** The same defect the donut KPI had, 138 times over in the examples: a font *face* constant named with no decoration, which the library rewrites to its base family before the lookup. The PPTX diff --git a/assets/readme/examples/chart-showcase.pdf b/assets/readme/examples/chart-showcase.pdf index 0fde69f31..1317546bd 100644 Binary files a/assets/readme/examples/chart-showcase.pdf and b/assets/readme/examples/chart-showcase.pdf differ diff --git a/assets/readme/examples/engine-deck-v2.pdf b/assets/readme/examples/engine-deck-v2.pdf index 323275d0b..05e6b8f35 100644 Binary files a/assets/readme/examples/engine-deck-v2.pdf and b/assets/readme/examples/engine-deck-v2.pdf differ diff --git a/assets/readme/examples/engine-deck.pdf b/assets/readme/examples/engine-deck.pdf index adf4488f3..4c7097424 100644 Binary files a/assets/readme/examples/engine-deck.pdf and b/assets/readme/examples/engine-deck.pdf differ diff --git a/assets/readme/examples/feature-catalog.pdf b/assets/readme/examples/feature-catalog.pdf index 860559b4d..fd27cbd49 100644 Binary files a/assets/readme/examples/feature-catalog.pdf and b/assets/readme/examples/feature-catalog.pdf differ diff --git a/assets/readme/examples/financial-report.pdf b/assets/readme/examples/financial-report.pdf index 8a0f47071..47194346e 100644 Binary files a/assets/readme/examples/financial-report.pdf and b/assets/readme/examples/financial-report.pdf differ diff --git a/assets/readme/examples/financial-report.pptx b/assets/readme/examples/financial-report.pptx index 7744cf16d..7481dbb38 100644 Binary files a/assets/readme/examples/financial-report.pptx and b/assets/readme/examples/financial-report.pptx differ diff --git a/core/src/main/java/com/demcha/compose/document/chart/BarChartLayout.java b/core/src/main/java/com/demcha/compose/document/chart/BarChartLayout.java index 0689f268e..a06f91f44 100644 --- a/core/src/main/java/com/demcha/compose/document/chart/BarChartLayout.java +++ b/core/src/main/java/com/demcha/compose/document/chart/BarChartLayout.java @@ -141,7 +141,7 @@ private static List resolveVertical(ChartSpec.Bar bar, ChartStyl double labelBottomY = v >= baseValue ? yBottom + h + labelGap : yBottom - labelGap - f.valueLineH(); - emitChipLabel(out, "value_c" + c + "_s" + s, text, valueStyle, null, + emitChipLabel(out, "value_c" + c + "_s" + s, text, valueStyle, halo, bx + innerBarW / 2.0, labelBottomY, labelW, f.valueLineH()); } @@ -338,7 +338,7 @@ private static List resolveHorizontal(ChartSpec.Bar bar, ChartSt double centerX = v >= baseValue ? xLeft + w + labelGap + labelW / 2.0 : xLeft - labelGap - labelW / 2.0; - emitChipLabel(out, "value_c" + c + "_s" + s, text, valueStyle, null, + emitChipLabel(out, "value_c" + c + "_s" + s, text, valueStyle, halo, centerX, barTop - innerBarH / 2.0 - valueInk, labelW, valueLineH); } } diff --git a/core/src/main/java/com/demcha/compose/document/chart/ChartDefaults.java b/core/src/main/java/com/demcha/compose/document/chart/ChartDefaults.java index c0ce56f81..fad5141ca 100644 --- a/core/src/main/java/com/demcha/compose/document/chart/ChartDefaults.java +++ b/core/src/main/java/com/demcha/compose/document/chart/ChartDefaults.java @@ -81,8 +81,8 @@ public final class ChartDefaults { .build(); /** - * Default value-label halo — a white chip behind line-chart value labels so - * digits stay legible where line strokes cross them. Charts rendered on a + * Default value-label halo — a white chip behind chart value labels so the + * digits stay legible where a grid line or a stroke crosses them. Charts on a * non-white surface should override it with the surface colour via * {@code ChartStyle.valueLabelHalo(...)}. */ diff --git a/core/src/main/java/com/demcha/compose/document/chart/ChartStyle.java b/core/src/main/java/com/demcha/compose/document/chart/ChartStyle.java index 485892de9..eadd5d660 100644 --- a/core/src/main/java/com/demcha/compose/document/chart/ChartStyle.java +++ b/core/src/main/java/com/demcha/compose/document/chart/ChartStyle.java @@ -45,9 +45,10 @@ * @param axisTextStyle tick / category label style * @param legendTextStyle legend label style * @param valueLabelTextStyle value-label style - * @param valueLabelHalo halo chip painted behind line-chart value labels so the - * digits stay legible where line strokes cross them; match - * it to the chart's surface colour on non-white backgrounds + * @param valueLabelHalo halo chip painted behind chart value labels so the digits + * stay legible where a grid line or a stroke crosses them; + * match it to the chart's surface colour on non-white + * backgrounds * @param areaOpacity opacity of the area fill under {@code ChartSpec.line().area(true)} * series, in (0..1], or {@code null} for the default (0.35) * @param sliceStroke pie/donut slice separator stroke, or {@code null} for the @@ -357,8 +358,9 @@ public Builder valueLabelTextStyle(DocumentTextStyle s) { } /** - * Sets the halo chip painted behind line-chart value labels. Match it to - * the chart's surface colour on non-white backgrounds. + * Sets the halo chip painted behind chart value labels — bar, line and + * pie alike. Match it to the chart's surface colour on non-white + * backgrounds. * * @param halo halo paint * @return this builder diff --git a/core/src/main/java/com/demcha/compose/document/chart/ChartTheme.java b/core/src/main/java/com/demcha/compose/document/chart/ChartTheme.java index ff83f8523..b366a6249 100644 --- a/core/src/main/java/com/demcha/compose/document/chart/ChartTheme.java +++ b/core/src/main/java/com/demcha/compose/document/chart/ChartTheme.java @@ -20,7 +20,7 @@ * @param axisTextStyle default tick / category label style * @param legendTextStyle default legend label style * @param valueLabelTextStyle default value-label style - * @param valueLabelHalo default halo chip painted behind line-chart value labels + * @param valueLabelHalo default halo chip painted behind chart value labels * so digits stay legible where line strokes cross them * @author Artem Demchyshyn * @since 1.8.0 diff --git a/core/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java b/core/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java index 15e569ce1..f8b29e213 100644 --- a/core/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java +++ b/core/src/test/java/com/demcha/compose/document/chart/ChartLayoutResolverTest.java @@ -3,6 +3,7 @@ import com.demcha.compose.document.node.LineNode; import com.demcha.compose.document.node.ParagraphNode; import com.demcha.compose.document.node.ShapeNode; +import com.demcha.compose.document.style.DocumentColor; import com.demcha.compose.document.style.DocumentPaint; import com.demcha.compose.document.style.DocumentTextStyle; import org.junit.jupiter.api.Test; @@ -134,15 +135,80 @@ void minimalChartHidesGridAxesButKeepsBarsAndValueLabels() { assertThat(count(out, LineNode.class)).isZero(); assertThat(out.stream().anyMatch(p -> p.node().name().startsWith("tick_"))).isFalse(); assertThat(out.stream().anyMatch(p -> p.node().name().startsWith("cat_"))).isFalse(); - // Only the bars and their value labels remain. - assertThat(count(out, ShapeNode.class)).isEqualTo(2); - assertThat(out.stream().filter(p -> p.node().name().startsWith("value_")).count()) + // Only the bars and their value labels remain — each label carrying the + // halo chip the theme defines, which is a ShapeNode of its own. + assertThat(count(out, ShapeNode.class)).isEqualTo(4); + assertThat(out.stream().filter(p -> p.node().name().startsWith("value_") + && !p.node().name().endsWith("_halo")).count()) .isEqualTo(2); // With no tick labels the value axis reserves no left gutter: bars start at x≈0. ChartPrimitive barA = byName(out, "bar_c0_s0"); assertThat(barA.x()).isLessThan(20.0); } + /** + * A bar's value label sits on the grid line as readily as a line chart's does, + * and the halo exists to punch the line out from behind the digits. The bar + * layout resolved the colour and then passed {@code null} for every label + * except a stacked total, so the chip the style asked for was dropped on the + * floor for the commonest chart in the library. + */ + @Test + void barValueLabelsCarryTheHaloTheStyleAsksFor() { + DocumentColor haloColour = DocumentColor.rgb(20, 27, 51); + ChartData data = ChartData.builder() + .categories("A", "B") + .series("S", 10.0, 20.0) + .build(); + ChartSpec.Bar bar = ChartSpec.bar() + .data(data) + .valueLabels(ValueLabelMode.OUTSIDE) + .build(); + ChartStyle style = baseStyle().mergedUnder(ChartStyle.builder() + .valueLabelHalo(DocumentPaint.solid(haloColour)) + .build()); + + List out = ChartLayoutResolver.resolve( + bar, style, ChartDefaults.DEFAULT_THEME, 200.0, 100.0, METRICS); + + // One chip per label, each filled with the colour the style named — not the + // theme's, which is what a dropped argument would silently fall back to. + List halos = out.stream() + .filter(p -> p.node().name().endsWith("_halo")) + .toList(); + assertThat(halos).hasSize(2); + assertThat(halos).allSatisfy(halo -> + assertThat(((ShapeNode) halo.node()).fillColor()).isEqualTo(haloColour)); + + // The chip is painted before its label, or it would cover the digits. + int chip = indexOfName(out, "value_c0_s0_halo"); + int label = indexOfName(out, "value_c0_s0"); + assertThat(chip).isLessThan(label); + + // And it is wider than the text box it backs, so the digits sit inside it. + assertThat(out.get(chip).width()).isGreaterThan(out.get(label).width()); + } + + /** + * The horizontal branch dropped the same argument, so it is pinned separately — + * a fix applied to one orientation would otherwise look complete. + */ + @Test + void horizontalBarValueLabelsCarryTheHaloToo() { + ChartData data = ChartData.builder().categories("A").series("S", 10.0).build(); + ChartSpec.Bar bar = ChartSpec.bar() + .data(data) + .horizontal(true) + .valueLabels(ValueLabelMode.OUTSIDE) + .build(); + + List out = ChartLayoutResolver.resolve( + bar, baseStyle(), ChartDefaults.DEFAULT_THEME, 200.0, 100.0, METRICS); + + assertThat(out.stream().filter(p -> p.node().name().endsWith("_halo")).count()) + .isEqualTo(1); + } + @Test void valueLabelsNoneEmitsNoValueText() { ChartData data = ChartData.builder().categories("A").series("S", 5.0).build(); @@ -942,4 +1008,14 @@ private static long count(List out, Class type) { private static ChartPrimitive byName(List out, String name) { return out.stream().filter(p -> p.node().name().equals(name)).findFirst().orElseThrow(); } + + /** Emission order matters for anything painted behind something else. */ + private static int indexOfName(List out, String name) { + for (int i = 0; i < out.size(); i++) { + if (out.get(i).node().name().equals(name)) { + return i; + } + } + throw new AssertionError("no primitive named " + name); + } } diff --git a/examples/src/main/java/com/demcha/examples/flagships/LinkedInCarouselExample.java b/examples/src/main/java/com/demcha/examples/flagships/LinkedInCarouselExample.java index dc884be78..09815f90b 100644 --- a/examples/src/main/java/com/demcha/examples/flagships/LinkedInCarouselExample.java +++ b/examples/src/main/java/com/demcha/examples/flagships/LinkedInCarouselExample.java @@ -503,6 +503,10 @@ private static ChartStyle latencyStyle() { .barWidthRatio(0.5) .axisTextStyle(mono(22, ON_DARK_MUTED)) .valueLabelTextStyle(mono(25, ON_DARK)) + // The labels sit over the gridlines, which on this surface read as + // strikethroughs across the digits. The halo punches a chip of the + // card's own fill out from under each one. + .valueLabelHalo(DocumentPaint.solid(SURFACE)) .build(); }