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

[Infrastructure UI] Use same no data messaging on hosts view #142063

Conversation

jennypavlova
Copy link
Member

@jennypavlova jennypavlova commented Sep 28, 2022

Closes #141273

Summary

This PR adds a No Data message similar to the Infrastructure page

Testing locally

  • Check if the component renders ( I put a date in the future in order to see the no data message)

image

  • Verify that onRefetch works

@jennypavlova jennypavlova self-assigned this Sep 28, 2022
@jennypavlova
Copy link
Member Author

@elasticmachine merge upstream

@jennypavlova jennypavlova added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services backport:skip This commit does not require backporting labels Sep 28, 2022
@neptunian
Copy link
Contributor

neptunian commented Sep 29, 2022

Testing this with metricbeat this didn't work. No actual request is sent when clicking "Check for new data" because the LensComponent is no longer mounted the first time there no data so the Lens callback onLoad never runs and never sets the state again. This also causes the Unified Search "refresh" button to stop working. So I think we will need to do this differently. Perhaps everytime they clicked on either of these two buttons we'd load the Lens component and keep track of that state and then we could get access to the onLoad to do something? It might cause a "flash" of the empty table. Just an idea.

To test with metricbeat try https://www.elastic.co/guide/en/beats/metricbeat/master/metricbeat-installation-configuration.html or happy to pair on that.

@jennypavlova
Copy link
Member Author

@neptunian Thank you for checking that! I saw the issue you mentioned. You are right that we should render the lens table in order to set the state so what I tried is to keep the component and visually hide it when there is no data. Probably it is not the best approach here, I just tried to make sure that the request is sent after click.
Of course, I am open to other ideas and to pair on that later today 🙂

@neptunian
Copy link
Contributor

neptunian commented Sep 30, 2022

Nice! This works. Instead of rendering the components and hiding them, though, I think it'd be better if we did not render them at all because there is going to be a lot of components on this page.

Using local state or a custom hook, we could track the loading state of the table instead and render something like this:

  if (noData && !isLoading) {
    return (
      <NoData
        titleText={i18n.translate('xpack.infra.metrics.emptyViewTitle', {
          defaultMessage: 'There is no data to display.',
        })}
        bodyText={i18n.translate('xpack.infra.metrics.emptyViewDescription', {
          defaultMessage: 'Try adjusting your time or filter.',
        })}
        refetchText={i18n.translate('xpack.infra.metrics.refetchButtonLabel', {
          defaultMessage: 'Check for new data',
        })}
        onRefetch={onRefetch}
        testString="metricsEmptyViewState"
      />
    );
  }

  return (
    <LensComponent
      id="hostsView"
      timeRange={timeRange}
      attributes={getLensHostsTable(dataView, query)}
      searchSessionId={searchSessionId}
      onLoad={(isLoading, adapters) => {
        if (!isLoading && adapters?.tables) {
          setNoData(adapters?.tables.tables.default?.rows.length === 0);
          onIsLoading(false)
        }
      }}
    />)

The loading would be set to false in the LensComponent and true when the query is initiated such as in onQuerySubmit. Since I think the table could drive whether we want to load other components (if there's no host data why load other components?) it could be useful to track/share the loading/no data state of the table. What do you think?

@jennypavlova
Copy link
Member Author

Instead of rendering the components and hiding them, though, I think it'd be better if we did not render them at all because there is going to be a lot of components on this page.

Good point, thanks! I added the change and now it works for me. Please let me know if it works for you.

Another thing I noticed is that on the infrastructure page when I change the search term/filter the result changes from no data to existing result and here I need to click on refresh (I think it was like that before those changes, not 100% sure) So that's still a difference between the buttons now that the refresh button will also apply the filters and if there is no filter change it will refresh. The no data page button won't apply the filters it will behave more like the light blue refresh (which I think is correct in this case)
image
image

@neptunian
Copy link
Contributor

@elasticmachine merge upstream

@neptunian
Copy link
Contributor

This works! 🚀

That seems like correct behaviour to me if I understand you correctly. The teal button state is unapplied changes so the "check for new data" would not apply the time range selected if you haven't applied them yet by clicking "refresh". I think that's fine.

@jennypavlova jennypavlova marked this pull request as ready for review October 4, 2022 07:51
@jennypavlova jennypavlova requested a review from a team as a code owner October 4, 2022 07:51
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@jennypavlova jennypavlova added the release_note:skip Skip the PR/issue when compiling release notes label Oct 4, 2022
@miltonhultgren miltonhultgren changed the title 141273 [Infrastructure UI] use same no data messaging on hosts view [Infrastructure UI] use same no data messaging on hosts view Oct 4, 2022
@miltonhultgren miltonhultgren changed the title [Infrastructure UI] use same no data messaging on hosts view [Infrastructure UI] Use same no data messaging on hosts view Oct 4, 2022
@kibana-ci
Copy link
Collaborator

💚 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
infra 1.0MB 1.0MB +836.0B

History

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

cc @jennypavlova

@jennypavlova jennypavlova merged commit d95e690 into elastic:main Oct 4, 2022
@jennypavlova jennypavlova deleted the 141273-infrastructure-ui-use-same-no-data-messaging-on-hosts-view branch October 5, 2022 08:34
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
…#142063)

* [WIP] Implement No Data message

* Implement refetch

* Render lens component and hide when there is no data

* Add onLoading hook and conditional rendering

Co-authored-by: Kibana Machine <[email protected]>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
…#142063)

* [WIP] Implement No Data message

* Implement refetch

* Render lens component and hide when there is no data

* Add onLoading hook and conditional rendering

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Epic: Host Observability release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Infrastructure UI] Use same No Data messaging on Hosts View
5 participants