-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation #182176
[ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation #182176
Conversation
Pinging @elastic/ml-ui (:ml) |
zoom === undefined && | ||
(focusRange === undefined || | ||
this.previousSelectedForecastId !== this.props.selectedForecastId) | ||
zoom === undefined || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fixing the bug when coming in from an annotation, but the issue fixed by #176969 is happening again where the zoom is not restored if the URL contains a forecastId
and zoom
parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed this is fixed by d614c66
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested latest changes and LGTM
LGTM 🎉 |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
…ing from a job annotation (elastic#182176) ## Summary Fixes elastic#181910 Ensures the position of the zoom slider gets set correctly when user navigates to the single metric viewer from the anomaly explorer page or jobs list from annotations -> job model snapshots -> single metric viewer. This PR removes the previous forecast id comparison as it was never getting correctly set so the `focusRange` was getting reset even when it should not have been because it was already defined. I believe this was introduced in elastic#176969. https://github.com/elastic/kibana/assets/6446462/cbd297ae-e3c2-43ab-ad06-27898ea06a3a Confirming the link with forecast id still works as expected: https://github.com/elastic/kibana/assets/6446462/9216c880-7bcf-4d46-b071-adb77f2db1e4 ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit a1b990d)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…en opening from a job annotation (#182176) (#182277) # Backport This will backport the following commits from `main` to `8.14`: - [[ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (#182176)](#182176) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Melissa Alvarez","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-05-01T17:29:28Z","message":"[ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (#182176)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/181910\r\n\r\nEnsures the position of the zoom slider gets set correctly when user\r\nnavigates to the single metric viewer from the anomaly explorer page or\r\njobs list from annotations -> job model snapshots -> single metric\r\nviewer.\r\n\r\nThis PR removes the previous forecast id comparison as it was never\r\ngetting correctly set so the `focusRange` was getting reset even when it\r\nshould not have been because it was already defined.\r\n\r\nI believe this was introduced in\r\nhttps://github.com//pull/176969.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6446462/cbd297ae-e3c2-43ab-ad06-27898ea06a3a\r\n\r\nConfirming the link with forecast id still works as expected:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6446462/9216c880-7bcf-4d46-b071-adb77f2db1e4\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [ ]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [ ] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI touched in this PR does not create any new axe failures\r\n(run axe in browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n- [ ] If a plugin configuration key changed, check if it needs to be\r\nallowlisted in the cloud and added to the [docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n- [ ] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [ ] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"a1b990ddd6a58519ecf6da55260de22c21aa0055","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix",":ml","Feature:Anomaly Detection","v8.14.0","v8.15.0"],"title":"[ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation","number":182176,"url":"https://github.com/elastic/kibana/pull/182176","mergeCommit":{"message":"[ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (#182176)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/181910\r\n\r\nEnsures the position of the zoom slider gets set correctly when user\r\nnavigates to the single metric viewer from the anomaly explorer page or\r\njobs list from annotations -> job model snapshots -> single metric\r\nviewer.\r\n\r\nThis PR removes the previous forecast id comparison as it was never\r\ngetting correctly set so the `focusRange` was getting reset even when it\r\nshould not have been because it was already defined.\r\n\r\nI believe this was introduced in\r\nhttps://github.com//pull/176969.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6446462/cbd297ae-e3c2-43ab-ad06-27898ea06a3a\r\n\r\nConfirming the link with forecast id still works as expected:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6446462/9216c880-7bcf-4d46-b071-adb77f2db1e4\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [ ]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [ ] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI touched in this PR does not create any new axe failures\r\n(run axe in browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n- [ ] If a plugin configuration key changed, check if it needs to be\r\nallowlisted in the cloud and added to the [docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n- [ ] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [ ] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"a1b990ddd6a58519ecf6da55260de22c21aa0055"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/182176","number":182176,"mergeCommit":{"message":"[ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (#182176)\n\n## Summary\r\n\r\nFixes https://github.com/elastic/kibana/issues/181910\r\n\r\nEnsures the position of the zoom slider gets set correctly when user\r\nnavigates to the single metric viewer from the anomaly explorer page or\r\njobs list from annotations -> job model snapshots -> single metric\r\nviewer.\r\n\r\nThis PR removes the previous forecast id comparison as it was never\r\ngetting correctly set so the `focusRange` was getting reset even when it\r\nshould not have been because it was already defined.\r\n\r\nI believe this was introduced in\r\nhttps://github.com//pull/176969.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6446462/cbd297ae-e3c2-43ab-ad06-27898ea06a3a\r\n\r\nConfirming the link with forecast id still works as expected:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6446462/9216c880-7bcf-4d46-b071-adb77f2db1e4\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [ ]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas added for features that require explanation or tutorials\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [ ] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI touched in this PR does not create any new axe failures\r\n(run axe in browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n- [ ] If a plugin configuration key changed, check if it needs to be\r\nallowlisted in the cloud and added to the [docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n- [ ] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [ ] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"a1b990ddd6a58519ecf6da55260de22c21aa0055"}}]}] BACKPORT--> Co-authored-by: Melissa Alvarez <[email protected]>
* master: (1654 commits) Bump ejs from 3.1.9 to 3.1.10 Don't render exceptions flyout if data is loading (elastic#181588) Enable value list modal (elastic#181593) skip flaky suite (elastic#181777) skip failing test suite (elastic#182263) [Mappings Editor] Disable _source field in serverless (elastic#181712) [data.search] Fix unhandled promise rejections (elastic#181785) [Fleet] Fix logic for detecting first time Elastic Agent users (elastic#182214) [ML] Decouple data_visualizer from MapEmbeddable (elastic#181928) [ES|QL] Sorting accepts expressions (elastic#181916) [ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (elastic#182176) Adding optional Description field to Roles APIs (elastic#182039) Upgrade Markdown-it to 14.1.0 (elastic#182244) Bump xml-crypto from 5.0.0 to 6.0.0 [DOCS] Fix docs and screenshots for rule creation changes (elastic#181925) Update dependency elastic-apm-node to ^4.5.3 (main) (elastic#182236) [Obs AI Assistant] register alert details context in observability plugin (elastic#181501) Add `@typescript-eslint/no-floating-promises` (elastic#181456) [Playground] Propagate Error message into FE (elastic#182201) [ES|QL] Rename the setting to a more generic one and move to the general section (elastic#182074) ...
Summary
Fixes #181910
Ensures the position of the zoom slider gets set correctly when user navigates to the single metric viewer from the anomaly explorer page or jobs list from annotations -> job model snapshots -> single metric viewer.
This PR removes the previous forecast id comparison as it was never getting correctly set so the
focusRange
was getting reset even when it should not have been because it was already defined.I believe this was introduced in #176969.
SMVfromAnnotationFix.mp4
Confirming the link with forecast id still works as expected:
ForecastLinkZoom.mp4
Checklist
Delete any items that are not applicable to this PR.