Skip to content

Commit

Permalink
fix(partition): rendering with small radius (#2273)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
markov00 and elastic-datavis[bot] committed Dec 7, 2023
1 parent 29cdcb3 commit 95a8537
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 41 deletions.
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down
31 changes: 31 additions & 0 deletions packages/charts/src/chart_types/partition_chart/partition.test.tsx
Expand Up @@ -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);
});
});
});
Expand Up @@ -165,7 +165,7 @@ export class HighlighterComponent extends React.Component<HighlighterProps> {
<>
<defs>
{renderedHighlightSet
.filter(({ geometries }) => geometries.length > 0)
.filter(({ geometries, outerRadius }) => geometries.length > 0 && outerRadius > 0)
.map(
({
geometries,
Expand All @@ -187,38 +187,40 @@ export class HighlighterComponent extends React.Component<HighlighterProps> {
),
)}
</defs>
{renderedHighlightSet.map(
({
diskCenter,
outerRadius,
index,
innerIndex,
layout,
marginLeftPx,
marginTopPx,
panel: { innerWidth, innerHeight },
}) =>
isSunburst(layout) ? (
<circle
key={`${index}__${innerIndex}`}
cx={diskCenter.x}
cy={diskCenter.y}
r={outerRadius}
mask={`url(#${maskId(index, innerIndex)})`}
className="echHighlighter__mask"
/>
) : (
<rect
key={`${index}__${innerIndex}`}
x={marginLeftPx}
y={marginTopPx}
width={innerWidth}
height={innerHeight}
mask={`url(#${maskId(index, innerIndex)})`}
className="echHighlighter__mask"
/>
),
)}
{renderedHighlightSet
.filter(({ outerRadius }) => outerRadius > 0)
.map(
({
diskCenter,
outerRadius,
index,
innerIndex,
layout,
marginLeftPx,
marginTopPx,
panel: { innerWidth, innerHeight },
}) =>
isSunburst(layout) ? (
<circle
key={`${index}__${innerIndex}`}
cx={diskCenter.x}
cy={diskCenter.y}
r={outerRadius}
mask={`url(#${maskId(index, innerIndex)})`}
className="echHighlighter__mask"
/>
) : (
<rect
key={`${index}__${innerIndex}`}
x={marginLeftPx}
y={marginTopPx}
width={innerWidth}
height={innerHeight}
mask={`url(#${maskId(index, innerIndex)})`}
className="echHighlighter__mask"
/>
),
)}
</>
);
}
Expand All @@ -228,7 +230,7 @@ export class HighlighterComponent extends React.Component<HighlighterProps> {
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 }) => (
<g key={`${index}|${innerIndex}`} transform={`translate(${diskCenter.x}, ${diskCenter.y})`}>
{renderGeometries(
Expand Down

0 comments on commit 95a8537

Please sign in to comment.