From 198c5d998816e940e4c8d92979504c86116bcd2d Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Thu, 1 Oct 2020 18:02:37 +0200 Subject: [PATCH] [Lens] Fix embeddable title and description for reporting and dashboard tooltip (#78767) Co-authored-by: Elastic Machine --- .../lib/panel/panel_header/panel_header.tsx | 9 ++- .../public/embeddable/visualize_embeddable.ts | 2 +- .../__snapshots__/expression.test.tsx.snap | 4 +- .../datatable_visualization/expression.tsx | 10 ++- .../datatable_visualization/visualization.tsx | 4 +- .../editor_frame/expression_helpers.ts | 9 ++- .../editor_frame/state_helpers.ts | 4 ++ .../embeddable/embeddable.tsx | 6 ++ .../metric_visualization/expression.test.tsx | 69 +++++++++++++++++-- .../metric_visualization/expression.tsx | 24 +++++-- .../lens/public/metric_visualization/types.ts | 2 + .../visualization.test.ts | 8 ++- .../metric_visualization/visualization.tsx | 10 +-- .../public/pie_visualization/expression.tsx | 8 +++ .../pie_visualization/render_function.tsx | 7 +- .../public/pie_visualization/to_expression.ts | 13 ++-- .../lens/public/pie_visualization/types.ts | 2 + x-pack/plugins/lens/public/types.ts | 3 +- .../public/visualization_container.test.tsx | 7 +- .../lens/public/visualization_container.tsx | 11 ++- .../__snapshots__/to_expression.test.ts.snap | 6 ++ .../public/xy_visualization/expression.tsx | 15 +++- .../public/xy_visualization/to_expression.ts | 10 ++- .../lens/public/xy_visualization/types.ts | 2 + 24 files changed, 206 insertions(+), 39 deletions(-) diff --git a/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx b/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx index 7dde84e510535..dea483efb349f 100644 --- a/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx +++ b/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx @@ -109,12 +109,11 @@ function renderTooltip(description: string) { ); } -const VISUALIZE_EMBEDDABLE_TYPE = 'visualization'; -type VisualizeEmbeddable = any; +type EmbeddableWithDescription = IEmbeddable & { getDescription: () => string }; -function getViewDescription(embeddable: IEmbeddable | VisualizeEmbeddable) { - if (embeddable.type === VISUALIZE_EMBEDDABLE_TYPE) { - const description = embeddable.getVisualizationDescription(); +function getViewDescription(embeddable: IEmbeddable | EmbeddableWithDescription) { + if ('getDescription' in embeddable) { + const description = embeddable.getDescription(); if (description) { return description; diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts index c091d396b4924..fe8a9adff4052 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts +++ b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts @@ -167,7 +167,7 @@ export class VisualizeEmbeddable typeof inspectorAdapters === 'function' ? inspectorAdapters() : inspectorAdapters; } } - public getVisualizationDescription() { + public getDescription() { return this.vis.description; } diff --git a/x-pack/plugins/lens/public/datatable_visualization/__snapshots__/expression.test.tsx.snap b/x-pack/plugins/lens/public/datatable_visualization/__snapshots__/expression.test.tsx.snap index c0210c3915ce8..9c7bdc3397f9c 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/__snapshots__/expression.test.tsx.snap +++ b/x-pack/plugins/lens/public/datatable_visualization/__snapshots__/expression.test.tsx.snap @@ -1,7 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`datatable_expression DatatableComponent it renders the title and value 1`] = ` - + + }; }, - toExpression(state, datasourceLayers): Ast { + toExpression(state, datasourceLayers, { title, description } = {}): Ast { const layer = state.layers[0]; const datasource = datasourceLayers[layer.layerId]; const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId); @@ -211,6 +211,8 @@ export const datatableVisualization: Visualization type: 'function', function: 'lens_datatable', arguments: { + title: [title || ''], + description: [description || ''], columns: [ { type: 'expression', diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts index 952718e13c8cf..e7568147dc568 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts @@ -73,7 +73,11 @@ export function buildExpression({ datasourceMap, datasourceStates, datasourceLayers, + title, + description, }: { + title?: string; + description?: string; visualization: Visualization | null; visualizationState: unknown; datasourceMap: Record; @@ -89,7 +93,10 @@ export function buildExpression({ if (visualization === null) { return null; } - const visualizationExpression = visualization.toExpression(visualizationState, datasourceLayers); + const visualizationExpression = visualization.toExpression(visualizationState, datasourceLayers, { + title, + description, + }); const completeExpression = prependDatasourceExpression( visualizationExpression, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts index 6deb9ffd37a06..1fe5224d0b1b4 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts @@ -61,6 +61,8 @@ export async function persistedStateToExpression( state: { visualization: visualizationState, datasourceStates: persistedDatasourceStates }, visualizationType, references, + title, + description, } = doc; if (!visualizationType) return null; const visualization = visualizations[visualizationType!]; @@ -78,6 +80,8 @@ export async function persistedStateToExpression( const datasourceLayers = createDatasourceLayers(datasources, datasourceStates); return buildExpression({ + title, + description, visualization, visualizationState, datasourceMap: datasources, diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 61a5d8cacdc4f..16b19ca0af849 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -295,6 +295,12 @@ export class Embeddable return this.deps.attributeService.getInputAsValueType(input); }; + // same API as Visualize + public getDescription() { + // mind that savedViz is loaded in async way here + return this.savedVis && this.savedVis.description; + } + destroy() { super.destroy(); if (this.domNode) { diff --git a/x-pack/plugins/lens/public/metric_visualization/expression.test.tsx b/x-pack/plugins/lens/public/metric_visualization/expression.test.tsx index 0c92cdb2c31fc..77a8ce64b21a2 100644 --- a/x-pack/plugins/lens/public/metric_visualization/expression.test.tsx +++ b/x-pack/plugins/lens/public/metric_visualization/expression.test.tsx @@ -32,10 +32,21 @@ function sampleArgs() { accessor: 'a', layerId: 'l1', title: 'My fanci metric chart', + description: 'Fancy chart description', + metricTitle: 'My fanci metric chart', mode: 'full', }; - return { data, args }; + const noAttributesArgs: MetricConfig = { + accessor: 'a', + layerId: 'l1', + title: '', + description: '', + metricTitle: 'My fanci metric chart', + mode: 'full', + }; + + return { data, args, noAttributesArgs }; } describe('metric_expression', () => { @@ -53,7 +64,7 @@ describe('metric_expression', () => { }); describe('MetricChart component', () => { - test('it renders the title and value', () => { + test('it renders the all attributes when passed (title, description, metricTitle, value)', () => { const { data, args } = sampleArgs(); expect( @@ -61,6 +72,7 @@ describe('metric_expression', () => { ).toMatchInlineSnapshot(` @@ -90,21 +102,66 @@ describe('metric_expression', () => { `); }); - test('it does not render title in reduced mode', () => { - const { data, args } = sampleArgs(); + test('it renders only chart content when title and description are empty strings', () => { + const { data, noAttributesArgs } = sampleArgs(); expect( shallow( x as IFieldFormat} /> ) ).toMatchInlineSnapshot(` + +
+ 10110 +
+
+ My fanci metric chart +
+
+
+ `); + }); + + test('it does not render metricTitle in reduced mode', () => { + const { data, noAttributesArgs } = sampleArgs(); + + expect( + shallow( + x as IFieldFormat} + /> + ) + ).toMatchInlineSnapshot(` +
+ ); } @@ -119,14 +131,18 @@ export function MetricChart({ } return ( - +
{value}
{mode === 'full' && (
- {title} + {metricTitle}
)}
diff --git a/x-pack/plugins/lens/public/metric_visualization/types.ts b/x-pack/plugins/lens/public/metric_visualization/types.ts index 86a781716b345..c4a3fd094abe6 100644 --- a/x-pack/plugins/lens/public/metric_visualization/types.ts +++ b/x-pack/plugins/lens/public/metric_visualization/types.ts @@ -11,5 +11,7 @@ export interface State { export interface MetricConfig extends State { title: string; + description: string; + metricTitle: string; mode: 'reduced' | 'full'; } diff --git a/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts b/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts index aa3de93013e66..80c7a174b3264 100644 --- a/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts @@ -171,11 +171,17 @@ describe('metric_visualization', () => { "accessor": Array [ "a", ], + "description": Array [ + "", + ], + "metricTitle": Array [ + "shazm", + ], "mode": Array [ "full", ], "title": Array [ - "shazm", + "", ], }, "function": "lens_metric_chart", diff --git a/x-pack/plugins/lens/public/metric_visualization/visualization.tsx b/x-pack/plugins/lens/public/metric_visualization/visualization.tsx index 72c07bed1acb2..77d189ce53d01 100644 --- a/x-pack/plugins/lens/public/metric_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/metric_visualization/visualization.tsx @@ -14,7 +14,7 @@ import { State } from './types'; const toExpression = ( state: State, datasourceLayers: Record, - mode: 'reduced' | 'full' = 'full' + attributes?: { mode?: 'reduced' | 'full'; title?: string; description?: string } ): Ast | null => { if (!state.accessor) { return null; @@ -30,9 +30,11 @@ const toExpression = ( type: 'function', function: 'lens_metric_chart', arguments: { - title: [(operation && operation.label) || ''], + title: [attributes?.title || ''], + description: [attributes?.description || ''], + metricTitle: [(operation && operation.label) || ''], accessor: [state.accessor], - mode: [mode], + mode: [attributes?.mode || 'full'], }, }, ], @@ -104,7 +106,7 @@ export const metricVisualization: Visualization = { toExpression, toPreviewExpression: (state, datasourceLayers) => - toExpression(state, datasourceLayers, 'reduced'), + toExpression(state, datasourceLayers, { mode: 'reduced' }), setDimension({ prevState, columnId }) { return { ...prevState, accessor: columnId }; diff --git a/x-pack/plugins/lens/public/pie_visualization/expression.tsx b/x-pack/plugins/lens/public/pie_visualization/expression.tsx index 89d93ab79233f..d93145f29aa89 100644 --- a/x-pack/plugins/lens/public/pie_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/expression.tsx @@ -37,6 +37,14 @@ export const pie: ExpressionFunctionDefinition< defaultMessage: 'Pie renderer', }), args: { + title: { + types: ['string'], + help: 'The chart title.', + }, + description: { + types: ['string'], + help: '', + }, groups: { types: ['string'], multi: true, diff --git a/x-pack/plugins/lens/public/pie_visualization/render_function.tsx b/x-pack/plugins/lens/public/pie_visualization/render_function.tsx index d97ab146e000d..8de810f9aa5d3 100644 --- a/x-pack/plugins/lens/public/pie_visualization/render_function.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/render_function.tsx @@ -228,7 +228,12 @@ export function PieComponent( ); } return ( - +