Skip to content

Commit f0a3f31

Browse files
luccas-harbourluccascorrea
authored andcommitted
fix: improve chart rendering fidelity for borders, gridlines, and legends
Render the chart-area outline from c:spPr (honoring a:noFill to disable it), gate value-axis gridlines behind the OOXML majorGridlines flag instead of always drawing them, and lay out right-positioned legends as a column beside bar charts that reserve right-edge space. Thread per-chart padding through the bar layout so the right legend gets dedicated room, and align gridline color with the axis color.
1 parent dae4cba commit f0a3f31

5 files changed

Lines changed: 195 additions & 50 deletions

File tree

packages/layout-engine/contracts/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,6 +1393,8 @@ export type ChartModel = {
13931393
legendPosition?: string;
13941394
/** OOXML chart style ID. */
13951395
styleId?: number;
1396+
/** Whether the chart area outline should be painted. */
1397+
chartAreaBorder?: boolean;
13961398
};
13971399

13981400
/** Chart drawing block. */

packages/layout-engine/painters/dom/src/chart-renderer.test.ts

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,20 @@ describe('createChartElement', () => {
113113
expect(el.querySelector('svg')).not.toBeNull();
114114
});
115115

116+
it('draws a chart area border around rendered charts', () => {
117+
const el = createChartElement(doc, makeBarChart(), defaultGeometry);
118+
expect(el.style.borderWidth).toBe('1px');
119+
expect(el.style.borderStyle).toBe('solid');
120+
expect(el.style.borderColor).toBe('rgb(191, 191, 191)');
121+
expect(el.style.boxSizing).toBe('border-box');
122+
});
123+
124+
it('omits the chart area border when the chart XML disables the outline', () => {
125+
const el = createChartElement(doc, makeBarChart({ chartAreaBorder: false }), defaultGeometry);
126+
expect(el.style.border).toBe('');
127+
expect(el.style.boxSizing).toBe('border-box');
128+
});
129+
116130
it('shows placeholder for missing chart data', () => {
117131
const el = createChartElement(doc, undefined, defaultGeometry);
118132
expect(el.textContent).toContain('No chart data');
@@ -131,13 +145,77 @@ describe('createChartElement', () => {
131145
it('renders bars for each series value', () => {
132146
const el = createChartElement(doc, makeBarChart(), defaultGeometry);
133147
const rects = el.querySelectorAll('svg rect');
134-
// 3 data points = 3 bar rects (no legend swatch for single series with legendPosition)
135-
// Wait — with the fix, legend shows for single series too. Let's check:
136-
// Series 1 has 3 values → 3 bars
137-
// Legend: 1 series with legendPosition → 1 swatch rect
138148
expect(rects.length).toBeGreaterThanOrEqual(3);
139149
});
140150

151+
it('paints vertical bar gridlines behind bars', () => {
152+
const el = createChartElement(doc, makeBarChart({ valueAxis: { majorGridlines: true } }), defaultGeometry);
153+
const svg = el.querySelector('svg')!;
154+
const children = Array.from(svg.children);
155+
const firstGridlineIndex = children.findIndex(
156+
(child) => child.tagName === 'line' && child.getAttribute('stroke-width') === '0.5',
157+
);
158+
const firstBarIndex = children.findIndex(
159+
(child) => child.tagName === 'rect' && child.getAttribute('fill') === '#4472C4',
160+
);
161+
const axisLine = Array.from(svg.querySelectorAll('line')).find(
162+
(line) => line.getAttribute('stroke-width') === '1',
163+
)!;
164+
const gridline = children[firstGridlineIndex]!;
165+
166+
expect(firstGridlineIndex).toBeGreaterThanOrEqual(0);
167+
expect(firstGridlineIndex).toBeLessThan(firstBarIndex);
168+
expect(gridline.getAttribute('stroke')).toBe(axisLine.getAttribute('stroke'));
169+
});
170+
171+
it('omits vertical bar gridlines when the value axis does not request them', () => {
172+
const el = createChartElement(doc, makeBarChart({ valueAxis: undefined }), defaultGeometry);
173+
const gridlines = Array.from(el.querySelectorAll('svg line')).filter(
174+
(line) => line.getAttribute('stroke-width') === '0.5',
175+
);
176+
177+
expect(gridlines).toHaveLength(0);
178+
});
179+
180+
it('renders right-positioned legends as a column beside vertical bar charts', () => {
181+
const el = createChartElement(
182+
doc,
183+
makeBarChart({
184+
legendPosition: 'r',
185+
series: [
186+
{ name: 'Series 1', categories: ['Q1'], values: [100] },
187+
{ name: 'Series 2', categories: ['Q1'], values: [80] },
188+
{ name: 'Series 3', categories: ['Q1'], values: [60] },
189+
],
190+
}),
191+
defaultGeometry,
192+
);
193+
const svg = el.querySelector('svg')!;
194+
const legendLabels = ['Series 1', 'Series 2', 'Series 3'].map(
195+
(name) => Array.from(svg.querySelectorAll('text')).find((text) => text.textContent === name)!,
196+
);
197+
const xPositions = legendLabels.map((label) => Number(label.getAttribute('x')));
198+
const yPositions = legendLabels.map((label) => Number(label.getAttribute('y')));
199+
const barRightEdge = Math.max(
200+
...Array.from(svg.querySelectorAll('rect'))
201+
.filter((rect) => rect.getAttribute('height') !== '10')
202+
.map((rect) => Number(rect.getAttribute('x')) + Number(rect.getAttribute('width'))),
203+
);
204+
205+
expect(new Set(xPositions).size).toBe(1);
206+
expect(xPositions[0]).toBeGreaterThan(barRightEdge);
207+
expect(yPositions[1] - yPositions[0]).toBeGreaterThan(0);
208+
expect(yPositions[2] - yPositions[1]).toBeGreaterThan(0);
209+
});
210+
211+
it('keeps right-positioned non-bar legends on the bottom until their plots reserve right space', () => {
212+
const el = createChartElement(doc, makeBubbleChart({ legendPosition: 'r' }), defaultGeometry);
213+
const legendLabel = Array.from(el.querySelectorAll('svg text')).find((text) => text.textContent === 'Series 1')!;
214+
215+
expect(Number(legendLabel.getAttribute('x'))).toBe(74);
216+
expect(Number(legendLabel.getAttribute('y'))).toBe(defaultGeometry.height - 12);
217+
});
218+
141219
it('renders horizontal bar charts with category labels and in-bar percentage data labels', () => {
142220
const el = createChartElement(
143221
doc,

0 commit comments

Comments
 (0)