From 95a8537248eff8e72db21f960852b20ccd64fa62 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Thu, 7 Dec 2023 16:56:01 +0100 Subject: [PATCH] fix(partition): rendering with small radius (#2273) * fix(partition): rendering with small radius * remove resizable from story [update vrts] * test(vrt): update screenshots [skip ci] * move VRT to unit test --------- Co-authored-by: elastic-datavis[bot] <98618603+elastic-datavis[bot]@users.noreply.github.com> --- .../layout/viewmodel/viewmodel.ts | 14 ++-- .../partition_chart/partition.test.tsx | 31 ++++++++ .../renderer/dom/highlighter.tsx | 70 ++++++++++--------- 3 files changed, 74 insertions(+), 41 deletions(-) diff --git a/packages/charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts b/packages/charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts index 2f22d6863c..7a2a839d66 100644 --- a/packages/charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts +++ b/packages/charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts @@ -341,8 +341,14 @@ export function shapeViewModel( y: marginTopPx, }; + // use the smaller of the two sizes, as a circle fits into a square + const circleMaximumSize = Math.min( + panel.innerWidth, + panel.innerHeight - (panel.title.length > 0 ? panel.fontSize * 2 : 0), + ); + const outerRadius: Radius = Math.min(outerSizeRatio * circleMaximumSize, circleMaximumSize - sectorLineWidth) / 2; // don't render anything if the total, the width or height is not positive - if (!(width > 0) || !(height > 0) || tree.length === 0) { + if (!(width > 0) || !(height > 0) || tree.length === 0 || outerRadius <= 0) { return nullShapeViewModel(layout, style, diskCenter); } @@ -371,12 +377,6 @@ export function shapeViewModel( return !layer || !layer.showAccessor || layer.showAccessor(entryKey(n.node)); }); - // use the smaller of the two sizes, as a circle fits into a square - const circleMaximumSize = Math.min( - panel.innerWidth, - panel.innerHeight - (panel.title.length > 0 ? panel.fontSize * 2 : 0), - ); - const outerRadius: Radius = Math.min(outerSizeRatio * circleMaximumSize, circleMaximumSize - sectorLineWidth) / 2; const innerRadius: Radius = outerRadius - (1 - emptySizeRatio) * outerRadius; const treeHeight = shownChildNodes.reduce((p: number, n: Part) => Math.max(p, entryValue(n.node).depth), 0); // 1: pie, 2: two-ring donut etc. const ringThickness = (outerRadius - innerRadius) / treeHeight; diff --git a/packages/charts/src/chart_types/partition_chart/partition.test.tsx b/packages/charts/src/chart_types/partition_chart/partition.test.tsx index caad8cb57f..cef9392370 100644 --- a/packages/charts/src/chart_types/partition_chart/partition.test.tsx +++ b/packages/charts/src/chart_types/partition_chart/partition.test.tsx @@ -188,5 +188,36 @@ describe('Retain hierarchy even with arbitrary names', () => { partitionMultiGeometries(store.getState()); }).not.toThrow(); }); + it('avoid rendering on too small outer radius', () => { + MockStore.updateDimensions(store, { width: 800, height: 10, top: 0, left: 0 }); + MockStore.addSpecs( + [ + MockGlobalSpec.settings({ + showLegend: false, + theme: { + chartMargins: { top: 5, bottom: 4 }, + }, + }), + MockSeriesSpec.treemap({ + data: [ + { cat: 'a', val: 1 }, + { cat: 'b', val: 1 }, + { cat: 'c', val: 0 }, + { cat: 'd', val: 1 }, + ], + valueAccessor: (d: { cat: string; val: number }) => d.val, + layers: [ + { + groupByRollup: (d: { cat: string; val: number }) => d.cat, + }, + ], + }), + ], + store, + ); + const geometries = partitionMultiGeometries(store.getState()); + const outerRadius = geometries?.[0]?.outerRadius ?? 0; + expect(outerRadius).toBeGreaterThanOrEqual(0); + }); }); }); diff --git a/packages/charts/src/chart_types/partition_chart/renderer/dom/highlighter.tsx b/packages/charts/src/chart_types/partition_chart/renderer/dom/highlighter.tsx index 7c56161cbf..e2154269e2 100644 --- a/packages/charts/src/chart_types/partition_chart/renderer/dom/highlighter.tsx +++ b/packages/charts/src/chart_types/partition_chart/renderer/dom/highlighter.tsx @@ -165,7 +165,7 @@ export class HighlighterComponent extends React.Component { <> {renderedHighlightSet - .filter(({ geometries }) => geometries.length > 0) + .filter(({ geometries, outerRadius }) => geometries.length > 0 && outerRadius > 0) .map( ({ geometries, @@ -187,38 +187,40 @@ export class HighlighterComponent extends React.Component { ), )} - {renderedHighlightSet.map( - ({ - diskCenter, - outerRadius, - index, - innerIndex, - layout, - marginLeftPx, - marginTopPx, - panel: { innerWidth, innerHeight }, - }) => - isSunburst(layout) ? ( - - ) : ( - - ), - )} + {renderedHighlightSet + .filter(({ outerRadius }) => outerRadius > 0) + .map( + ({ + diskCenter, + outerRadius, + index, + innerIndex, + layout, + marginLeftPx, + marginTopPx, + panel: { innerWidth, innerHeight }, + }) => + isSunburst(layout) ? ( + + ) : ( + + ), + )} ); } @@ -228,7 +230,7 @@ export class HighlighterComponent extends React.Component { canvasDimension: { width }, } = this.props; return this.props.highlightSets - .filter(({ geometries }) => geometries.length > 0) + .filter(({ geometries, outerRadius }) => geometries.length > 0 && outerRadius > 0) .map(({ index, innerIndex, layout, geometries, diskCenter, geometriesFoci }) => ( {renderGeometries(