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

[Exploratory View]added loading state for metric selector #115748

Merged
merged 10 commits into from
Oct 28, 2021

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Oct 20, 2021

Summary

Fixes #115740

Added a loading state while index pattern isn't available.

After loading if there is no index pattern at all, it shows no data available message.

@shahzad31 shahzad31 marked this pull request as ready for review October 20, 2021 11:36
@shahzad31 shahzad31 requested a review from a team as a code owner October 20, 2021 11:36
@shahzad31 shahzad31 self-assigned this Oct 20, 2021
@shahzad31 shahzad31 added v7.16.0 release_note:skip Skip the PR/issue when compiling release notes v8.0.0 labels Oct 20, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
observability 383.9KB 384.1KB +173.0B

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

cc @shahzad31

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Empty state looks good, although I think maybe the content shouldn't have a period. @liciavale wdyt? Also I think it would be nice if we could have a popover with a link to instructions to add rum data, or synthetics data, for empty states. Wdyt? I can create a follow up ticket for 8.0.0 as that would be out of scope of this bug fix.
Screen Shot 2021-10-25 at 2 08 14 PM

Also, part of the issue with the loading for me is the empty badge when loading. Can this be resolved as well?
Screen Shot 2021-10-25 at 2 07 02 PM

@liciavale
Copy link
Contributor

@dominiqueclarke

Also I think it would be nice if we could have a popover with a link to instructions to add rum data, or synthetics data, for empty states. Wdyt? I can create a follow up ticket for 8.0.0 as that would be out of scope of this bug fix.

Yes, this is definitely a good idea and we should look into empty states and onboarding as one of our next priorities. Good idea to create a follow-up ticket and we can take it from there.

@dominiqueclarke
Copy link
Contributor

@dominiqueclarke

Also I think it would be nice if we could have a popover with a link to instructions to add rum data, or synthetics data, for empty states. Wdyt? I can create a follow up ticket for 8.0.0 as that would be out of scope of this bug fix.

Yes, this is definitely a good idea and we should look into empty states and onboarding as one of our next priorities. Good idea to create a follow-up ticket and we can take it from there.

Raised here #116293

@shahzad31
Copy link
Contributor Author

@dominiqueclarke i have added loading for empty badge as well.

@shahzad31 shahzad31 force-pushed the metric-dropdown-loading-state branch from 8af1b50 to e77a31a Compare October 26, 2021 14:30
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM, I still think we should remove the period from "No data available."

…ory_view/series_editor/report_metric_options.tsx

Co-authored-by: Dominique Clarke <[email protected]>
@shahzad31 shahzad31 added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 27, 2021
@shahzad31 shahzad31 enabled auto-merge (squash) October 27, 2021 08:26
@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@shahzad31 shahzad31 merged commit 4a92e3b into elastic:master Oct 28, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
observability 385.7KB 385.9KB +220.0B

History

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

cc @shahzad31

@kibanamachine
Copy link
Contributor

The following labels were identified as gaps in your version labels and will be added automatically:

  • v8.1.0

If any of these should not be on your pull request, please manually remove them.

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0
7.16

The backport PRs will be merged automatically after passing CI.

@shahzad31 shahzad31 deleted the metric-dropdown-loading-state branch October 28, 2021 10:41
kibanamachine added a commit that referenced this pull request Oct 28, 2021
…116589)

Co-authored-by: Dominique Clarke <[email protected]>

Co-authored-by: Shahzad <[email protected]>
Co-authored-by: Dominique Clarke <[email protected]>
academo pushed a commit to academo/kibana that referenced this pull request Oct 28, 2021
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 29, 2021
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

2 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

shahzad31 added a commit that referenced this pull request Nov 2, 2021
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Exploratory view] No loading state for metric type dropdown
4 participants