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.

Sign up for GitHub

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

Jump to bottom

[Logs UI] Use useMlHref hook for ML links #90935

Merged
merged 8 commits into from
Feb 15, 2021
Merged

[Logs UI] Use useMlHref hook for ML links #90935

merged 8 commits into from
Feb 15, 2021

Conversation

Kerry350
Copy link
Contributor

Summary

This PR implements #79426. It removes our custom generation of ML links and switches to the useMlHref hook from the ML team.

(I've added release_note:fix as these links are actually broken at the moment).

Testing

These links all need verifying:

Screenshot 2021-02-10 at 13 21 45

(With both log rate and category anomalies)

Screenshot 2021-02-10 at 13 21 59

Screenshot 2021-02-10 at 13 22 08

@Kerry350 Kerry350 added release_note:fix 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.12.0 labels Feb 10, 2021
@Kerry350 Kerry350 added this to the Logs UI 7.12 milestone Feb 10, 2021
@Kerry350 Kerry350 requested a review from a team February 10, 2021 13:26
@Kerry350 Kerry350 self-assigned this Feb 10, 2021
@elasticmachine
Copy link
Contributor

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

@Kerry350
Copy link
Contributor Author

@elasticmachine merge upstream

@afgomez afgomez self-requested a review February 11, 2021 12:31
afgomez
afgomez suggested changes Feb 11, 2021
View reviewed changes
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.

Links seem to work and go to the right places 🙌.

There's one small thing: when the user Cmd+Clicks on "View anomaly in machine learning" or the small machine learning icon, it should open in a new window without changing the current tab. This works correctly on master.

@Kerry350
Copy link
Contributor Author

@afgomez Ah, good catch. We usually get that handling for free from the useLinkProps hook. I've exported that helper from there and used it in the ML link callbacks (I've added it to the "Analyze in ML" button as well for consistency, although it won't apply there as it's a button). The two actual links should work properly now.

@Kerry350
Copy link
Contributor Author

@elasticmachine merge upstream

@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
infra 1.9MB 1.9MB -51.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
infra 190.0KB 190.1KB +149.0B

History

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

afgomez
afgomez approved these changes Feb 15, 2021
View reviewed changes
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.

Everything works nice!

@Kerry350 Kerry350 merged commit 711724f into elastic:master Feb 15, 2021
@Kerry350 Kerry350 mentioned this pull request Feb 15, 2021
Merged
Kerry350 added a commit to Kerry350/kibana that referenced this pull request Feb 15, 2021
@Kerry350
[Logs UI] Use useMlHref hook for ML links (elastic#90935)
b78361e 
* Use useMlHref hook for ML related links
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 15, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ilm/update-co…
28ed48d 
…ndition-for-hiding-recommded-allocation

* 'master' of github.com:elastic/kibana:
  [Discover] Fix toggling multi fields from doc view table (elastic#91121)
  [ML] Data Frame Analytics: ROC Curve Chart (elastic#89991)
  skip flaky suite (elastic#86948)
  skip flaky suite (elastic#91191)
  Fix date histogram time zone for rollup index (elastic#90632)
  [Search Source] Fix retrieval of unmapped fields; Add field filters (elastic#89837)
  [Logs UI] Use useMlHref hook for ML links (elastic#90935)
  Fix values of `products.min_price` field in Kibana sample ecommerce data set (elastic#90428)
  [APM] Darker shade for Error group details labels (elastic#91349)
  [Lens] Adjust new copy for 7.12 (elastic#90413)
  [ML] Unskip test. Fix modelMemoryLimit value. (elastic#91280)
  [Lens] Fix empty display name issue in XY chart (elastic#91132)
  [Lens] Improves error messages when in Dashboard (elastic#90668)
  [Lens] Keyboard-selected items follow user traversal of drop zones (elastic#90546)
  [Lens] Improves ranking feature in Top values (elastic#90749)
  [ILM] Rollover min age tooltip and copy fixes (elastic#91110)

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts
Kerry350 added a commit that referenced this pull request Feb 15, 2021
@Kerry350
[Logs UI] Use useMlHref hook for ML links (#90935) (#91395)
f5bad32 
* Use useMlHref hook for ML related links
This was referenced Feb 22, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afgomez afgomez afgomez approved these changes

Assignees

@Kerry350 Kerry350

Labels
Feature:Logs UI Logs UI feature release_note:fix Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.12.0 v8.0.0
Projects
None yet
Milestone
Logs UI 7.12
Development

Successfully merging this pull request may close these issues.
4 participants
@Kerry350 @elasticmachine @kibanamachine @afgomez