Implement tagcloud renderer #77910

sulemanof · 2020-09-18T15:41:25Z

Summary

Part of #46801

Implement own renderer for Tag Cloud visualization.
lazy load the visualization component
Implement toExpressionAst function for building pipeline.
add unit tests

Fixes extra scrollbar at visualization:

Checklist

Unit or functional tests were updated or added to match the most common scenarios
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

This reverts commit fddf019.

elasticmachine · 2020-09-23T09:59:55Z

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

ppisljar

code LGTM for changes related to renderers and toAST. didn't check changes to actual tag cloud implementation apart from emitting events.

cchaos

A couple things I noticed.

1. When resizing so small that it can't render all the items, increasing the size again makes the tags overlap.

This could easily happen in a Dashboard when resizing the window.

2. We really need to get rid of `Open Sans`.

By just removing the font-family declaration from the <text> elements, they'll properly inherit our UI font family Inter.

sulemanof · 2020-09-23T21:18:00Z

A couple things I noticed.

1. When resizing so small that it can't render all the items, increasing the size again makes the tags overlap.

This could easily happen in a Dashboard when resizing the window.

2. We really need to get rid of Open Sans.

By just removing the font-family declaration from the <text> elements, they'll properly inherit our UI font family Inter.

It seems that the first issue is a result of changing the font-family in tag cloud config (as far as I see the font was changed).
I can't reproduce the same issue without the change.
Also it seems it's not so simple to get rid of setting font-family explicitly, because the d3 tagcloud should be built laying on it. possible explanation
At least, it needs more investigation.
Since it's not a regression caused by this PR, let's create a separate issue for replacing the font-family.

sulemanof · 2020-09-24T10:55:57Z

@elasticmachine merge upstream

cchaos · 2020-09-24T13:01:27Z

The problem with leaving "Open Sans" in is that Kibana no longer ships with the "Open Sans" font. It hasn't since 7.0. So for most users it is going to fallback to system fonts which is most likely going to still mess with rendering. So I think it's really important to change this.

sulemanof · 2020-09-24T13:51:31Z

The problem with leaving "Open Sans" in is that Kibana no longer ships with the "Open Sans" font. It hasn't since 7.0. So for most users it is going to fallback to system fonts which is most likely going to still mess with rendering. So I think it's really important to change this.

ok, I see.. so it def need to be fixed, but anyway I think that's not in scope of this PR.
I opened an issue #78407 to track this.
If it has high priority, we'll try to resolve this ASAP. @stratoula @timroes what are your thoughts?

stratoula · 2020-09-24T13:56:22Z

@sulemanof agree! Let's push it on 7.11

cchaos

Purely from the SASS files, looks fine

sulemanof · 2020-09-28T08:08:23Z

@elasticmachine merge upstream

stratoula

LGTM, tested it locally on Chrome ❤️

…derer

kibanamachine · 2020-09-28T13:08:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 6ff952f

Metrics [docs]

@kbn/optimizer bundle module count

id	value	diff	baseline
visTypeTagcloud	29	+3	26

async chunks size

id	value	diff	baseline
visTypeTagcloud	295.4KB	⚠️ +295.4KB	0.0B

page load bundle size

id	value	diff	baseline
visTypeTagcloud	22.1KB	-285.7KB	307.8KB
visualizations	272.6KB	-765.0B	273.4KB
total		-286.4KB

distributable file count

id	value	diff	baseline
default	45649	+6	45643
oss	26391	+6	26385

History

💚 Build #77290 succeeded d7e0414
💚 Build #76872 succeeded f35bd4c
💚 Build #76466 succeeded 5c72381
💚 Build #76212 succeeded aed0355
💔 Build #76154 failed fddf019

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

@timestamp

