Skip to content

[Discover] Replace EuiTooltip by native title for better performance#68280

Merged
kertal merged 5 commits intoelastic:masterfrom
kertal:kertal-pr-2020-06-04-improve-sidebar-performance
Jun 8, 2020
Merged

[Discover] Replace EuiTooltip by native title for better performance#68280
kertal merged 5 commits intoelastic:masterfrom
kertal:kertal-pr-2020-06-04-improve-sidebar-performance

Conversation

@kertal
Copy link
Copy Markdown
Member

@kertal kertal commented Jun 4, 2020 •
edited
Loading

Summary

Replace Discover Sidebar Fieldname EuiTooltip by a simple native html title attribute for better performance and snappier interface

Old:
Using the EuiTooltip caused delays in the interface, causing a big workload on systems when fields were hovered rapidly (you can check this in #67732). Also CSS transitions in this case were to slow.

image

New:
Using native title attribute is not as pretty as the tooltip, but snappy and no workload issues. CSS transitions were also set to none.
image

Fixes #67732

Checklist

Delete any items that are not applicable to this PR.

@kertal kertal added the Feature:Discover Discover Application label Jun 4, 2020
@kertal kertal self-assigned this Jun 4, 2020
@kertal kertal marked this pull request as ready for review June 5, 2020 15:22
@kertal kertal requested a review from a team June 5, 2020 15:22
@kertal kertal requested a review from a team as a code owner June 5, 2020 15:22
@kertal kertal requested a review from timroes June 5, 2020 15:23
@kertal kertal added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Jun 5, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 5, 2020

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

@kertal kertal mentioned this pull request Jun 5, 2020
Closed
@timroes timroes added the v7.9.0 label Jun 8, 2020
timroes
timroes approved these changes Jun 8, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in Firefox and Chrome on Linux. I could reproduce the very high CPU load beforehand (spiked to 100% when hovering over the field list), and with this PR it got significantly better. Code LGTM.

@kertal
Copy link
Copy Markdown
Member Author

kertal commented Jun 8, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 8, 2020

💚 Build Succeeded

History

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

@flash1293
Copy link
Copy Markdown
Contributor

flash1293 commented Jun 8, 2020

Side note: Should we create an issue in the EUI repo so this performance bottlenecked is adressed somehow? Or are tooltips just not meant to be used like this?

@timroes
Copy link
Copy Markdown
Contributor

timroes commented Jun 8, 2020

@flash1293 Matthias created this already: elastic/eui#3568

@flash1293
Copy link
Copy Markdown
Contributor

flash1293 commented Jun 8, 2020

Missed that, sorry @kertal

snide
snide approved these changes Jun 8, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in browser. LGTM. Thanks for the fix. We'll check on the underlying issue in EUI.

@kertal kertal merged commit 23be466 into elastic:master Jun 8, 2020
kertal added a commit to kertal/kibana that referenced this pull request Jun 8, 2020
@kertal
[Discover] Replace EuiTooltip by native title for better performance (e…
4300c7a 
…lastic#68280)
@kertal kertal mentioned this pull request Jun 8, 2020
Merged
kertal added a commit that referenced this pull request Jun 9, 2020
@kertal
[Discover] Replace EuiTooltip by native title for better performance (#…
6a9dbbc 
…68280) (#68546)
kertal added a commit to kertal/kibana that referenced this pull request Jun 9, 2020
@kertal
[Discover] Replace EuiTooltip by native title for better performance (e…
2987b07 
…lastic#68280)

# Conflicts:
#	src/plugins/discover/public/application/components/sidebar/discover_field.tsx
@kertal kertal mentioned this pull request Jun 9, 2020
Merged
kertal added a commit to kertal/kibana that referenced this pull request Jun 9, 2020
@kertal
[Discover] Replace EuiTooltip by native title for better performance (e…
d64bfcb 
…lastic#68280)

# Conflicts:
#	src/plugins/discover/public/application/components/sidebar/discover_field.tsx
#	src/plugins/discover/public/application/components/sidebar/discover_sidebar.scss
@kertal kertal mentioned this pull request Jun 9, 2020
Merged
kertal added a commit that referenced this pull request Jun 9, 2020
@kertal
[Discover] Replace EuiTooltip by native title for better performance (#…
82016fe 
…68280) (#68616)
kertal added a commit that referenced this pull request Jun 10, 2020
@kertal
[7.7] [Discover] Replace EuiTooltip by native title for better perfor…
d4753dc 
…mance (#68280) (#68618)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@snide snide snide approved these changes

@timroes timroes timroes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kertal kertal

Labels

Feature:Discover Discover Application release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.7.2 v7.8.0 v7.9.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Kibana Discover high CPU usage, slow and laggy UI

6 participants

@kertal @elasticmachine @kibanamachine @flash1293 @timroes @snide