Timelion visualization renderer #78540

sulemanof · 2020-09-25T15:50:40Z

Summary

Part of #46801

Implement own renderer for Timelion visualization;
lazy load the timelion visualization component and styles;
Implement toExpressionAst function for building pipeline;
cleanup the code, remove extra wrappers;
add unit tests;

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

…derer

ppisljar

code LGTM for toASt/render parts, didn't check changes to timelion

elasticmachine · 2020-09-29T13:20:35Z

Pinging @elastic/kibana-app (Team:KibanaApp)

alexwizp · 2020-09-29T17:38:28Z

src/plugins/vis_type_timelion/public/timelion_vis_renderer.tsx

+  deps: TimelionVisDependencies
+) => ExpressionRenderDefinition<TimelionRenderValue> = (deps) => ({
+  name: 'timelion_vis',
+  displayName: 'Timelion visualization',


should this value be localized?

This is a good question.
Despite the fact that docs says:

/** * A user friendly name of the renderer as will be displayed to user in UI. */ displayName: string;

I can't find any usage of the displayName in src/plugins, so this never affects our basic plugins.
But this could be an option for canvas app.
I had a talk with @streamich around the ExpressionRenderer keys, and he'll define their necessity after their team meeting today.
For now, we decided to make it an optional property.

src/plugins/vis_type_timelion/public/timelion_vis_renderer.tsx

src/plugins/vis_type_timelion/public/components/timelion_vis_component.tsx

src/plugins/vis_type_timelion/public/timelion_vis_renderer.tsx

alexwizp

Overall LGTM! Just added some questions

…derer

stratoula

LGTM 🥳 Thanx @sulemanof ❤️

cchaos

I'm slightly confused about what's goin on with these imports, but as it's a deprecated plugin, I'm not going to worry too much.

sulemanof · 2020-10-01T07:54:06Z

I'm slightly confused about what's goin on with these imports, but as it's a deprecated plugin, I'm not going to worry too much.

Sorry if the description is confusing!
What I meant is that the styles file vis_type_timelion/public/components/index.scss could be loaded twice in case if user open both Timelion app and Timelion visualization (no matter either through the default editor or dashboard).
Since those styles were always loaded just at vis_type_timelion plugin initialization, they were shared between the legacy app and visualization itself.
Now, styles are lazy loaded only if you display the visualization, which would break the legacy app if it is opened first.

sulemanof · 2020-10-01T07:54:34Z

@elasticmachine merge upstream

flash1293 · 2020-10-01T08:05:18Z

src/plugins/vis_type_timelion/public/timelion_vis_type.tsx

-        <KibanaContextProvider services={{ ...dependencies }}>
-          <TimelionVisComponent {...props} />
-        </KibanaContextProvider>
-      ),
    },
    editorConfig: {
      optionsTemplate: (props: TimelionOptionsProps) => (


I know this is technically out of scope for this PR, but I think we could lazy-load TimelionOptions as well to shave off a few additional KB from the initial bundle size - wdyt?

Makes sense! Especially since it loads monaco 🤔
Will try!

Done!

Tested against the default editor PR, works well

sulemanof · 2020-10-01T12:28:46Z

@elasticmachine merge upstream

kibanamachine · 2020-10-01T14:04:57Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4ecd34d

Metrics [docs]

async chunks size

id	before	after	diff
`timelion`	455.5KB	463.1KB	+7.6KB
`visTypeTimelion`	0.0B	154.3KB	+154.3KB
total			+161.9KB

distributable file count

id	before	after	diff
`default`	45840	45846	+6
`oss`	26563	26569	+6

page load bundle size

id	before	after	diff
`visTypeTimelion`	183.4KB	36.1KB	-147.3KB
`visualizations`	272.6KB	272.4KB	-210.0B
total			-147.5KB

History

💚 Build #78654 succeeded dbfcef5
💚 Build #78623 succeeded c49073e
💚 Build #78231 succeeded fb1d99b
💔 Build #78213 failed e3601bc
💔 Build #78206 failed 03741e6

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Update styles * Implement toExpressionAst fn * Implement renderer * Update unit tests * Add unit tests * Update types * Remove unused vars * Fix types * Update types * Show error message when no data * Update ExpressionRenderDefinition api * Update renderer when there is no data * Make options component lazy Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> # Conflicts: # docs/development/plugins/expressions/public/kibana-plugin-plugins-expressions-public.expressionrenderdefinition.displayname.md # docs/development/plugins/expressions/server/kibana-plugin-plugins-expressions-server.expressionrenderdefinition.displayname.md # src/plugins/expressions/public/public.api.md # src/plugins/expressions/server/server.api.md

sulemanof added 7 commits September 24, 2020 16:26

Update styles

fd5dacf

Implement toExpressionAst fn

e443f61

Implement renderer

f91560b

Update unit tests

b337e8d

Add unit tests

5662e4b

Merge remote-tracking branch 'upstream/master' into feat/timelion_ren…

33063cd

…derer

Merge remote-tracking branch 'upstream/master' into feat/timelion_ren…

fb0b70d

…derer

sulemanof mentioned this pull request Sep 28, 2020

Renderers for all visualizations #46801

Closed

13 tasks

sulemanof added 4 commits September 29, 2020 11:27

Update types

aa0ed04

Merge remote-tracking branch 'upstream/master' into feat/timelion_ren…

26d1c48

…derer

Remove unused vars

f3d3e31

Fix types

6052295

sulemanof requested review from ppisljar, VladLasitsa, DianaDerevyankina, stratoula and alexwizp September 29, 2020 10:13

ppisljar approved these changes Sep 29, 2020

View reviewed changes

sulemanof marked this pull request as ready for review September 29, 2020 13:20

sulemanof requested a review from a team September 29, 2020 13:20

sulemanof requested a review from a team as a code owner September 29, 2020 13:20

alexwizp reviewed Sep 29, 2020

View reviewed changes

src/plugins/vis_type_timelion/public/timelion_vis_renderer.tsx Outdated Show resolved Hide resolved

alexwizp reviewed Sep 29, 2020

View reviewed changes

src/plugins/vis_type_timelion/public/components/timelion_vis_component.tsx Outdated Show resolved Hide resolved

alexwizp reviewed Sep 29, 2020

View reviewed changes

src/plugins/vis_type_timelion/public/components/timelion_vis_component.tsx Outdated Show resolved Hide resolved

alexwizp reviewed Sep 29, 2020

View reviewed changes

src/plugins/vis_type_timelion/public/timelion_vis_renderer.tsx Outdated Show resolved Hide resolved

alexwizp approved these changes Sep 29, 2020

View reviewed changes

sulemanof added 2 commits September 30, 2020 12:08

Update types

c50d1f0

Merge remote-tracking branch 'upstream/master' into feat/timelion_ren…

03741e6

…derer

sulemanof requested a review from a team as a code owner September 30, 2020 09:17

botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Sep 30, 2020

stratoula approved these changes Sep 30, 2020

View reviewed changes

Show error message when no data

84efe76

sulemanof mentioned this pull request Sep 30, 2020

Timelion shows a red error when no data is available #23743

Closed

sulemanof added 2 commits September 30, 2020 12:52

Update ExpressionRenderDefinition api

e3601bc

Update renderer when there is no data

fb1d99b

cchaos approved these changes Sep 30, 2020

View reviewed changes

Merge branch 'master' into feat/timelion_renderer

c49073e

flash1293 reviewed Oct 1, 2020

View reviewed changes

Make options component lazy

dbfcef5

Merge branch 'master' into feat/timelion_renderer

4ecd34d

sulemanof merged commit b692c37 into elastic:master Oct 1, 2020

sulemanof deleted the feat/timelion_renderer branch October 1, 2020 14:08

sulemanof mentioned this pull request Oct 1, 2020

[7.x] Timelion visualization renderer (#78540) #79124

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Timelion visualization renderer #78540

Timelion visualization renderer #78540

sulemanof commented Sep 25, 2020 •

edited

ppisljar left a comment

elasticmachine commented Sep 29, 2020

alexwizp Sep 29, 2020

sulemanof Sep 30, 2020

alexwizp left a comment

stratoula left a comment

cchaos left a comment

sulemanof commented Oct 1, 2020

sulemanof commented Oct 1, 2020

flash1293 Oct 1, 2020

sulemanof Oct 1, 2020 •

edited

sulemanof Oct 1, 2020

sulemanof commented Oct 1, 2020

kibanamachine commented Oct 1, 2020

Timelion visualization renderer #78540

Timelion visualization renderer #78540

Conversation

sulemanof commented Sep 25, 2020 • edited

Summary

Checklist

For maintainers

ppisljar left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 29, 2020

alexwizp Sep 29, 2020

Choose a reason for hiding this comment

sulemanof Sep 30, 2020

Choose a reason for hiding this comment

alexwizp left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

sulemanof commented Oct 1, 2020

sulemanof commented Oct 1, 2020

flash1293 Oct 1, 2020

Choose a reason for hiding this comment

sulemanof Oct 1, 2020 • edited

Choose a reason for hiding this comment

sulemanof Oct 1, 2020

Choose a reason for hiding this comment

sulemanof commented Oct 1, 2020

kibanamachine commented Oct 1, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

distributable file count

page load bundle size

History

sulemanof commented Sep 25, 2020 •

edited

sulemanof Oct 1, 2020 •

edited