Vislib visualization renderer by nickofthyme · Pull Request #80744 · elastic/kibana

nickofthyme · 2020-10-15T20:04:15Z

Summary

Part of #46801

Implement a custom renderer for table visualization. This PR includes next changes:

implement toExpressionAst function for all vislib types and separate function for pie type.
implement a single custom legacy renderer for all vislib types
fix unit tests;
enhanced types;
lazy load components for the editor - reduce the initial bundle size;

Before

After

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2020-10-21T17:02:55Z

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

- use a serialiizable ms form of interval - recompute interval from esValue and esUnit not ms - remove legacy vislib code from build_pipeline

nickofthyme · 2020-10-27T16:52:42Z

@elastic/kibana-design and @elastic/kibana-app could I get a review please 🙏

sulemanof

LGTM 👍
Tested locally all affected vis types in Chrome & Firefox. Didn't find any regressions.

elizabetdev

@nickofthyme, I tested locally and everything seems to work as expected. There's only one minor scss change that you can do.

elizabetdev · 2020-10-28T13:36:20Z

src/plugins/vis_type_vislib/public/index.scss

-@import './vislib/index'
+@import './vislib/index';
+
+.vislib .visLegend__toggle {


These styles can be moved to src/plugins/vis_type_vislib/public/vislib/components/legend/_legend.scss.

stratoula

It looks great 👏 I have some minor comments that mainly have to do to remove lodash but it is LGTM. Tested locally on Safari and can't find any regression. Great size reduction 🥳

stratoula · 2020-10-29T10:23:47Z

src/plugins/vis_type_vislib/public/pie_fn.ts

-
 interface RenderValue {
-  visConfig: VisParams;
+  visData: any;


can we use unknown instead here?

stratoula · 2020-10-29T10:35:08Z

src/plugins/vis_type_vislib/public/to_ast.ts

+      const usedValueAxis = (visConfig.valueAxes || []).find(
+        (valueAxis) => valueAxis.id === seriesParam.valueAxis
+      );
+      if (get(usedValueAxis, 'scale.mode') === 'percentage') {


Can we avoid import get from lodash here and use typescript instead?

stratoula · 2020-10-29T10:37:42Z

src/plugins/vis_type_vislib/public/to_ast.ts

+        yDimension.format = { id: 'percent' };
+      }
+    }
+    if (get(visConfig, 'gauge.percentageMode') === true) {


Here too, in that way we don't need to use lodash at all 😄

stratoula · 2020-10-29T10:49:14Z

src/plugins/vis_type_vislib/public/vis_controller.tsx

-    mountLegend(visData: any, position: Positions) {
-      this.unmount = mountReactNode(
+    mountLegend(
+      visData: any,


And I think this could be unknown too

stratoula · 2020-10-29T10:50:27Z

src/plugins/vis_type_vislib/public/vis_renderer.tsx

+    return false;
+  }
+
+  const rows: object[] | undefined = get(visData, 'rows');


I suggest to not use lodash here too.

nickofthyme · 2020-10-29T16:10:06Z

@elasticmachine merge upstream

kibanamachine · 2020-10-29T18:17:58Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 90751e4

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`visTypeVislib`	275	280	+5

async chunk count

id	before	after	diff
`visTypeVislib`	1	7	+6

async chunks size

id	before	after	diff
`visTypeTagcloud`	284.5KB	284.5KB	+21.0B
`visTypeVislib`	538.0KB	700.3KB	+162.2KB
total			+162.3KB

distributable file count

id	before	after	diff
`default`	48117	48134	+17
`oss`	28602	28619	+17

page load bundle size

id	before	after	diff
`visTypeTimelion`	34.3KB	34.3KB	-33.0B
`visTypeVislib`	207.9KB	66.1KB	-141.9KB
`visualizations`	207.2KB	203.8KB	-3.3KB
total			-145.2KB

History

💔 Build #84599 failed a7d06e8
💔 Build #84550 failed 04764b1
💔 Build #84543 failed 96cd1c7
💚 Build #84281 succeeded 00b3fc8
💚 Build #84247 succeeded 51c5c3c

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

refactor vis_type_vislib to use async vis renderer

c1429e0

nickofthyme requested a review from sulemanof October 15, 2020 20:04

nickofthyme mentioned this pull request Oct 15, 2020

Renderers for all visualizations #46801

Closed

13 tasks

nickofthyme added 7 commits October 15, 2020 16:38

fix resizing issue

3307b26

Merge branch 'master' into vislib-renderer

4fca736

fix resizing issue

70c3584

Merge remote-tracking branch 'upstream/master' into vislib-renderer

d4ae5f0

fix jest tests

439afd5

wrap vislib controller in react component

a940e96

fix dashboard vis reloading

263ae4f

nickofthyme marked this pull request as ready for review October 21, 2020 17:02

nickofthyme requested a review from a team October 21, 2020 17:02

nickofthyme requested a review from a team as a code owner October 21, 2020 17:02

nickofthyme and others added 9 commits October 21, 2020 12:03

Merge remote-tracking branch 'upstream/master' into vislib-renderer

c100a82

fix error selector and color updating

0d7ab3f

refactor ast to pass proper interval

92dc916

- use a serialiizable ms form of interval - recompute interval from esValue and esUnit not ms - remove legacy vislib code from build_pipeline

move old vislib tests

28c3224

fix type errors

6a64754

Merge remote-tracking branch 'upstream/master' into vislib-renderer

c9121f7

Refactoring

aa9b053

Move visError styles into visualizations

1dc6151

Correctly handle uiState updates

bc81480

sulemanof added 4 commits October 28, 2020 11:38

Fix overflow in gauge/goal visulaizations

4fe6472

Lazy load editor components

caedcff

Merge remote-tracking branch 'upstream/master' into vislib-renderer

d49aa2d

Minor fixes

51c5c3c

sulemanof approved these changes Oct 28, 2020

View reviewed changes

elizabetdev approved these changes Oct 28, 2020

View reviewed changes

move styles to legend

00b3fc8

stratoula approved these changes Oct 29, 2020

View reviewed changes

nickofthyme added 3 commits October 29, 2020 09:05

fix unknown types and remove lodash.get

8a115a5

Merge branch 'master' into vislib-renderer

96cd1c7

fix type error in jest tests

04764b1

kibanamachine and others added 3 commits October 29, 2020 12:10

Merge branch 'master' into vislib-renderer

a7d06e8

fix noResults state

e1d44f4

fix type definition

90751e4

nickofthyme merged commit e0b6b0b into elastic:master Oct 29, 2020

nickofthyme deleted the vislib-renderer branch October 29, 2020 18:34

nickofthyme mentioned this pull request Oct 29, 2020

[7.x] Vislib visualization renderer (#80744) #82059

Merged

nickofthyme added a commit that referenced this pull request Oct 29, 2020

Vislib visualization renderer (#80744) (#82059)

95eecde

nickofthyme mentioned this pull request Nov 12, 2020

fix tall vislib charts in visualize #83340

Merged

lukeelmers mentioned this pull request Dec 7, 2020

[data.search] Clean up arguments to esaggs. #84973

Merged

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Dec 9, 2020

Fix vislib regression from elastic#80744.

256d7b1

VladLasitsa mentioned this pull request Dec 10, 2020

Selecting optimize for printing exports pdf graphs with the X-axis labels moved into the chart #72314

Closed

Conversation

nickofthyme commented Oct 15, 2020 • edited by sulemanof Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before

After

Checklist

Uh oh!

elasticmachine commented Oct 21, 2020

Uh oh!

nickofthyme commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sulemanof left a comment

Choose a reason for hiding this comment

Uh oh!

elizabetdev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elizabetdev Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Oct 29, 2020

Uh oh!

kibanamachine commented Oct 29, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunk count

async chunks size

distributable file count

page load bundle size

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nickofthyme commented Oct 15, 2020 •

edited by sulemanof

Loading

nickofthyme commented Oct 27, 2020 •

edited

Loading

elizabetdev left a comment •

edited

Loading

elizabetdev Oct 28, 2020 •

edited

Loading