Skip to content

Commit

Permalink
Modularize render complete (elastic#74504) (elastic#75546)
Browse files Browse the repository at this point in the history
* chore: 🤖 remove unused render-complete logic

* feat: 🎸 add render-complete observables to IEmbeddable

* Revert "chore: 🤖 remove unused render-complete logic"

This reverts commit 0049c01.

* refactor: 💡 rename render complete "helper" to "listener"

* feat: 🎸 add render complete dispatcher to embeddable

* refactor: 💡 move data-title setup to Embeddable

* refactor: 💡 move embeddable data-title setting to render-compl

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	src/plugins/visualizations/public/embeddable/visualize_embeddable.ts
  • Loading branch information
streamich authored Aug 20, 2020
1 parent 932e0b5 commit eeba9c7
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
* under the License.
*/
import { IScope } from 'angular';
import { RenderCompleteHelper } from '../../../../../kibana_utils/public';
import { RenderCompleteListener } from '../../../../../kibana_utils/public';

export function createRenderCompleteDirective() {
return {
controller($scope: IScope, $element: JQLite) {
const el = $element[0];
const renderCompleteHelper = new RenderCompleteHelper(el);
$scope.$on('$destroy', renderCompleteHelper.destroy);
const renderCompleteListener = new RenderCompleteListener(el);
$scope.$on('$destroy', renderCompleteListener.destroy);
},
};
}
3 changes: 2 additions & 1 deletion src/plugins/embeddable/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
],
"requiredBundles": [
"savedObjects",
"kibanaReact"
"kibanaReact",
"kibanaUtils"
]
}
22 changes: 19 additions & 3 deletions src/plugins/embeddable/public/lib/embeddables/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import { cloneDeep, isEqual } from 'lodash';
import * as Rx from 'rxjs';
import { distinctUntilChanged, map } from 'rxjs/operators';
import { RenderCompleteDispatcher } from '../../../../kibana_utils/public';
import { Adapters, ViewMode } from '../types';
import { IContainer } from '../containers';
import { EmbeddableInput, EmbeddableOutput, IEmbeddable } from './i_embeddable';
Expand Down Expand Up @@ -47,6 +49,8 @@ export abstract class Embeddable<
private readonly input$: Rx.BehaviorSubject<TEmbeddableInput>;
private readonly output$: Rx.BehaviorSubject<TEmbeddableOutput>;

protected renderComplete = new RenderCompleteDispatcher();

// Listener to parent changes, if this embeddable exists in a parent, in order
// to update input when the parent changes.
private parentSubscription?: Rx.Subscription;
Expand Down Expand Up @@ -77,6 +81,15 @@ export abstract class Embeddable<
this.onResetInput(newInput);
});
}

this.getOutput$()
.pipe(
map(({ title }) => title || ''),
distinctUntilChanged()
)
.subscribe((title) => {
this.renderComplete.setTitle(title);
});
}

public getIsContainer(): this is IContainer {
Expand Down Expand Up @@ -105,8 +118,8 @@ export abstract class Embeddable<
return this.input;
}