* master: (226 commits) [Enterprise Search] Added Logic for the Credentials View (elastic#77626) [CSM] Js errors (elastic#77919) Add the @kbn/apm-config-loader package (elastic#77855) [Security Solution] Refactor useSelector (elastic#75297) Implement tagcloud renderer (elastic#77910) [APM] Alerting: Add global option to create all alert types (elastic#78151) [Ingest pipelines] Upload indexed document to test a pipeline (elastic#77939) TypeScript cleanup in visualizations plugin (elastic#78428) Lazy load metric & mardown visualizations (elastic#78391) [Detections][EQL] EQL rule execution in detection engine (elastic#77419) Update tutorial-full-experience.asciidoc (elastic#75836) Update tutorial-define-index.asciidoc (elastic#75754) Add support for runtime field types to mappings editor. (elastic#77420) [Monitoring] Usage collection (elastic#75878) [Docs][Actions] Add docs for Jira and IBM Resilient (elastic#78316) [Security Solution][Resolver] Update @timestamp formatting (elastic#78166) [Security Solution] Fix app layout (elastic#76668) [Security Solution][Resolver] 2 new functions to DAL (elastic#78477) Adds new elasticsearch client to telemetry plugin (elastic#78046) skip flaky suite (elastic#78512) (elastic#78511) (elastic#78510) (elastic#78509) (elastic#78508) (elastic#78507) (elastic#78506) (elastic#78505) (elastic#78504) (elastic#78503) (elastic#78502) (elastic#78501) (elastic#78500) ...

* Implement toExpressionAst for tagcloud * Implement tagcloud vis renderer * Use resize observer * Use common no data message * Update build_pipeline.test * Update tag cloud tests * Revert "Use common no data message" This reverts commit fddf019. * Update interpreter functional tests * Add tests for toExpressionAst fn * Use throttled chart update * Update renderer Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

kibanamachine · 2020-09-29T13:18:45Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…t tagcloud renderer (#77910) | Fix types (#78619) (#78666) * TypeScript cleanup in visualizations plugin (#78428) Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * Implement tagcloud renderer (#77910) * Implement toExpressionAst for tagcloud * Implement tagcloud vis renderer * Use resize observer * Use common no data message * Update build_pipeline.test * Update tag cloud tests * Revert "Use common no data message" This reverts commit fddf019. * Update interpreter functional tests * Add tests for toExpressionAst fn * Use throttled chart update * Update renderer Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * Fix types (#78619) Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Daniil Suleiman <31325372+sulemanof@users.noreply.github.com>

sulemanof added 9 commits September 18, 2020 18:27

Implement toExpressionAst for tagcloud

a4aa1e7

Implement tagcloud vis renderer

41cff2e

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

bce20af

…derer

Use resize observer

d02577f

Use common no data message

fddf019

Update build_pipeline.test

f140d99

Update tag cloud tests

83e5740

Revert "Use common no data message"

f43b0bd

This reverts commit fddf019.

Update interpreter functional tests

0e444f6

sulemanof mentioned this pull request Sep 22, 2020

Renderers for all visualizations #46801

Closed

13 tasks

Add tests for toExpressionAst fn

aed0355

sulemanof requested review from ppisljar and stratoula September 23, 2020 09:59

sulemanof marked this pull request as ready for review September 23, 2020 09:59

sulemanof requested a review from a team September 23, 2020 09:59

sulemanof requested review from a team as code owners September 23, 2020 09:59

Use throttled chart update

5c72381

ppisljar approved these changes Sep 23, 2020

View reviewed changes

cchaos suggested changes Sep 23, 2020

View reviewed changes

Merge branch 'master' into feat/tagcloud_renderer

f35bd4c

This was referenced Sep 24, 2020

Use Resize polyfill in Tag cloud #9450

Closed

Make tagcloud vis to use kibana font family #78407

Closed

cchaos approved these changes Sep 24, 2020

View reviewed changes

Merge branch 'master' into feat/tagcloud_renderer

d7e0414

stratoula approved these changes Sep 28, 2020

View reviewed changes

sulemanof added 2 commits September 28, 2020 14:26

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

a676d85

…derer

Update renderer

6ff952f

sulemanof merged commit 53d4938 into elastic:master Sep 28, 2020

sulemanof deleted the feat/tagcloud_renderer branch September 28, 2020 13:14

sulemanof added v7.10.0 and removed v7.11.0 labels Sep 28, 2020

sulemanof mentioned this pull request Sep 28, 2020

[7.x] Implement tagcloud renderer (#77910) #78580

Closed

sulemanof mentioned this pull request Sep 28, 2020

Fix visualizations types #78619

Merged

7 tasks

timroes mentioned this pull request Sep 28, 2020

[7.x] TypeScript cleanup in visualizations plugin (#78428) | Implement tagcloud renderer (#77910) | Fix types (#78619) #78666

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 29, 2020

sulemanof added backported and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Sep 29, 2020

lukeelmers mentioned this pull request Nov 20, 2020

Migrated tag cloud from 5.6.16 displays error in 7.10/7.11 #83876

Closed

sulemanof mentioned this pull request Nov 23, 2020

Fix: tagcloud explicitly pass params #84107

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement tagcloud renderer #77910

Implement tagcloud renderer #77910

sulemanof commented Sep 18, 2020 •

edited

Loading

elasticmachine commented Sep 23, 2020

ppisljar left a comment

cchaos left a comment

sulemanof commented Sep 23, 2020

1. When resizing so small that it can't render all the items, increasing the size again makes the tags overlap.

2. We really need to get rid of `Open Sans`.

sulemanof commented Sep 24, 2020

cchaos commented Sep 24, 2020

sulemanof commented Sep 24, 2020

stratoula commented Sep 24, 2020

cchaos left a comment

sulemanof commented Sep 28, 2020

stratoula left a comment

kibanamachine commented Sep 28, 2020

kibanamachine commented Sep 29, 2020

Implement tagcloud renderer #77910

Implement tagcloud renderer #77910

Conversation

sulemanof commented Sep 18, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Sep 23, 2020

ppisljar left a comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

1. When resizing so small that it can't render all the items, increasing the size again makes the tags overlap.

2. We really need to get rid of Open Sans.

sulemanof commented Sep 23, 2020

1. When resizing so small that it can't render all the items, increasing the size again makes the tags overlap.

2. We really need to get rid of Open Sans.

sulemanof commented Sep 24, 2020

cchaos commented Sep 24, 2020

sulemanof commented Sep 24, 2020

stratoula commented Sep 24, 2020

cchaos left a comment

Choose a reason for hiding this comment

sulemanof commented Sep 28, 2020

stratoula left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 28, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

page load bundle size

distributable file count

History

kibanamachine commented Sep 29, 2020

sulemanof commented Sep 18, 2020 •

edited

Loading

2. We really need to get rid of `Open Sans`.

2. We really need to get rid of `Open Sans`.