Skip to content

Commit

Permalink
[Lens] Workspace panel dimensions by chart type (#168651)
Browse files Browse the repository at this point in the history
## Summary

Close #136993

All charts remain the same except the following

## Metric
Each tile gets `200px` if there are multiple and `300px` if it's a
single. The default background is EUI empty.

<img width="600" alt="Screenshot 2023-10-30 at 3 55 33 PM"
src="https://github.com/elastic/kibana/assets/315764/74d7e6dc-c90a-4f60-bf56-94ab1556ad42">

<img width="600" alt="Screenshot 2023-10-30 at 3 56 36 PM"
src="https://github.com/elastic/kibana/assets/315764/3254160a-b18a-4c04-b059-f20b8f1f246a">

## XY
Vertical time-series charts have `16:9` ratio but will stretch to any
available width and won't shrink below 300px height.


https://github.com/elastic/kibana/assets/315764/3e056ae8-bc65-4851-9ad9-6c8a81bdf58a




## Gauge

Gauge gets `600px` for the long side, `300px` for the short side.

<img width="600" alt="Screenshot 2023-11-28 at 11 22 20 AM"
src="https://github.com/elastic/kibana/assets/315764/2b774515-f060-4c26-a0aa-efdeebfff0e5">

<img width="600" alt="Screenshot 2023-11-28 at 11 22 33 AM"
src="https://github.com/elastic/kibana/assets/315764/62181021-d88a-4cb6-862b-42768a2df13e">



## Known issues
- [ ] text resizing on the metric
elastic/elastic-charts#2238


https://github.com/elastic/kibana/assets/315764/0162f461-b544-44a9-971c-b2a0265d7d3a

- [x] gauge isn't showing veil on willRender

### 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

Flaky test runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4755

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: nickofthyme <nicholas.partridge@elastic.co>
Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
Co-authored-by: Marta Bondyra <4283304+mbondyra@users.noreply.github.com>
  • Loading branch information
6 people committed Jan 14, 2024
1 parent 2a44f75 commit e6a5647
Show file tree
Hide file tree
Showing 53 changed files with 1,063 additions and 245 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pageLoadAssetSize:
expressionLegacyMetricVis: 23121
expressionMetric: 22238
expressionMetricVis: 23121
expressionPartitionVis: 28000
expressionPartitionVis: 29000
expressionRepeatImage: 22341
expressionRevealImage: 25675
expressions: 140958
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import React, { useEffect } from 'react';
import { useCallback, useRef, useState } from 'react';
import fastIsEqual from 'fast-deep-equal';
import { useResizeObserver } from '@elastic/eui';
import { euiThemeVars } from '@kbn/ui-theme';
import type { ChartSizeSpec } from './types';

/**
* This hook is used to show a veil over the chart while it is being resized
* in response to a change in the container dimensions.
*
* It is only relevant if client dimensions are being requested based on chart configuration.
*
* This whole feature is a nice-to-have. If it proves to be a source of bugs,
* we can consider removing it and accepting the aesthetic drawback.
*/
export function useSizeTransitionVeil(
chartSizeSpec: ChartSizeSpec,
setChartSize: (d: ChartSizeSpec) => void,
// should be retrieved from handlers.shouldUseSizeTransitionVeil function
shouldUseVeil: boolean
) {
const containerRef = useRef<HTMLDivElement>(null);
const containerSize = useResizeObserver(containerRef.current);
const currentContainerSize = useRef<{ width: number; height: number }>(containerSize);

// This useEffect hook is here to make up for the fact that the initial container size
// is not correctly reported by the useResizeObserver hook (see https://github.com/elastic/eui/issues/7458).
useEffect(() => {
currentContainerSize.current = {
width: containerRef.current?.clientWidth ?? 0,
height: containerRef.current?.clientHeight ?? 0,
};
}, []);

const [showVeil, setShowVeil] = useState(false);
const currentChartSizeSpec = useRef<ChartSizeSpec>();
const specJustChanged = useRef<boolean>(false);

useEffect(() => {
if (!fastIsEqual(containerSize, currentContainerSize.current) && specJustChanged.current) {
// If the container size has changed, we need to show the veil to hide the chart since it
// be rendered based on the previous container size before being updated to the current size.
//
// 1. we show the veil
// 2. the charts library will render the chart based on the previous container dimensions
// 3. the charts library will resize the chart to the updated container dimensions
// 4. we hide the veil
setShowVeil(true);
currentContainerSize.current = containerSize;
}
}, [setShowVeil, containerSize]);

useEffect(() => {
if (!fastIsEqual(chartSizeSpec, currentChartSizeSpec.current)) {
setChartSize(chartSizeSpec);
currentChartSizeSpec.current = chartSizeSpec;
specJustChanged.current = true;

setTimeout(() => {
specJustChanged.current = false;
}, 500);
}
}, [chartSizeSpec, setChartSize]);

const onResize = useCallback(() => {
setShowVeil(false);
}, []);

return {
veil: (
<div
css={{
height: '100%',
width: '100%',
backgroundColor: euiThemeVars.euiColorEmptyShade,
position: 'absolute',
zIndex: 1,
display: shouldUseVeil && showVeil ? 'block' : 'none',
}}
/>
),
onResize,
containerRef,
};
}
4 changes: 3 additions & 1 deletion src/plugins/chart_expressions/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@ export {
getOverridesFor,
isOnAggBasedEditor,
} from './utils';
export type { Simplify, MakeOverridesSerializable } from './types';
export type { Simplify, MakeOverridesSerializable, ChartSizeSpec, ChartSizeEvent } from './types';
export { isChartSizeEvent } from './types';
export { getColorCategories } from './color_categories';
export { useSizeTransitionVeil } from './chart_size_transition_veil';
5 changes: 4 additions & 1 deletion src/plugins/chart_expressions/common/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
"outDir": "target/types",
"types": [
"jest",
"node"
"node",
"@emotion/react/types/css-prop",
]
},
"include": [
"**/*.ts",
"**/*.tsx",
],
"exclude": [
"target/**/*"
Expand All @@ -17,5 +19,6 @@
"@kbn/core-execution-context-common",
"@kbn/expressions-plugin",
"@kbn/data-plugin",
"@kbn/ui-theme",
]
}
25 changes: 25 additions & 0 deletions src/plugins/chart_expressions/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import type { ExpressionRendererEvent } from '@kbn/expressions-plugin/public';
import React from 'react';

