diff --git a/src/plugins/chart_expressions/common/index.ts b/src/plugins/chart_expressions/common/index.ts index 4373260657909..0983b1ed28d4d 100644 --- a/src/plugins/chart_expressions/common/index.ts +++ b/src/plugins/chart_expressions/common/index.ts @@ -6,5 +6,10 @@ * Side Public License, v 1. */ -export { extractContainerType, extractVisualizationType, getOverridesFor } from './utils'; +export { + extractContainerType, + extractVisualizationType, + getOverridesFor, + isOnAggBasedEditor, +} from './utils'; export type { Simplify, MakeOverridesSerializable } from './types'; diff --git a/src/plugins/chart_expressions/common/utils.test.ts b/src/plugins/chart_expressions/common/utils.test.ts index 2ed71e9a17b92..48519c9f6f1a9 100644 --- a/src/plugins/chart_expressions/common/utils.test.ts +++ b/src/plugins/chart_expressions/common/utils.test.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { getOverridesFor } from './utils'; +import { getOverridesFor, isOnAggBasedEditor } from './utils'; describe('Overrides utilities', () => { describe('getOverridesFor', () => { @@ -31,3 +31,77 @@ describe('Overrides utilities', () => { }); }); }); + +describe('isOnAggBasedEditor', () => { + it('should return false if is on dashboard', () => { + const context = { + type: 'dashboard', + description: 'test', + child: { + type: 'lens', + name: 'lnsPie', + id: 'd8bb29a7-13a4-43fa-a162-d7705050bb6c', + description: 'test', + url: '/gdu/app/lens#/edit_by_value', + }, + }; + expect(isOnAggBasedEditor(context)).toEqual(false); + }); + + it('should return false if is on editor but lens', () => { + const context = { + type: 'application', + description: 'test', + child: { + type: 'lens', + name: 'lnsPie', + id: 'd8bb29a7-13a4-43fa-a162-d7705050bb6c', + description: 'test', + url: '/gdu/app/lens#/edit_by_value', + }, + }; + expect(isOnAggBasedEditor(context)).toEqual(false); + }); + + it('should return false if is on dashboard but agg_based', () => { + const context = { + type: 'dashboard', + description: 'test', + child: { + type: 'agg_based', + name: 'pie', + id: 'd8bb29a7-13a4-43fa-a162-d7705050bb6c', + description: 'test', + url: '', + }, + }; + expect(isOnAggBasedEditor(context)).toEqual(false); + }); + + it('should return true if is on editor but agg_based', () => { + const context = { + type: 'application', + description: 'test', + child: { + type: 'agg_based', + name: 'pie', + id: 'd8bb29a7-13a4-43fa-a162-d7705050bb6c', + description: 'test', + url: '', + }, + }; + expect(isOnAggBasedEditor(context)).toEqual(true); + }); + + it('should return false if child is missing', () => { + const context = { + type: 'application', + description: 'test', + }; + expect(isOnAggBasedEditor(context)).toEqual(false); + }); + + it('should return false if context is missing', () => { + expect(isOnAggBasedEditor()).toEqual(false); + }); +}); diff --git a/src/plugins/chart_expressions/common/utils.ts b/src/plugins/chart_expressions/common/utils.ts index 2966532c44117..db2e564efc4b3 100644 --- a/src/plugins/chart_expressions/common/utils.ts +++ b/src/plugins/chart_expressions/common/utils.ts @@ -20,6 +20,31 @@ export const extractContainerType = (context?: KibanaExecutionContext): string | } }; +/* Function to identify if the pie is rendered inside the aggBased editor + Context comes with this format + { + type: 'dashboard', // application for lens, agg based charts + description: 'test', + child: { + type: 'lens', // agg_based for legacy editor + name: 'pie', + id: 'id', + description: 'test', + url: '', + }, + }; */ +export const isOnAggBasedEditor = (context?: KibanaExecutionContext): boolean => { + if (context) { + return Boolean( + context.type && + context.type === 'application' && + context.child && + context.child.type === 'agg_based' + ); + } + return false; +}; + export const extractVisualizationType = (context?: KibanaExecutionContext): string | undefined => { if (context) { const recursiveGet = (item: KibanaExecutionContext): KibanaExecutionContext | undefined => { diff --git a/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.tsx b/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.tsx index 94590ff164555..4ce300a7d9bb9 100644 --- a/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.tsx +++ b/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.tsx @@ -93,6 +93,7 @@ export type PartitionVisComponentProps = Omit< palettesRegistry: PaletteRegistry; services: Pick; columnCellValueActions: ColumnCellValueActions; + hasOpenedOnAggBasedEditor: boolean; }; const PartitionVisComponent = (props: PartitionVisComponentProps) => { @@ -105,6 +106,7 @@ const PartitionVisComponent = (props: PartitionVisComponentProps) => { syncColors, interactive, overrides, + hasOpenedOnAggBasedEditor, } = props; const visParams = useMemo(() => filterOutConfig(visType, preVisParams), [preVisParams, visType]); const chartTheme = props.chartsThemeService.useChartsTheme(); @@ -148,7 +150,7 @@ const PartitionVisComponent = (props: PartitionVisComponentProps) => { const [showLegend, setShowLegend] = useState(() => showLegendDefault()); const showToggleLegendElement = props.uiState !== undefined; - + const [chartIsLoaded, setChartIsLoaded] = useState(false); const [containerDimensions, setContainerDimensions] = useState< undefined | PieContainerDimensions >(); @@ -156,12 +158,14 @@ const PartitionVisComponent = (props: PartitionVisComponentProps) => { const parentRef = useRef(null); useEffect(() => { - if (parentRef && parentRef.current) { + // chart should be loaded to compute the dimensions + // otherwise the height is set to 0 + if (parentRef && parentRef.current && chartIsLoaded) { const parentHeight = parentRef.current!.getBoundingClientRect().height; const parentWidth = parentRef.current!.getBoundingClientRect().width; setContainerDimensions({ width: parentWidth, height: parentHeight }); } - }, [parentRef]); + }, [chartIsLoaded, parentRef]); useEffect(() => { const legendShow = showLegendDefault(); @@ -172,6 +176,7 @@ const PartitionVisComponent = (props: PartitionVisComponentProps) => { (isRendered: boolean = true) => { if (isRendered) { props.renderComplete(); + setChartIsLoaded(true); } }, [props] @@ -363,8 +368,16 @@ const PartitionVisComponent = (props: PartitionVisComponentProps) => { ) as Partial; const themeOverrides = useMemo( - () => getPartitionTheme(visType, visParams, chartTheme, containerDimensions, rescaleFactor), - [visType, visParams, chartTheme, containerDimensions, rescaleFactor] + () => + getPartitionTheme( + visType, + visParams, + chartTheme, + containerDimensions, + rescaleFactor, + hasOpenedOnAggBasedEditor + ), + [visType, visParams, chartTheme, containerDimensions, rescaleFactor, hasOpenedOnAggBasedEditor] ); const fixedViewPort = document.getElementById('app-fixed-viewport'); diff --git a/src/plugins/chart_expressions/expression_partition_vis/public/expression_renderers/partition_vis_renderer.tsx b/src/plugins/chart_expressions/expression_partition_vis/public/expression_renderers/partition_vis_renderer.tsx index 056ba6b7136ce..2379096796639 100644 --- a/src/plugins/chart_expressions/expression_partition_vis/public/expression_renderers/partition_vis_renderer.tsx +++ b/src/plugins/chart_expressions/expression_partition_vis/public/expression_renderers/partition_vis_renderer.tsx @@ -21,7 +21,11 @@ import { withSuspense } from '@kbn/presentation-util-plugin/public'; import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; import { METRIC_TYPE } from '@kbn/analytics'; import { getColumnByAccessor } from '@kbn/visualizations-plugin/common/utils'; -import { extractContainerType, extractVisualizationType } from '@kbn/chart-expressions-common'; +import { + extractContainerType, + extractVisualizationType, + isOnAggBasedEditor, +} from '@kbn/chart-expressions-common'; import { VisTypePieDependencies } from '../plugin'; import { PARTITION_VIS_RENDERER_NAME } from '../../common/constants'; import { CellValueAction, GetCompatibleCellValueActions } from '../types'; @@ -110,6 +114,8 @@ export const getPartitionVisRenderer: ( plugins.charts.palettes.getPalettes(), ]); + const hasOpenedOnAggBasedEditor = isOnAggBasedEditor(handlers.getExecutionContext()); + render( @@ -128,6 +134,7 @@ export const getPartitionVisRenderer: ( syncColors={syncColors} columnCellValueActions={columnCellValueActions} overrides={overrides} + hasOpenedOnAggBasedEditor={hasOpenedOnAggBasedEditor} /> diff --git a/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.test.ts b/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.test.ts index 345d6ce068d0c..31d025ac0310f 100644 --- a/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.test.ts +++ b/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.test.ts @@ -144,9 +144,16 @@ const runPieDonutWaffleTestSuites = (chartType: ChartTypes, visParams: Partition }); }); - it('should return adjusted padding settings if dimensions are specified', () => { + it('should return adjusted padding settings if dimensions are specified and is on aggBased editor', () => { const specifiedDimensions = { width: 2000, height: 2000 }; - const theme = getPartitionTheme(chartType, visParams, chartTheme, specifiedDimensions); + const theme = getPartitionTheme( + chartType, + visParams, + chartTheme, + specifiedDimensions, + undefined, + true + ); expect(theme).toEqual({ ...getStaticThemeOptions(chartTheme, visParams), @@ -233,7 +240,6 @@ const runPieDonutWaffleTestSuites = (chartType: ChartTypes, visParams: Partition expect(theme).toEqual({ ...getStaticThemeOptions(chartTheme, visParams), - chartPaddings: { top: 500, bottom: 500, left: 500, right: 500 }, partition: { ...getStaticThemePartition(chartTheme, visParams), outerSizeRatio: rescaleFactor, @@ -263,7 +269,6 @@ const runPieDonutWaffleTestSuites = (chartType: ChartTypes, visParams: Partition expect(theme).toEqual({ ...getStaticThemeOptions(chartTheme, vParams), - chartPaddings: { top: 500, bottom: 500, left: 500, right: 500 }, partition: { ...getStaticThemePartition(chartTheme, vParams), outerSizeRatio: 0.5, @@ -285,7 +290,6 @@ const runPieDonutWaffleTestSuites = (chartType: ChartTypes, visParams: Partition expect(theme).toEqual({ ...getStaticThemeOptions(chartTheme, vParams), - chartPaddings: { top: 500, bottom: 500, left: 500, right: 500 }, partition: { ...getStaticThemePartition(chartTheme, vParams), linkLabel: linkLabelWithEnoughSpace(vParams), @@ -420,7 +424,14 @@ const runTreemapMosaicTestSuites = (chartType: ChartTypes, visParams: PartitionV it('should return fullfilled padding settings if dimensions are specified', () => { const specifiedDimensions = { width: 2000, height: 2000 }; - const theme = getPartitionTheme(chartType, visParams, chartTheme, specifiedDimensions); + const theme = getPartitionTheme( + chartType, + visParams, + chartTheme, + specifiedDimensions, + undefined, + true + ); expect(theme).toEqual({ ...getStaticThemeOptions(chartTheme, visParams), diff --git a/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.ts b/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.ts index edb1aaea64aad..3714cac911829 100644 --- a/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.ts +++ b/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_partition_theme.ts @@ -26,7 +26,8 @@ type GetThemeFn = ( visParams: PartitionVisParams, chartTheme: RecursivePartial, dimensions?: PieContainerDimensions, - rescaleFactor?: number + rescaleFactor?: number, + hasOpenedOnAggBasedEditor?: boolean ) => PartialTheme; type GetPieDonutWaffleThemeFn = ( @@ -118,12 +119,13 @@ export const getPartitionTheme: GetThemeFn = ( visParams, chartTheme, dimensions, - rescaleFactor = 1 + rescaleFactor = 1, + hasOpenedOnAggBasedEditor ) => { // On small multiples we want the labels to only appear inside const isSplitChart = Boolean(visParams.dimensions.splitColumn || visParams.dimensions.splitRow); const paddingProps: PartialTheme | null = - dimensions && !isSplitChart + dimensions && !isSplitChart && hasOpenedOnAggBasedEditor ? { chartPaddings: { top: ((1 - Math.min(1, MAX_SIZE / dimensions?.height)) / 2) * dimensions?.height,