Skip to content

Commit

Permalink
[Lens] Fix no data collapse by scenario with Metric chart (elastic#19…
Browse files Browse the repository at this point in the history
…0966)

## Summary

Fixes elastic#182628 

This PR provides a fix for the editor crash on no data when using
collapsing by.
In addition the rendering of no data in such scenario now matches the no
data rendering with a single tile.

<img width="533" alt="Screenshot 2024-08-21 at 12 57 43"
src="https://github.com/user-attachments/assets/71c40e3b-5e1a-48f1-a03f-5a4a5774e9d7">
<img width="1008" alt="Screenshot 2024-08-21 at 12 57 28"
src="https://github.com/user-attachments/assets/e35b9612-743b-412e-9565-0afcb5fd8735">
<img width="1229" alt="Screenshot 2024-08-21 at 12 56 31"
src="https://github.com/user-attachments/assets/f64f38f8-8995-4214-9954-386aac62f0c7">


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
dej611 authored Aug 21, 2024
1 parent 8b7de03 commit 9c35c15
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,12 @@ describe('MetricVisComponent', function () {

const [[visConfig]] = component.find(Metric).props().data!;

expect(visConfig!.value).toMatchInlineSnapshot(
`
expect(visConfig!.value).toMatchInlineSnapshot(`
Array [
"text-28.984375",
"text-100",
]
`
);
`);
});

it('should display multi-values numeric values formatted and without quotes', () => {
Expand All @@ -451,14 +449,24 @@ describe('MetricVisComponent', function () {

const [[visConfig]] = component.find(Metric).props().data!;

expect(visConfig!.value).toMatchInlineSnapshot(
`
expect(visConfig!.value).toMatchInlineSnapshot(`
Array [
"number-28.984375",
"number-100",
]
`
);
`);
});

it('should display an empty tile if no data is provided', () => {
const newTable = {
...table,
rows: [],
};
const component = shallow(<MetricVis config={config} data={newTable} {...defaultProps} />);

const [[visConfig]] = component.find(Metric).props().data!;

expect(visConfig!.value).toMatchInlineSnapshot(`NaN`);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export const MetricVis = ({
);

const onWillRender = useCallback(() => {
const maxTileSideLength = grid.current.length * grid.current[0].length > 1 ? 200 : 300;
const maxTileSideLength = grid.current.length * grid.current[0]?.length > 1 ? 200 : 300;
const event: ChartSizeEvent = {
name: 'chartSize',
data: {
Expand Down Expand Up @@ -197,8 +197,15 @@ export const MetricVis = ({
? getColumnByAccessor(config.dimensions.max, data.columns)?.id
: undefined;

// For a sigle tile configuration, either no breakdown or with a collapse by, provide
// a fallback in case of missing data. Make sure to provide an exact "null" value to render a N/A metric.
// For reference, undefined will render as - instead of N/A and it is used in a breakdown scenario
const firstRowForNonBreakdown = (
data.rows.length ? data.rows : [{ [primaryMetricColumn.id]: null }]
).slice(0, 1);

const metricConfigs: MetricSpec['data'][number] = (
breakdownByColumn ? data.rows : data.rows.slice(0, 1)
breakdownByColumn ? data.rows : firstRowForNonBreakdown
).map((row, rowIdx) => {
const value: number | string =
row[primaryMetricColumn.id] !== null ? row[primaryMetricColumn.id] : NaN;
Expand Down

0 comments on commit 9c35c15

Please sign in to comment.