public getTitle() {
return this.output.title;
public getTitle(): string {
return this.output.title || '';
}

/**
Expand All @@ -133,7 +146,10 @@ export abstract class Embeddable<
}
}

public render(domNode: HTMLElement | Element): void {
public render(el: HTMLElement): void {
this.renderComplete.setEl(el);
this.renderComplete.setTitle(this.output.title || '');

if (this.destroyed) {
throw new Error('Embeddable has been destroyed');
}
Expand Down
18 changes: 2 additions & 16 deletions src/plugins/kibana_utils/public/render_complete/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,5 @@
* under the License.
*/

const dispatchCustomEvent = (el: HTMLElement, eventName: string) => {
// we're using the native events so that we aren't tied to the jQuery custom events,
// otherwise we have to use jQuery(element).on(...) because jQuery's events sit on top
// of the native events per https://github.com/jquery/jquery/issues/2476
el.dispatchEvent(new CustomEvent(eventName, { bubbles: true }));
};

export function dispatchRenderComplete(el: HTMLElement) {
dispatchCustomEvent(el, 'renderComplete');
}

export function dispatchRenderStart(el: HTMLElement) {
dispatchCustomEvent(el, 'renderStart');
}

export * from './render_complete_helper';
export * from './render_complete_listener';
export * from './render_complete_dispatcher';
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

const dispatchEvent = (el: HTMLElement, eventName: string) => {
el.dispatchEvent(new CustomEvent(eventName, { bubbles: true }));
};

export function dispatchRenderComplete(el: HTMLElement) {
dispatchEvent(el, 'renderComplete');
}

export function dispatchRenderStart(el: HTMLElement) {
dispatchEvent(el, 'renderStart');
}

/**
* Should call `dispatchComplete()` when UI block has finished loading its data and has
* completely rendered. Should `dispatchInProgress()` every time UI block
* starts loading data again. At the start it is assumed that UI block is loading
* so it dispatches "in progress" automatically, so you need to call `setRenderComplete`
* at least once.
*
* This is used for reporting to know that UI block is ready, so
* it can take a screenshot. It is also used in functional tests to know that
* page has stabilized.
*/
export class RenderCompleteDispatcher {
private count: number = 0;
private el?: HTMLElement;

constructor(el?: HTMLElement) {
this.setEl(el);
}

public setEl(el?: HTMLElement) {
this.el = el;
this.count = 0;
if (el) this.dispatchInProgress();
}

public dispatchInProgress() {
if (!this.el) return;
this.el.setAttribute('data-render-complete', 'false');
this.el.setAttribute('data-rendering-count', String(this.count));
dispatchRenderStart(this.el);
}

public dispatchComplete() {
if (!this.el) return;
this.count++;
this.el.setAttribute('data-render-complete', 'true');
this.el.setAttribute('data-rendering-count', String(this.count));
dispatchRenderComplete(this.el);
}

public dispatchError() {
if (!this.el) return;
this.count++;
this.el.setAttribute('data-render-complete', 'false');
this.el.setAttribute('data-rendering-count', String(this.count));
}

public setTitle(title: string) {
if (!this.el) return;
this.el.setAttribute('data-title', title);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
* under the License.
*/

const attributeName = 'data-render-complete';
export class RenderCompleteListener {
private readonly attributeName = 'data-render-complete';

export class RenderCompleteHelper {
constructor(private readonly element: HTMLElement) {
this.setup();
}
Expand All @@ -30,23 +30,23 @@ export class RenderCompleteHelper {
};

public setup = () => {
this.element.setAttribute(attributeName, 'false');
this.element.setAttribute(this.attributeName, 'false');
this.element.addEventListener('renderStart', this.start);
this.element.addEventListener('renderComplete', this.complete);
};

public disable = () => {
this.element.setAttribute(attributeName, 'disabled');
this.element.setAttribute(this.attributeName, 'disabled');
this.destroy();
};

private start = () => {
this.element.setAttribute(attributeName, 'false');
this.element.setAttribute(this.attributeName, 'false');
return true;
};

private complete = () => {
this.element.setAttribute(attributeName, 'true');
this.element.setAttribute(this.attributeName, 'true');
return true;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import {
IContainer,
Adapters,
} from '../../../../plugins/embeddable/public';
import { dispatchRenderComplete } from '../../../../plugins/kibana_utils/public';
import {
IExpressionLoaderParams,
ExpressionsStart,
Expand Down Expand Up @@ -85,7 +84,6 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
private timefilter: TimefilterContract;
private timeRange?: TimeRange;
private query?: Query;
private title?: string;
private filters?: Filter[];
private visCustomizations?: Pick<VisualizeInput, 'vis' | 'table'>;
private subscriptions: Subscription[] = [];
Expand Down Expand Up @@ -158,7 +156,7 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
if (!adapters) return;

return this.deps.start().plugins.inspector.open(adapters, {
title: this.getTitle() || '',
title: this.getTitle(),
});
};

Expand Down Expand Up @@ -216,11 +214,10 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
dirty = true;
}

if (this.output.title !== this.title) {
this.title = this.output.title;
if (this.domNode) {
this.domNode.setAttribute('data-title', this.title || '');
}
// propagate the title to the output embeddable
// but only when the visualization is in edit/Visualize mode
if (!this.parent && this.vis.title !== this.output.title) {
this.updateOutput({ title: this.vis.title });
}

if (this.vis.description && this.domNode) {
Expand All @@ -237,26 +234,20 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
hasInspector = () => Boolean(this.getInspectorAdapters());

onContainerLoading = () => {
this.domNode.setAttribute('data-render-complete', 'false');
this.renderComplete.dispatchInProgress();
this.updateOutput({ loading: true, error: undefined });
};

onContainerRender = (count: number) => {
this.domNode.setAttribute('data-render-complete', 'true');
this.domNode.setAttribute('data-rendering-count', count.toString());
onContainerRender = () => {
this.renderComplete.dispatchComplete();
this.updateOutput({ loading: false, error: undefined });
dispatchRenderComplete(this.domNode);
};

onContainerError = (error: ExpressionRenderError) => {
if (this.abortController) {
this.abortController.abort();
}
this.domNode.setAttribute(
'data-rendering-count',
this.domNode.getAttribute('data-rendering-count') + 1
);
this.domNode.setAttribute('data-render-complete', 'false');
this.renderComplete.dispatchError();
this.updateOutput({ loading: false, error });
};

Expand All @@ -265,7 +256,6 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
* @param {Element} domNode
*/
public async render(domNode: HTMLElement) {
super.render(domNode);
this.timeRange = _.cloneDeep(this.input.timeRange);

this.transferCustomizationsToUiState();
Expand All @@ -275,6 +265,7 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
domNode.appendChild(div);

this.domNode = div;
super.render(this.domNode);

const expressions = getExpressions();
this.handler = new expressions.ExpressionLoader(this.domNode, undefined, {
Expand Down Expand Up @@ -323,19 +314,14 @@ export class VisualizeEmbeddable extends Embeddable<VisualizeInput, VisualizeOut
})
);

div.setAttribute('data-title', this.output.title || '');

if (this.vis.description) {
div.setAttribute('data-description', this.vis.description);
}

div.setAttribute('data-test-subj', 'visualizationLoader');
div.setAttribute('data-shared-item', '');
div.setAttribute('data-rendering-count', '0');
div.setAttribute('data-render-complete', 'false');

this.subscriptions.push(this.handler.loading$.subscribe(this.onContainerLoading));

this.subscriptions.push(this.handler.render$.subscribe(this.onContainerRender));

this.updateHandler();
Expand Down

0 comments on commit eeba9c7

Please sign in to comment.