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

[ML] Improve browser history navigation #83792

Merged
merged 17 commits into from
Nov 24, 2020

Conversation

darnautov
Copy link
Contributor

Summary

Fixes #73261
Part of #83033

  • Adds history state replacement for default URL states. It solves the issue on the Anomaly Explorer and Single Metric Viewer pages when multiple clicks were required to navigate back from the page.
  • Fix URL generation for Anomaly Explorer
  • Update Anomaly Explorer URL generation for Security app

Checklist

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@darnautov darnautov requested a review from a team as a code owner November 19, 2020 14:46
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Nov 19, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@peteharverson
Copy link
Contributor

For a job using a metric detector and a partition field, if I open the Single Metric Viewer from the Jobs list, hitting the back button after picking a partitioning field value doesn't return me to the Jobs list.

Used cloudwatch and this config (need to create the job in the Advanced wizard):

  "analysis_config": {
    "bucket_span": "15m",
    "detectors": [
      {
        "detector_description": "metric(DiskWriteBytes) partitionfield=instance",
        "function": "metric",
        "field_name": "DiskWriteBytes",
        "partition_field_name": "instance",
        "detector_index": 0
      }
    ],
    "influencers": [
      "instance",
      "region"
    ]
  }

@peteharverson
Copy link
Contributor

As discussed, would be nice if only one click was required to return the Jobs list after opening the Single Metric Viewer for a job with a partition field. Currently two clicks are required (because the zoom changes after selecting the partition field value). But if significant code changes are required, I don't think it's needed for this PR.

@darnautov
Copy link
Contributor Author

For a job using a metric detector and a partition field, if I open the Single Metric Viewer from the Jobs list, hitting the back button after picking a partitioning field value doesn't return me to the Jobs list.

Used cloudwatch and this config (need to create the job in the Advanced wizard):

  "analysis_config": {
    "bucket_span": "15m",
    "detectors": [
      {
        "detector_description": "metric(DiskWriteBytes) partitionfield=instance",
        "function": "metric",
        "field_name": "DiskWriteBytes",
        "partition_field_name": "instance",
        "detector_index": 0
      }
    ],
    "influencers": [
      "instance",
      "region"
    ]
  }

Shouldn't be the case anymore, could you please check with the latest changes @peteharverson

@qn895
Copy link
Member

qn895 commented Nov 20, 2020

Tested and LGTM 🎉

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Great improvement, LGTM pending the issue Pete found :) . Just added one question/suggestion.

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested latest edits and LGTM.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 1586 1587 +1

Async chunks

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

id before after diff
ml 5.2MB 5.2MB +1.8KB
securitySolution 7.9MB 7.9MB -82.0B
total +1.7KB

Page load bundle

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

id before after diff
ml 67.7KB 67.8KB +77.0B

History

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

Copy link
Contributor

@XavierM XavierM left a comment

Choose a reason for hiding this comment

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

LGTM for security_solution

@darnautov darnautov merged commit bfbb43e into elastic:master Nov 24, 2020
@darnautov darnautov deleted the ML-support-replace-state branch November 24, 2020 16:14
darnautov added a commit that referenced this pull request Nov 24, 2020
* [ML] replace history support

* [ML] explorer url state

* [ML] timeseriesexplorer url state

* [ML] fix state keys for mlSelectSeverity and mlSelectInterval

* [ML] fix useSelectedCells

* [ML] update urls and tests in security app

* [ML] fix TS

* [ML] fix apm unit tests

* [ML] fix typo

* [ML] remove state sync

* [ML] fix initial zoom set

* [ML] fix initial zoom set

* [ML]: update with useMlHref

* [ML] fix TS issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Multiple clicks required to navigate back from SMV/Anomaly Explorer to jobs list
8 participants