Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Avoid unnecessary data fetching on dimension flyout open #82957

Merged
merged 3 commits into from
Nov 16, 2020

Conversation

dej611
Copy link
Contributor

@dej611 dej611 commented Nov 9, 2020

Summary

This PR addresses a common behaviour of some operation on Dimension flyout opening action.
Some components for the terms or range operations were updating the state on mount, due to the useDebounce implementation.

A custom useDebounce wrapper has been created that skips the first render - but ideally it could be extended as well in the future.

This new implementation avoids most of the data fetching scenario.

There's still an initial data fetching in case of first panel opening for the range base operation as the frontend needs to convert the "auto" value into a numeric value: note this was happening in all modes before, while now it has a really specific target.
Tests have been adapted to handle the new implementation, and new tests have been added to detect this specific scenario.

Fixes: #82605

Checklist

@dej611 dej611 added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v7.11.0 labels Nov 9, 2020
@dej611 dej611 marked this pull request as ready for review November 10, 2020 14:47
@dej611 dej611 requested a review from a team November 10, 2020 14:47
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested and no unexpected double fetches anymore, LGTM.

I'm unsure about the max bars value handling in general, but let's discuss separately.

@dej611
Copy link
Contributor Author

dej611 commented Nov 11, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@dej611
Copy link
Contributor Author

dej611 commented Nov 16, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 433 434 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 917.9KB 918.4KB +468.0B

History

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

@dej611 dej611 merged commit c8b8a0a into elastic:master Nov 16, 2020
@dej611 dej611 deleted the fix/82605 branch November 16, 2020 13:35
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 16, 2020
… into add-logs-to-node-details

* 'add-logs-to-node-details' of github.com:phillipb/kibana:
  fix tall vislib charts in visualize (elastic#83340)
  [Lens] Avoid unnecessary data fetching on dimension flyout open (elastic#82957)
  [Security Solution][Case] Change case connector minimum required license to basic (elastic#83401)
  fix logstash central pipeline management test  (elastic#83281)
  [Search] Send to background UI (elastic#81793)
  Migrate `/translations` route to core (elastic#83280)
  [APM] Ensure APM jest script can run (elastic#83398)
  [Uptime] Monitor status alert use url as instance (elastic#81736)
  [ML] Add basic license test run details to ML+Transform READMEs (elastic#83259)
  TSVB doesn't communicate it's index-patterns to dashboard (elastic#82964)
  [Alerting UI] Added ability to assign alert actions to resolved action group in UI (elastic#83139)
  Skips Vega test
  skip flaky suite (elastic#79967)
  [bundle optimization] Update to semver 7.x to get tree-shaking (elastic#83020)
  Added ability to fire actions when an alert instance is resolved (elastic#82799)
dej611 added a commit that referenced this pull request Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Data is fetched on opening dimension flyout
4 participants