[ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation#182176
Merged
alvarezmelissa87 merged 3 commits intoelastic:mainfrom May 1, 2024
Conversation
Contributor
|
Pinging @elastic/ml-ui (:ml) |
| zoom === undefined && | ||
| (focusRange === undefined || | ||
| this.previousSelectedForecastId !== this.props.selectedForecastId) | ||
| zoom === undefined || |
Contributor
There was a problem hiding this comment.
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.
Contributor
Author
|
@elasticmachine merge upstream |
peteharverson
approved these changes
May 1, 2024
Contributor
peteharverson
left a comment
There was a problem hiding this comment.
Tested latest changes and LGTM
Member
|
LGTM 🎉 |
qn895
approved these changes
May 1, 2024
qn895
approved these changes
May 1, 2024
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
kibanamachine
added a commit
to kibanamachine/kibana
that referenced
this pull request
May 1, 2024
…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 <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit a1b990d)
Contributor
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
May 1, 2024
…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":"melissa.alvarez@elastic.co"},"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 <42973632+kibanamachine@users.noreply.github.com>","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 <42973632+kibanamachine@users.noreply.github.com>","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 <42973632+kibanamachine@users.noreply.github.com>","sha":"a1b990ddd6a58519ecf6da55260de22c21aa0055"}}]}] BACKPORT--> Co-authored-by: Melissa Alvarez <melissa.alvarez@elastic.co>
TinLe
added a commit
to TinLe/kibana
that referenced
this pull request
May 1, 2024
* 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) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
focusRangewas 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.