Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Fix embeddable title and description for reporting and dashboard tooltip #78767

Merged
merged 10 commits into from
Oct 1, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,10 @@ function renderTooltip(description: string) {
);
}

const VISUALIZE_EMBEDDABLE_TYPE = 'visualization';
type VisualizeEmbeddable = any;
Copy link
Contributor

@ThomThomson ThomThomson Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this wasn't added in this PR, but I'm not a huge fan of using any like this. Now that this description is added to both lens and visualize, I wonder if it would be worth adding an optional getDescription function to IEmbeddable.

If not, at least the any could be removed with something like:

type EmbeddableWithDescription = IEmbeddable & { getDescription: () => string };

function getViewDescription(embeddable: IEmbeddable) {
  return (embeddable as EmbeddableWithDescription).getDescription?.() || '';
}


function getViewDescription(embeddable: IEmbeddable | VisualizeEmbeddable) {
if (embeddable.type === VISUALIZE_EMBEDDABLE_TYPE) {
if (embeddable.getVisualizationDescription) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be embeddable.getDescription instead? Not all embeddables are visualizations

const description = embeddable.getVisualizationDescription();

if (description) {
Expand Down

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 @@ -33,6 +33,7 @@ export interface DatatableColumns {

interface Args {
title: string;
description?: string;
columns: DatatableColumns & { type: 'lens_datatable_columns' };
}

Expand Down Expand Up @@ -72,6 +73,10 @@ export const datatable: ExpressionFunctionDefinition<
defaultMessage: 'Title',
}),
},
description: {
types: ['string'],
help: '',
},
columns: {
types: ['lens_datatable_columns'],
help: '',
Expand Down Expand Up @@ -207,7 +212,10 @@ export function DatatableComponent(props: DatatableRenderProps) {
}

return (
<VisualizationContainer>
<VisualizationContainer
reportTitle={props.args.title}
reportDescription={props.args.description}
>
<EuiBasicTable
className="lnsDataTable"
data-test-subj="lnsDataTable"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
};
},

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);
Expand All @@ -211,6 +211,8 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
type: 'function',
function: 'lens_datatable',
arguments: {
title: [title || ''],
description: [description || ''],
columns: [
{
type: 'expression',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ export function buildExpression({
datasourceMap,
datasourceStates,
datasourceLayers,
title,
description,
}: {
title?: string;
description?: string;
visualization: Visualization | null;
visualizationState: unknown;
datasourceMap: Record<string, Datasource>;
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!];
Expand All @@ -78,6 +80,8 @@ export async function persistedStateToExpression(
const datasourceLayers = createDatasourceLayers(datasources, datasourceStates);

return buildExpression({
title,
description,
visualization,
visualizationState,
datasourceMap: datasources,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,12 @@ export class Embeddable
return this.deps.attributeService.getInputAsValueType(input);
};

// same API as Visualize
public getVisualizationDescription() {
// mind that savedViz is loaded in async way here
return this.savedVis && this.savedVis.description;
}

destroy() {
super.destroy();
if (this.domNode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -53,14 +64,15 @@ 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(
shallow(<MetricChart data={data} args={args} formatFactory={(x) => x as IFieldFormat} />)
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription="Fancy chart description"
reportTitle="My fanci metric chart"
>
<AutoScale>
Expand Down Expand Up @@ -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(
<MetricChart
data={data}
args={{ ...args, mode: 'reduced' }}
args={noAttributesArgs}
formatFactory={(x) => x as IFieldFormat}
/>
)
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportTitle="My fanci metric chart"
reportDescription=""
reportTitle=""
>
<AutoScale>
<div
data-test-subj="lns_metric_value"
style={
Object {
"fontSize": "60pt",
"fontWeight": 600,
}
}
>
10110
</div>
<div
data-test-subj="lns_metric_title"
style={
Object {
"fontSize": "24pt",
}
}
>
My fanci metric chart
</div>
</AutoScale>
</VisualizationContainer>
`);
});

test('it does not render metricTitle in reduced mode', () => {
const { data, noAttributesArgs } = sampleArgs();

expect(
shallow(
<MetricChart
data={data}
args={{ ...noAttributesArgs, mode: 'reduced' }}
formatFactory={(x) => x as IFieldFormat}
/>
)
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription=""
reportTitle=""
>
<AutoScale>
<div
Expand Down
24 changes: 20 additions & 4 deletions x-pack/plugins/lens/public/metric_visualization/expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ export const metricChart: ExpressionFunctionDefinition<
types: ['string'],
help: 'The chart title.',
},
description: {
types: ['string'],
help: '',
},
metricTitle: {
types: ['string'],
help: 'The title of the metric shown.',
},
accessor: {
types: ['string'],
help: 'The column whose value is being displayed',
Expand Down Expand Up @@ -98,12 +106,16 @@ export function MetricChart({
args,
formatFactory,
}: MetricChartProps & { formatFactory: FormatFactory }) {
const { title, accessor, mode } = args;
const { metricTitle, title, description, accessor, mode } = args;
let value = '-';
const firstTable = Object.values(data.tables)[0];
if (!accessor) {
return (
<VisualizationContainer reportTitle={title} className="lnsMetricExpression__container" />
<VisualizationContainer
reportTitle={title}
reportDescription={description}
className="lnsMetricExpression__container"
/>
);
}

Expand All @@ -119,14 +131,18 @@ export function MetricChart({
}

return (
<VisualizationContainer reportTitle={title} className="lnsMetricExpression__container">
<VisualizationContainer
reportTitle={title}
reportDescription={description}
className="lnsMetricExpression__container"
>
<AutoScale>
<div data-test-subj="lns_metric_value" style={{ fontSize: '60pt', fontWeight: 600 }}>
{value}
</div>
{mode === 'full' && (
<div data-test-subj="lns_metric_title" style={{ fontSize: '24pt' }}>
{title}
{metricTitle}
</div>
)}
</AutoScale>
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/lens/public/metric_visualization/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ export interface State {

export interface MetricConfig extends State {
title: string;
description: string;
metricTitle: string;
mode: 'reduced' | 'full';
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { State } from './types';
const toExpression = (
state: State,
datasourceLayers: Record<string, DatasourcePublicAPI>,
mode: 'reduced' | 'full' = 'full'
attributes?: { mode?: 'reduced' | 'full'; title?: string; description?: string }
): Ast | null => {
if (!state.accessor) {
return null;
Expand All @@ -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'],
},
},
],
Expand Down Expand Up @@ -104,7 +106,7 @@ export const metricVisualization: Visualization<State> = {

toExpression,
toPreviewExpression: (state, datasourceLayers) =>
toExpression(state, datasourceLayers, 'reduced'),
toExpression(state, datasourceLayers, { mode: 'reduced' }),

setDimension({ prevState, columnId }) {
return { ...prevState, accessor: columnId };
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/lens/public/pie_visualization/expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,12 @@ export function PieComponent(
);
}
return (
<VisualizationContainer className="lnsPieExpression__container" isReady={state.isReady}>
<VisualizationContainer
reportTitle={props.args.title}
reportDescription={props.args.description}
className="lnsPieExpression__container"
isReady={state.isReady}
>
<Chart>
<Settings
// Legend is hidden in many scenarios
Expand Down
Loading