export type Simplify<T> = { [KeyType in keyof T]: T[KeyType] } & {};
Expand All @@ -26,3 +27,27 @@ export type MakeOverridesSerializable<T> = {
? MakeOverridesSerializable<T[KeyType]>
: NonNullable<T[KeyType]>;
};

export interface ChartSizeEvent extends ExpressionRendererEvent {
name: 'chartSize';
data: ChartSizeSpec;
}

export type ChartSizeUnit = 'pixels' | 'percentage';

interface ChartSizeDimensions {
x?: { value: number; unit: ChartSizeUnit };
y?: { value: number; unit: ChartSizeUnit };
}

export interface ChartSizeSpec {
// if maxDimensions are provided, the aspect ratio will be computed from them
maxDimensions?: ChartSizeDimensions;
minDimensions?: ChartSizeDimensions;
aspectRatio?: { x: number; y: number };
}

export function isChartSizeEvent(event: ExpressionRendererEvent): event is ChartSizeEvent {
const expectedName: ChartSizeEvent['name'] = 'chartSize';
return event.name === expectedName;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { PersistedState } from '@kbn/visualizations-plugin/public';
import type { ChartsPluginSetup } from '@kbn/charts-plugin/public';
import type { IFieldFormat, SerializedFieldFormat } from '@kbn/field-formats-plugin/common';
import type { AllowedSettingsOverrides, AllowedChartOverrides } from '@kbn/charts-plugin/common';
import type { ChartSizeSpec } from '@kbn/chart-expressions-common';
import type { AllowedGaugeOverrides, GaugeExpressionProps } from './expression_functions';

export type FormatFactory = (mapping?: SerializedFieldFormat) => IFieldFormat;
Expand All @@ -22,4 +23,6 @@ export type GaugeRenderProps = GaugeExpressionProps & {
renderComplete: () => void;
uiState: PersistedState;
overrides?: AllowedGaugeOverrides & AllowedSettingsOverrides & AllowedChartOverrides;
shouldUseVeil: boolean;
setChartSize: (d: ChartSizeSpec) => void;
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ describe('GaugeComponent', function () {
paletteService: await paletteThemeService.getPalettes(),
uiState,
renderComplete: jest.fn(),
setChartSize: jest.fn(),
shouldUseVeil: false,
};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import type { PaletteOutput } from '@kbn/coloring';
import { FieldFormat } from '@kbn/field-formats-plugin/common';
import type { CustomPaletteState } from '@kbn/charts-plugin/public';
import { EmptyPlaceholder } from '@kbn/charts-plugin/public';
import { getOverridesFor } from '@kbn/chart-expressions-common';
import {
type ChartSizeSpec,
getOverridesFor,
useSizeTransitionVeil,
} from '@kbn/chart-expressions-common';
import { isVisDimension } from '@kbn/visualizations-plugin/common/utils';
import { i18n } from '@kbn/i18n';
import {
Expand Down Expand Up @@ -178,6 +182,8 @@ export const GaugeComponent: FC<GaugeRenderProps> = memo(
chartsThemeService,
renderComplete,
overrides,
shouldUseVeil,
setChartSize,
}) => {
const {
shape: gaugeType,
Expand Down Expand Up @@ -253,6 +259,26 @@ export const GaugeComponent: FC<GaugeRenderProps> = memo(
[renderComplete]
);

const chartSizeSpec: ChartSizeSpec = {
maxDimensions: {
...(gaugeType === GaugeShapes.HORIZONTAL_BULLET
? {
x: { value: 600, unit: 'pixels' },
y: { value: 300, unit: 'pixels' },
}
: {
y: { value: 600, unit: 'pixels' },
x: { value: 300, unit: 'pixels' },
}),
},
};

const { veil, onResize, containerRef } = useSizeTransitionVeil(
chartSizeSpec,
setChartSize,
shouldUseVeil
);

const table = data;
const accessors = getAccessorsFromArgs(args, table.columns);

Expand Down Expand Up @@ -359,7 +385,8 @@ export const GaugeComponent: FC<GaugeRenderProps> = memo(
: {};

return (
<div className="gauge__wrapper">
<div className="gauge__wrapper" ref={containerRef}>
{veil}
<Chart {...getOverridesFor(overrides, 'chart')}>
<Settings
noResults={<EmptyPlaceholder icon={icon} renderComplete={onRenderChange} />}
Expand All @@ -369,6 +396,7 @@ export const GaugeComponent: FC<GaugeRenderProps> = memo(
ariaLabel={args.ariaLabel}
ariaUseDefaultSummary={!args.ariaLabel}
onRenderChange={onRenderChange}
onResize={onResize}
locale={i18n.getLocale()}
{...getOverridesFor(overrides, 'settings')}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { ExpressionRenderDefinition } from '@kbn/expressions-plugin/common/expression_renderers';
import { StartServicesGetter } from '@kbn/kibana-utils-plugin/public';
import { METRIC_TYPE } from '@kbn/analytics';
import { extractContainerType, extractVisualizationType } from '@kbn/chart-expressions-common';
import {
ChartSizeEvent,
extractContainerType,
extractVisualizationType,
} from '@kbn/chart-expressions-common';
import { ExpressionGaugePluginStart } from '../plugin';
import { EXPRESSION_GAUGE_NAME, GaugeExpressionProps, GaugeShapes } from '../../common';
import { getFormatService, getPaletteService } from '../services';
Expand Down Expand Up @@ -66,16 +70,27 @@ export const gaugeRenderer: (
handlers.done();
};

const setChartSize = (chartSizeSpec: ChartSizeEvent['data']) => {
const event: ChartSizeEvent = {
name: 'chartSize',
data: chartSizeSpec,
};

handlers.event(event);
};

const { GaugeComponent } = await import('../components/gauge_component');
render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<div className="gauge-container" data-test-subj="gaugeChart">
<GaugeComponent
{...config}
setChartSize={setChartSize}
formatFactory={getFormatService().deserialize}
chartsThemeService={plugins.charts.theme}
paletteService={getPaletteService()}
renderComplete={renderComplete}
shouldUseVeil={handlers.shouldUseSizeTransitionVeil()}
uiState={handlers.uiState as PersistedState}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { ExpressionRenderDefinition } from '@kbn/expressions-plugin/common/expression_renderers';
import { StartServicesGetter } from '@kbn/kibana-utils-plugin/public';
import { METRIC_TYPE } from '@kbn/analytics';
import { extractContainerType, extractVisualizationType } from '@kbn/chart-expressions-common';
import {
ChartSizeEvent,
extractContainerType,
extractVisualizationType,
} from '@kbn/chart-expressions-common';
import { I18nProvider } from '@kbn/i18n-react';
import { MultiFilterEvent } from '../../common/types';
import { ExpressionHeatmapPluginStart } from '../plugin';
Expand Down Expand Up @@ -78,6 +82,18 @@ export const heatmapRenderer: (
handlers.done();
};

const chartSizeEvent: ChartSizeEvent = {
name: 'chartSize',
data: {
maxDimensions: {
x: { value: 100, unit: 'percentage' },
y: { value: 100, unit: 'percentage' },
},
},
};

handlers.event(chartSizeEvent);

const timeZone = getTimeZone(getUISettings());
const { HeatmapComponent } = await import('../components/heatmap_component');
const { isInteractive } = handlers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ import {
import { getColumnByAccessor } from '@kbn/visualizations-plugin/common/utils';
import { Datatable } from '@kbn/expressions-plugin/common';
import { StartServicesGetter } from '@kbn/kibana-utils-plugin/public';
import { extractContainerType, extractVisualizationType } from '@kbn/chart-expressions-common';
import {
ChartSizeEvent,
extractContainerType,
extractVisualizationType,
} from '@kbn/chart-expressions-common';
import { ExpressionLegacyMetricPluginStart } from '../plugin';
import { EXPRESSION_METRIC_NAME, MetricVisRenderConfig, VisParams } from '../../common';

Expand Down Expand Up @@ -92,6 +96,18 @@ export const getMetricVisRenderer: (
handlers.done();
};

const chartSizeEvent: ChartSizeEvent = {
name: 'chartSize',
data: {
maxDimensions: {
x: { value: 100, unit: 'percentage' },
y: { value: 100, unit: 'percentage' },
},
},
};

handlers.event(chartSizeEvent);

render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<VisualizationContainer
Expand Down
Loading

0 comments on commit e6a5647

Please sign in to comment.