Skip to content

Commit 9a3061f

Browse files
authored
fix: improve chart precision and scaling (#55)
* fix: improve chart precision and scaling * fix: mock Worker for tests
1 parent 00253c4 commit 9a3061f

File tree

8 files changed

+813
-558
lines changed

8 files changed

+813
-558
lines changed

src/App.jsx

Lines changed: 271 additions & 179 deletions
Large diffs are not rendered by default.

src/components/ChartContainer.jsx

Lines changed: 137 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -46,26 +46,26 @@ const ChartWrapper = ({ data, options, chartId, onRegisterChart, onSyncHover, sy
4646
// Find the data point closest to the cursor
4747
let closestElement = activeElements[0];
4848
let minDistance = Infinity;
49-
49+
5050
// Ensure canvas exists (may not in tests)
5151
if (chartRef.current.canvas && chartRef.current.canvas.getBoundingClientRect) {
5252
const canvasRect = chartRef.current.canvas.getBoundingClientRect();
5353
const mouseX = event.native ? event.native.clientX - canvasRect.left : event.x;
54-
54+
5555
activeElements.forEach(element => {
5656
const { datasetIndex, index } = element;
5757
const dataset = chartRef.current.data.datasets[datasetIndex];
5858
const point = dataset.data[index];
5959
const pixelX = chartRef.current.scales.x.getPixelForValue(point.x);
6060
const distance = Math.abs(mouseX - pixelX);
61-
61+
6262
if (distance < minDistance) {
6363
minDistance = distance;
6464
closestElement = element;
6565
}
6666
});
6767
}
68-
68+
6969
const { datasetIndex, index } = closestElement;
7070
const dataset = chartRef.current.data.datasets[datasetIndex];
7171
const point = dataset.data[index];
@@ -180,15 +180,15 @@ export default function ChartContainer({
180180
const elementKey = `${datasetIndex}-${idx}`;
181181
if (!seen.has(elementKey)) {
182182
// Validate element
183-
if (datasetIndex >= 0 && datasetIndex < chart.data.datasets.length &&
184-
idx >= 0 && idx < dataset.data.length) {
183+
if (datasetIndex >= 0 && datasetIndex < chart.data.datasets.length &&
184+
idx >= 0 && idx < dataset.data.length) {
185185
activeElements.push({ datasetIndex, index: idx });
186186
seen.add(elementKey);
187187
}
188188
}
189189
}
190190
});
191-
191+
192192
// Only set when activeElements are valid
193193
if (activeElements.length > 0) {
194194
try {
@@ -214,105 +214,44 @@ export default function ChartContainer({
214214
syncLockRef.current = false;
215215
}, []);
216216

217-
const parsedData = useMemo(() => {
218-
const enabled = files.filter(f => f.enabled !== false);
219-
return enabled.map(file => {
220-
if (!file.content) return { ...file, metricsData: {} };
221-
const lines = file.content.split('\n');
222-
const metricsData = {};
223-
224-
const stepCfg = {
225-
enabled: file.config?.useStepKeyword,
226-
keyword: file.config?.stepKeyword || 'step:'
227-
};
228-
229-
const extractStep = (line) => {
230-
if (!stepCfg.enabled) return null;
231-
const idx = line.toLowerCase().indexOf(stepCfg.keyword.toLowerCase());
232-
if (idx !== -1) {
233-
const after = line.substring(idx + stepCfg.keyword.length);
234-
const match = after.match(/[+-]?\d+/);
235-
if (match) {
236-
const s = parseInt(match[0], 10);
237-
if (!isNaN(s)) return s;
238-
}
239-
}
240-
return null;
241-
};
217+
const parsedData = useMemo(() => {
218+
const enabled = files.filter(f => f.enabled !== false);
219+
return enabled.map(file => {
220+
// Use pre-parsed data from worker
221+
let metricsData = file.metricsData || {};
242222

243-
const extractByKeyword = (linesArr, keyword) => {
244-
const results = [];
245-
const numberRegex = /[+-]?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/;
246-
linesArr.forEach(line => {
247-
const idx = line.toLowerCase().indexOf(keyword.toLowerCase());
248-
if (idx !== -1) {
249-
const after = line.substring(idx + keyword.length);
250-
const match = after.match(numberRegex);
251-
if (match) {
252-
const v = parseFloat(match[0]);
253-
if (!isNaN(v)) {
254-
const step = extractStep(line);
255-
results.push({ x: step !== null ? step : results.length, y: v });
256-
}
257-
}
258-
}
259-
});
260-
return results;
261-
};
262-
263-
metrics.forEach((metric, idx) => {
264-
const fileMetric = file.config?.metrics?.[idx] || metric;
265-
let points = [];
266-
if (fileMetric.mode === 'keyword') {
267-
points = extractByKeyword(lines, fileMetric.keyword);
268-
} else if (fileMetric.regex) {
269-
const reg = new RegExp(fileMetric.regex);
270-
lines.forEach(line => {
271-
reg.lastIndex = 0;
272-
const m = reg.exec(line);
273-
if (m && m[1]) {
274-
const v = parseFloat(m[1]);
275-
if (!isNaN(v)) {
276-
const step = extractStep(line);
277-
points.push({ x: step !== null ? step : points.length, y: v });
278-
}
279-
}
280-
});
281-
}
282-
283-
let key = '';
284-
if (metric.name && metric.name.trim()) {
285-
key = metric.name.trim();
286-
} else if (metric.keyword) {
287-
key = metric.keyword.replace(/[:]/g, '').trim();
288-
} else if (metric.regex) {
289-
const sanitized = metric.regex.replace(/[^a-zA-Z0-9_]/g, '').trim();
290-
key = sanitized || `metric${idx + 1}`;
291-
} else {
292-
key = `metric${idx + 1}`;
293-
}
223+
// Clone to avoid mutation during range application
224+
metricsData = { ...metricsData };
294225

295-
metricsData[key] = points;
296-
});
226+
const stepCfg = {
227+
enabled: file.config?.useStepKeyword,
228+
keyword: file.config?.stepKeyword || 'step:'
229+
};
297230

298231
const range = file.config?.dataRange;
299232
if (range && (range.start > 0 || range.end !== undefined)) {
300233
const applyRange = data => {
301-
if (data.length === 0) return data;
234+
if (!data || data.length === 0) return data;
302235
const start = Math.max(0, parseInt(range.start) || 0);
303236
const end = range.end !== undefined ? parseInt(range.end) : data.length;
304237
const endIndex = Math.min(data.length, end);
305238
return data.slice(start, endIndex);
306239
};
240+
241+
// If not using step keyword, reindex x to 0, 1, 2... after slicing
242+
// This matches original behavior where x-axis resets if we just treat lines as steps
307243
const reindex = data => stepCfg.enabled ? data : data.map((p, idx) => ({ x: idx, y: p.y }));
244+
308245
Object.keys(metricsData).forEach(k => {
309-
metricsData[k] = reindex(applyRange(metricsData[k]));
246+
if (metricsData[k]) {
247+
metricsData[k] = reindex(applyRange(metricsData[k]));
248+
}
310249
});
311250
}
312251

313252
return { ...file, metricsData };
314253
});
315-
}, [files, metrics]);
254+
}, [files]);
316255

317256
useEffect(() => {
318257
const maxStep = parsedData.reduce((m, f) => {
@@ -343,7 +282,7 @@ export default function ChartContainer({
343282
}
344283
return acc;
345284
}, []);
346-
285+
347286
return {
348287
datasets: uniqueItems.map((item, index) => {
349288
const color = colors[index % colors.length];
@@ -399,36 +338,46 @@ export default function ChartContainer({
399338
return result;
400339
};
401340

402-
const calculateYRange = useCallback((dataArray) => {
403-
let min = Infinity;
404-
let max = -Infinity;
341+
const getMaxDecimals = useCallback((dataArray) => {
342+
let maxDecimals = 0;
405343
dataArray.forEach(item => {
406344
item.data.forEach(point => {
407-
const inRange =
408-
(xRange.min === undefined || point.x >= xRange.min) &&
409-
(xRange.max === undefined || point.x <= xRange.max);
410-
if (inRange) {
411-
if (point.y < min) min = point.y;
412-
if (point.y > max) max = point.y;
345+
const valStr = point.y.toString();
346+
if (valStr.includes('.')) {
347+
const decimals = valStr.split('.')[1].length;
348+
if (decimals > maxDecimals) maxDecimals = decimals;
413349
}
414350
});
415351
});
416-
if (min === Infinity || max === -Infinity) {
417-
return { min: 0, max: 1, step: 1 };
418-
}
419-
if (min === max) {
420-
return { min: min - 1, max: max + 1, step: 1 };
421-
}
422-
const pad = (max - min) * 0.05;
423-
const paddedMin = min - pad;
424-
const paddedMax = max + pad;
425-
const range = paddedMax - paddedMin;
426-
let step = Math.pow(10, Math.floor(Math.log10(range)));
427-
if (range / step < 3) {
428-
step /= 10;
429-
}
430-
return { min: paddedMin, max: paddedMax, step };
431-
}, [xRange]);
352+
return Math.min(maxDecimals, 10); // Cap at 10 to avoid extreme cases
353+
}, []);
354+
355+
const calculateNiceScale = useCallback((min, max) => {
356+
if (min === Infinity || max === -Infinity) return { min: 0, max: 1, step: 0.1 };
357+
if (min === max) return { min: min - 0.5, max: max + 0.5, step: 0.1 };
358+
359+
// Calculate raw range
360+
let range = max - min;
361+
362+
// Calculate "nice" interval
363+
const roughStep = range / 5; // Aim for approx 5-6 ticks
364+
const magnitude = Math.pow(10, Math.floor(Math.log10(roughStep)));
365+
const normalizedStep = roughStep / magnitude;
366+
367+
let niceStep;
368+
if (normalizedStep < 1.5) niceStep = 1;
369+
else if (normalizedStep < 3) niceStep = 2;
370+
else if (normalizedStep < 7) niceStep = 5;
371+
else niceStep = 10;
372+
373+
const step = niceStep * magnitude;
374+
375+
// Calculate nice min and max
376+
const niceMin = Math.floor(min / step) * step;
377+
const niceMax = Math.ceil(max / step) * step;
378+
379+
return { min: niceMin, max: niceMax, step };
380+
}, []);
432381

433382
const chartOptions = useMemo(() => ({
434383
responsive: true,
@@ -510,8 +459,14 @@ export default function ChartContainer({
510459
return `Step ${context[0].parsed.x}`;
511460
},
512461
label: function (context) {
513-
const value = Number(context.parsed.y.toPrecision(4));
462+
// Dynamic precision handled in render loop via options update,
463+
// but here we need to access the chart options or dataset context
464+
// We'll use a default safe fallback or try to read from chart config if possible.
465+
// Actually, we can bind the precision in the render loop.
466+
// For now, let's use the raw value which is most accurate.
467+
const value = context.parsed.y;
514468
const label = context.dataset?.label || 'Dataset';
469+
// We will format this in the parent component's options generation
515470
return ` ${label}: ${value}`;
516471
},
517472
labelColor: function (context) {
@@ -544,11 +499,7 @@ export default function ChartContainer({
544499
display: true,
545500
title: { display: true, text: 'Value' },
546501
bounds: 'data',
547-
ticks: {
548-
callback: function (value) {
549-
return Number(value.toPrecision(2));
550-
}
551-
}
502+
// Ticks callback will be overridden in the render loop
552503
}
553504
},
554505
elements: { point: { radius: 0 } }
@@ -686,10 +637,41 @@ export default function ChartContainer({
686637
const dataArray = metricDataArrays[key] || [];
687638
const showComparison = dataArray.length >= 2;
688639

689-
const yRange = calculateYRange(dataArray);
690-
const yDecimals = Math.max(0, -Math.floor(Math.log10(yRange.step)));
640+
const yDecimals = getMaxDecimals(dataArray);
641+
642+
// Calculate min/max for scaling
643+
let min = Infinity;
644+
let max = -Infinity;
645+
dataArray.forEach(item => {
646+
item.data.forEach(point => {
647+
const inRange =
648+
(xRange.min === undefined || point.x >= xRange.min) &&
649+
(xRange.max === undefined || point.x <= xRange.max);
650+
if (inRange) {
651+
if (point.y < min) min = point.y;
652+
if (point.y > max) max = point.y;
653+
}
654+
});
655+
});
656+
657+
const yRange = calculateNiceScale(min, max);
658+
691659
const options = {
692660
...chartOptions,
661+
plugins: {
662+
...chartOptions.plugins,
663+
tooltip: {
664+
...chartOptions.plugins.tooltip,
665+
callbacks: {
666+
...chartOptions.plugins.tooltip.callbacks,
667+
label: function (context) {
668+
const value = Number(context.parsed.y).toFixed(yDecimals);
669+
const label = context.dataset?.label || 'Dataset';
670+
return ` ${label}: ${value}`;
671+
}
672+
}
673+
}
674+
},
693675
scales: {
694676
...chartOptions.scales,
695677
y: {
@@ -709,10 +691,41 @@ export default function ChartContainer({
709691
if (showComparison) {
710692
const compResult = buildComparisonChartData(dataArray);
711693
stats = compResult.stats.length > 0 ? compResult.stats : null;
712-
const compRange = calculateYRange(compResult.datasets);
713-
const compDecimals = Math.max(0, -Math.floor(Math.log10(compRange.step)));
694+
695+
// Calculate comparison range
696+
let cMin = Infinity;
697+
let cMax = -Infinity;
698+
compResult.datasets.forEach(ds => {
699+
ds.data.forEach(point => {
700+
const inRange =
701+
(xRange.min === undefined || point.x >= xRange.min) &&
702+
(xRange.max === undefined || point.x <= xRange.max);
703+
if (inRange) {
704+
if (point.y < cMin) cMin = point.y;
705+
if (point.y > cMax) cMax = point.y;
706+
}
707+
});
708+
});
709+
710+
const compRange = calculateNiceScale(cMin, cMax);
711+
const compDecimals = Math.max(4, getMaxDecimals(compResult.datasets)); // Ensure at least 4 for diffs
712+
714713
const compOptions = {
715714
...chartOptions,
715+
plugins: {
716+
...chartOptions.plugins,
717+
tooltip: {
718+
...chartOptions.plugins.tooltip,
719+
callbacks: {
720+
...chartOptions.plugins.tooltip.callbacks,
721+
label: function (context) {
722+
const value = Number(context.parsed.y).toFixed(compDecimals);
723+
const label = context.dataset?.label || 'Dataset';
724+
return ` ${label}: ${value}`;
725+
}
726+
}
727+
}
728+
},
716729
scales: {
717730
...chartOptions.scales,
718731
y: {

0 commit comments

Comments
 (0)