[Lens] Move reporting data- attributes from within the renderer to the embeddable (#116950)

* Move logic about data-attrs for reporting to embeddable layer

* Fix test

* Fix CI

* Fix test

* Add onRender method so that correctly handle dispatchComplete

* Update src/plugins/expressions/public/react_expression_renderer.tsx

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
This commit is contained in:
Uladzislau Lasitsa 2021-11-15 19:35:07 +03:00 committed by GitHub
parent 20b63ad5be
commit 28d8e2da27
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 35 additions and 174 deletions

View file

@ -38,6 +38,7 @@ export interface ReactExpressionRendererProps extends IExpressionLoaderParams {
* An observable which can be used to re-run the expression without destroying the component
*/
reload$?: Observable<unknown>;
onRender$?: (item: number) => void;
debounce?: number;
}
@ -66,6 +67,7 @@ export default function ReactExpressionRenderer({
expression,
onEvent,
onData$,
onRender$,
reload$,
debounce,
...expressionLoaderOptions
@ -155,6 +157,7 @@ export default function ReactExpressionRenderer({
...defaultState,
isEmpty: false,
}));
onRender$?.(item);
})
);

View file

@ -3,7 +3,6 @@
exports[`DatatableComponent it renders actions column when there are row actions 1`] = `
<VisualizationContainer
className="lnsDataTableContainer"
reportTitle="My fanci metric chart"
>
<ContextProvider
value={
@ -253,7 +252,6 @@ exports[`DatatableComponent it renders actions column when there are row actions
exports[`DatatableComponent it renders the title and value 1`] = `
<VisualizationContainer
className="lnsDataTableContainer"
reportTitle="My fanci metric chart"
>
<ContextProvider
value={
@ -490,7 +488,6 @@ exports[`DatatableComponent it renders the title and value 1`] = `
exports[`DatatableComponent it should render hide, reset, and sort actions on header even when it is in read only mode 1`] = `
<VisualizationContainer
className="lnsDataTableContainer"
reportTitle="My fanci metric chart"
>
<ContextProvider
value={

View file

@ -322,11 +322,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => {
if (isEmpty) {
return (
<VisualizationContainer
className="lnsDataTableContainer"
reportTitle={props.args.title}
reportDescription={props.args.description}
>
<VisualizationContainer className="lnsDataTableContainer">
<EmptyPlaceholder icon={LensIconChartDatatable} />
</VisualizationContainer>
);
@ -339,11 +335,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => {
});
return (
<VisualizationContainer
className="lnsDataTableContainer"
reportTitle={props.args.title}
reportDescription={props.args.description}
>
<VisualizationContainer className="lnsDataTableContainer">
<DataContext.Provider
value={{
table: firstLocalTable,

View file

@ -364,6 +364,10 @@ export class Embeddable
}
};
private onRender: ExpressionWrapperProps['onRender$'] = () => {
this.renderComplete.dispatchComplete();
};
/**
*
* @param {HTMLElement} domNode
@ -371,6 +375,7 @@ export class Embeddable
*/
render(domNode: HTMLElement | Element) {
this.domNode = domNode;
super.render(domNode as HTMLElement);
if (!this.savedVis || !this.isInitialized || this.isDestroyed) {
return;
}
@ -378,6 +383,10 @@ export class Embeddable
this.input.onLoad(true);
}
this.domNode.setAttribute('data-shared-item', '');
this.renderComplete.dispatchInProgress();
const executionContext = {
type: 'lens',
name: this.savedVis.visualizationType ?? '',
@ -400,6 +409,7 @@ export class Embeddable
searchSessionId={this.externalSearchContext.searchSessionId}
handleEvent={this.handleEvent}
onData$={this.updateActiveData}
onRender$={this.onRender}
interactive={!input.disableTriggers}
renderMode={input.renderMode}
syncColors={input.syncColors}

View file

@ -35,6 +35,7 @@ export interface ExpressionWrapperProps {
data: unknown,
inspectorAdapters?: Partial<DefaultInspectorAdapters> | undefined
) => void;
onRender$: () => void;
renderMode?: RenderMode;
syncColors?: boolean;
hasCompatibleActions?: ReactExpressionRendererProps['hasCompatibleActions'];
@ -106,6 +107,7 @@ export function ExpressionWrapper({
interactive,
searchSessionId,
onData$,
onRender$,
renderMode,
syncColors,
hasCompatibleActions,
@ -132,6 +134,7 @@ export function ExpressionWrapper({
searchContext={searchContext}
searchSessionId={searchSessionId}
onData$={onData$}
onRender$={onRender$}
inspectorAdapters={lensInspector.adapters}
renderMode={renderMode}
syncColors={syncColors}

View file

@ -5,7 +5,7 @@
* 2.0.
*/
import React, { FC, useEffect, useMemo, useState } from 'react';
import React, { FC, useMemo } from 'react';
import {
Chart,
ElementClickListener,
@ -396,23 +396,8 @@ export const HeatmapComponent: FC<HeatmapRenderProps> = ({
const MemoizedChart = React.memo(HeatmapComponent);
export function HeatmapChartReportable(props: HeatmapRenderProps) {
const [state, setState] = useState({
isReady: false,
});
// It takes a cycle for the chart to render. This prevents
// reporting from printing a blank chart placeholder.
useEffect(() => {
setState({ isReady: true });
}, [setState]);
return (
<VisualizationContainer
className="lnsHeatmapExpression__container"
isReady={state.isReady}
reportTitle={props.args.title}
reportDescription={props.args.description}
>
<VisualizationContainer className="lnsHeatmapExpression__container">
<MemoizedChart {...props} />
</VisualizationContainer>
);

View file

@ -87,8 +87,6 @@ describe('metric_expression', () => {
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription="Fancy chart description"
reportTitle="My fanci metric chart"
>
<AutoScale
key="3"
@ -134,8 +132,6 @@ describe('metric_expression', () => {
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription="Fancy chart description"
reportTitle="My fanci metric chart"
>
<AutoScale
key="last"
@ -180,8 +176,6 @@ describe('metric_expression', () => {
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription=""
reportTitle=""
>
<AutoScale
key="3"
@ -226,8 +220,6 @@ describe('metric_expression', () => {
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription=""
reportTitle=""
>
<AutoScale
key="3"
@ -262,8 +254,6 @@ describe('metric_expression', () => {
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription=""
reportTitle=""
>
<EmptyPlaceholder
icon={[Function]}
@ -288,8 +278,6 @@ describe('metric_expression', () => {
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription=""
reportTitle=""
>
<EmptyPlaceholder
icon={[Function]}
@ -314,8 +302,6 @@ describe('metric_expression', () => {
).toMatchInlineSnapshot(`
<VisualizationContainer
className="lnsMetricExpression__container"
reportDescription=""
reportTitle=""
>
<AutoScale
key="0"

