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

[Logs UI] Show navigation bar while loading source configuration #59997

Merged

Conversation

weltenwort
Copy link
Member

Summary

This moves the navigation bar and the source configuration loading indicator such that the bar is immediately visible and usable even before the source configuration has finished loading.

closes #56728

grafik

Checklist

Delete any items that are not applicable to this PR.

@weltenwort weltenwort added release_note:enhancement v8.0.0 Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.7.0 labels Mar 12, 2020
@weltenwort weltenwort self-assigned this Mar 12, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

@weltenwort weltenwort marked this pull request as ready for review March 12, 2020 14:04
@weltenwort weltenwort requested a review from a team as a code owner March 12, 2020 14:04
@afgomez afgomez self-requested a review March 12, 2020 14:14
@@ -146,6 +146,27 @@ export const useLogAnalysisModule = <JobType extends string>({
sourceId,
]);

useEffect(() => {
dispatchModuleStatus({
Copy link
Contributor

@afgomez afgomez Mar 12, 2020

Choose a reason for hiding this comment

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

I'm curious of how all of this connects together.

What does this effect do exactly to allow showing the tabs earlier in the loading process?

Copy link
Member Author

Choose a reason for hiding this comment

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

Previously, the module status reducer only used the source configuration it was initialized with to determine whether the job configuration is up-to-date. This meant that the source configuration had to be known before the provider was mounted.

Since this PR moves the loading state further down the hierarchy, the source configuration is now loaded in parallel to the job configuration. This tries to make sure that the reducer correctly evaluates the job configuration's currentness if the source configuration arrives asynchronously.

Does that make sense?

Copy link
Contributor

@afgomez afgomez left a comment

Choose a reason for hiding this comment

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

Tested locally. The tabs show correctly with and without ML enabled 🙌

@weltenwort weltenwort merged commit cf5ba6f into elastic:master Mar 12, 2020
@weltenwort weltenwort deleted the logs-ui-improve-source-loading-state branch March 12, 2020 22:26
weltenwort added a commit to weltenwort/kibana that referenced this pull request Mar 12, 2020
…tic#59997)

This moves the navigation bar and the source configuration loading indicator such that the bar is immediately visible and usable even before the source configuration has finished loading.

closes elastic#56728
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 13, 2020
* master:
  [Alerting] extend Alert Type with names/descriptions of action variables (elastic#59756)
  [Ingest] Fix data source creation and double system data source (elastic#60069)
  Add button to view full service map (elastic#59394)
  unskip tests for code coverage (elastic#59725)
  [Ingest] Add Fleet & EPM features (elastic#59376)
  [Logs UI] Show navigation bar while loading source configurati… (elastic#59997)
  [Endpoint] ERT-82 Alerts search bar (elastic#59702)
  Aggregate queue types being used by Beats (elastic#59850)
  skip flaky suite (elastic#59541)
  Convert Timeline to TypeScript (elastic#59966)
  Make context.core required argument to context providers (elastic#59996)
weltenwort added a commit that referenced this pull request Mar 13, 2020
Backports the following commits to 7.x:
 - [Logs UI] Show navigation bar while loading source configurati… (#59997)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Logs UI] navigation and UX improvements
4 participants