View file

@ -48,15 +48,11 @@ export function MetricChart({
args,
formatFactory,
}: MetricChartProps & { formatFactory: FormatFactory }) {
const { metricTitle, title, description, accessor, mode } = args;
const { metricTitle, accessor, mode } = args;
const firstTable = Object.values(data.tables)[0];
const getEmptyState = () => (
<VisualizationContainer
reportTitle={title}
reportDescription={description}
className="lnsMetricExpression__container"
>
<VisualizationContainer className="lnsMetricExpression__container">
<EmptyPlaceholder icon={LensIconChartMetric} />
</VisualizationContainer>
);
@ -84,11 +80,7 @@ export function MetricChart({
: Number(Number(row[accessor]).toFixed(3)).toString();
return (
<VisualizationContainer
reportTitle={title}
reportDescription={description}
className="lnsMetricExpression__container"
>
<VisualizationContainer className="lnsMetricExpression__container">
<AutoScale key={value}>
<div data-test-subj="lns_metric_value" style={{ fontSize: '60pt', fontWeight: 600 }}>
{value}

View file

@ -6,7 +6,7 @@
*/
import { uniq } from 'lodash';
import React, { useEffect, useState } from 'react';
import React from 'react';
import { FormattedMessage } from '@kbn/i18n/react';
import { EuiText } from '@elastic/eui';
import {
@ -224,13 +224,6 @@ export function PieComponent(
},
});
const [isReady, setIsReady] = useState(false);
// It takes a cycle for the chart to render. This prevents
// reporting from printing a blank chart placeholder.
useEffect(() => {
setIsReady(true);
}, []);
const hasNegative = firstTable.rows.some((row) => {
const value = row[metricColumn.id];
return typeof value === 'number' && value < 0;
@ -247,11 +240,7 @@ export function PieComponent(
if (isEmpty) {
return (
<VisualizationContainer
reportTitle={props.args.title}
reportDescription={props.args.description}
className="lnsPieExpression__container"
>
<VisualizationContainer className="lnsPieExpression__container">
<EmptyPlaceholder icon={LensIconChartDonut} />
</VisualizationContainer>
);
@ -278,12 +267,7 @@ export function PieComponent(
};
return (
<VisualizationContainer
reportTitle={props.args.title}
reportDescription={props.args.description}
className="lnsPieExpression__container"
isReady={isReady}
>
<VisualizationContainer className="lnsPieExpression__container">
<Chart>
<Settings
tooltip={{ boundary: document.getElementById('app-fixed-viewport') ?? undefined }}

View file

@ -10,79 +10,27 @@ import { mount } from 'enzyme';
import { VisualizationContainer } from './visualization_container';
describe('VisualizationContainer', () => {
test('renders reporting data attributes when ready', () => {
const component = mount(<VisualizationContainer isReady={true}>Hello!</VisualizationContainer>);
const reportingEl = component.find('[data-shared-item]').first();
expect(reportingEl.prop('data-render-complete')).toBeTruthy();
expect(reportingEl.prop('data-shared-item')).toBeTruthy();
});
test('does not render data attributes when not ready', () => {
const component = mount(
<VisualizationContainer isReady={false}>Hello!</VisualizationContainer>
);
const reportingEl = component.find('[data-shared-item]').first();
expect(reportingEl.prop('data-render-complete')).toBeFalsy();
expect(reportingEl.prop('data-shared-item')).toBeTruthy();
});
test('increments counter in data attribute for each render', () => {
const component = mount(<VisualizationContainer isReady={true}>Hello!</VisualizationContainer>);
let reportingEl = component.find('[data-shared-item]').first();
expect(reportingEl.prop('data-rendering-count')).toEqual(1);
component.setProps({ children: 'Hello2!' });
reportingEl = component.find('[data-shared-item]').first();
expect(reportingEl.prop('data-rendering-count')).toEqual(2);
});
test('renders child content', () => {
const component = mount(
<VisualizationContainer isReady={false}>Hello!</VisualizationContainer>
);
const component = mount(<VisualizationContainer>Hello!</VisualizationContainer>);
expect(component.text()).toEqual('Hello!');
});
test('defaults to rendered', () => {
const component = mount(<VisualizationContainer>Hello!</VisualizationContainer>);
const reportingEl = component.find('[data-shared-item]').first();
expect(reportingEl.prop('data-render-complete')).toBeTruthy();
expect(reportingEl.prop('data-shared-item')).toBeTruthy();
});
test('renders title and description for reporting, if provided', () => {
const component = mount(
<VisualizationContainer reportTitle="shazam!" reportDescription="Description">
Hello!
</VisualizationContainer>
);
const reportingEl = component.find('[data-shared-item]').first();
expect(reportingEl.prop('data-title')).toEqual('shazam!');
expect(reportingEl.prop('data-description')).toEqual('Description');
});
test('renders style', () => {
const component = mount(
<VisualizationContainer style={{ color: 'blue' }}>Hello!</VisualizationContainer>
);
const reportingEl = component.find('[data-shared-item]').first();
const el = component.find('.lnsVisualizationContainer').first();
expect(reportingEl.prop('style')).toEqual({ color: 'blue' });
expect(el.prop('style')).toEqual({ color: 'blue' });
});
test('combines class names with container class', () => {
const component = mount(
<VisualizationContainer className="myClass">Hello!</VisualizationContainer>
);
const reportingEl = component.find('[data-shared-item]').first();
const el = component.find('.lnsVisualizationContainer').first();
expect(reportingEl.prop('className')).toEqual('myClass lnsVisualizationContainer');
expect(el.prop('className')).toEqual('myClass lnsVisualizationContainer');
});
});

View file

@ -7,44 +7,18 @@
import './visualization_container.scss';
import React, { useRef } from 'react';
import React from 'react';
import classNames from 'classnames';
interface Props extends React.HTMLAttributes<HTMLDivElement> {
isReady?: boolean;
reportTitle?: string;
reportDescription?: string;
}
/**
* This is a convenience component that wraps rendered Lens visualizations. It adds reporting
* attributes (data-shared-item, data-render-complete, and data-title).
*/
export function VisualizationContainer({
isReady = true,
reportTitle,
reportDescription,
children,
className,
...rest
}: Props) {
const counterRef = useRef(0);
counterRef.current++;
const attributes: Partial<{ 'data-title': string; 'data-description': string }> = {};
if (reportTitle) {
attributes['data-title'] = reportTitle;
}
if (reportDescription) {
attributes['data-description'] = reportDescription;
}
}: React.HTMLAttributes<HTMLDivElement>) {
return (
<div
data-shared-item
data-render-complete={isReady}
data-rendering-count={counterRef.current}
data-test-subj="lnsVisualizationContainer"
className={classNames(className, 'lnsVisualizationContainer')}
{...attributes}
{...rest}
>
{children}

View file

@ -7,7 +7,7 @@
import './expression.scss';
import React, { useState, useEffect, useRef } from 'react';
import React, { useRef } from 'react';
import ReactDOM from 'react-dom';
import {
Chart,
@ -207,21 +207,8 @@ function getIconForSeriesType(seriesType: SeriesType): IconType {
const MemoizedChart = React.memo(XYChart);
export function XYChartReportable(props: XYChartRenderProps) {
const [isReady, setIsReady] = useState(false);
// It takes a cycle for the XY chart to render. This prevents
// reporting from printing a blank chart placeholder.
useEffect(() => {
setIsReady(true);
}, [setIsReady]);
return (
<VisualizationContainer
className="lnsXyExpression__container"
isReady={isReady}
reportTitle={props.args.title}
reportDescription={props.args.description}
>
<VisualizationContainer className="lnsXyExpression__container">
<MemoizedChart {...props} />
</VisualizationContainer>
);