[Discover] Reset layout on profile change#265174
Conversation
|
/ci |
💔 Build Failed
Failed CI StepsMetrics [docs]Async chunks
Historycc @davismcphee |
e281bb5 to
d657e24
Compare
|
/ci |
Catch flakiness early (recommended)Recommended before merge: run the flaky test runner against this PR to catch flakiness early. A new Trigger a run with the Flaky Test Runner UI or post this comment on the PR: Share feedback in the #appex-qa channel. Posted via Macroscope — Flaky Test Runner nudge |
4fff6de to
a264467
Compare
|
/ci |
|
/flaky ftrConfig:src/platform/test/functional/apps/discover/group1/config.ts:30 |
Flaky Test Runner✅ Build triggered - kibana-flaky-test-suite-runner#11957
|
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#11957[✅] src/platform/test/functional/apps/discover/group1/config.ts: 30/30 tests passed. |
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
jughosta
left a comment
There was a problem hiding this comment.
Tested the mentioned cases and they LGTM 👍
| {chartVisible && ( | ||
| <> | ||
| <section | ||
| ref={(element) => (chartRef.current.element = element)} |
There was a problem hiding this comment.
Looks like we lost the logic of returning the focus to the chart.
There was a problem hiding this comment.
Just double checked again in main to be sure, but looks like it's been dead code for a long time now (likely since we added renderCustomChartToggleActions in #171638). We could consider making it work again with our current approach, but I guess it hasn't been missed so far at least.
| hideChart: getChartHidden(services.storage, 'discover'), | ||
| hideTable: getTableHidden(services.storage, 'discover'), |
There was a problem hiding this comment.
What if in the URL the states were different?
There was a problem hiding this comment.
This doesn't run on init (!isFirstResolution) so it'll use URL state there, only on fetch when switching profiles to one that hasn't been resolved previously. So I think we're good here, but please let me know if I misunderstood.
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Unknown metric groupsAPI count
History
cc @davismcphee |
|
Starting backport for target branches: 9.4 https://github.com/elastic/kibana/actions/runs/25078582140 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `9.4`: - [[Discover] Reset layout on profile change (#265174)](#265174) <!--- Backport version: 11.0.2 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Davis McPhee","email":"davis.mcphee@elastic.co"},"sourceCommit":{"committedDate":"2026-04-28T21:27:08Z","message":"[Discover] Reset layout on profile change (#265174)\n\n## Summary\n\nThis PR fixes the layout carry-over that happens when Discover switches\nbetween profiles. Currently when resolving the metrics profile first and\nthen switching to logs, the collapsed table state from metrics leaks\ninto logs even though logs doesn't define that as a profile default:\n<img\nsrc=\"https://github.com/user-attachments/assets/80a7280c-f2df-4580-83fe-c9c7fd0d769f\"\n/>\n\nNow Discover treats initial profile resolution differently from later\nprofile switches. On profile change, Discover restores saved state for\nthat profile when it exists, otherwise it resets layout state back to\nthe shared local storage values before applying profile defaults. This\nkeeps metrics defaults intact while preventing logs from inheriting the\ntable layout when switching from metrics.\n\nThe PR also moves layout local storage helpers to `@kbn/discover-utils`,\nsimplifies how panel toggle actions are wired into unified histogram,\nand adds test coverage for the new layout reset behaviour.\n\n### Testing notes\n\nSetup:\n- Ingest some logs and metrics, e.g. with `node scripts/synthtrace\nlogs_and_metrics.ts`.\n- Create a new o11y space in Kibana.\n- Use a clean local storage (e.g. private window).\n\nThe table should shown in this case (same as before):\n1. Start with `FROM logs-*` (by refreshing the page once you run that\nquery)\n2. `TS metrics-*`\n3. `FROM logs-*`\n\nThe table should now also be shown in this case (new to this PR):\n1. Start with `TS metrics-*` (by refreshing the page once you run that\nquery)\n2. `FROM logs-*`\n\nResolves #266092.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"09769b0ba1f9e46e84dedbb7e89b2a0bd0984a81","branchLabelMapping":{"^v9.5.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:DataDiscovery","backport:version","v9.4.0","v9.5.0"],"title":"[Discover] Reset layout on profile change","number":265174,"url":"https://github.com/elastic/kibana/pull/265174","mergeCommit":{"message":"[Discover] Reset layout on profile change (#265174)\n\n## Summary\n\nThis PR fixes the layout carry-over that happens when Discover switches\nbetween profiles. Currently when resolving the metrics profile first and\nthen switching to logs, the collapsed table state from metrics leaks\ninto logs even though logs doesn't define that as a profile default:\n<img\nsrc=\"https://github.com/user-attachments/assets/80a7280c-f2df-4580-83fe-c9c7fd0d769f\"\n/>\n\nNow Discover treats initial profile resolution differently from later\nprofile switches. On profile change, Discover restores saved state for\nthat profile when it exists, otherwise it resets layout state back to\nthe shared local storage values before applying profile defaults. This\nkeeps metrics defaults intact while preventing logs from inheriting the\ntable layout when switching from metrics.\n\nThe PR also moves layout local storage helpers to `@kbn/discover-utils`,\nsimplifies how panel toggle actions are wired into unified histogram,\nand adds test coverage for the new layout reset behaviour.\n\n### Testing notes\n\nSetup:\n- Ingest some logs and metrics, e.g. with `node scripts/synthtrace\nlogs_and_metrics.ts`.\n- Create a new o11y space in Kibana.\n- Use a clean local storage (e.g. private window).\n\nThe table should shown in this case (same as before):\n1. Start with `FROM logs-*` (by refreshing the page once you run that\nquery)\n2. `TS metrics-*`\n3. `FROM logs-*`\n\nThe table should now also be shown in this case (new to this PR):\n1. Start with `TS metrics-*` (by refreshing the page once you run that\nquery)\n2. `FROM logs-*`\n\nResolves #266092.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"09769b0ba1f9e46e84dedbb7e89b2a0bd0984a81"}},"sourceBranch":"main","suggestedTargetBranches":["9.4"],"targetPullRequestStates":[{"branch":"9.4","label":"v9.4.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.5.0","branchLabelMappingKey":"^v9.5.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/265174","number":265174,"mergeCommit":{"message":"[Discover] Reset layout on profile change (#265174)\n\n## Summary\n\nThis PR fixes the layout carry-over that happens when Discover switches\nbetween profiles. Currently when resolving the metrics profile first and\nthen switching to logs, the collapsed table state from metrics leaks\ninto logs even though logs doesn't define that as a profile default:\n<img\nsrc=\"https://github.com/user-attachments/assets/80a7280c-f2df-4580-83fe-c9c7fd0d769f\"\n/>\n\nNow Discover treats initial profile resolution differently from later\nprofile switches. On profile change, Discover restores saved state for\nthat profile when it exists, otherwise it resets layout state back to\nthe shared local storage values before applying profile defaults. This\nkeeps metrics defaults intact while preventing logs from inheriting the\ntable layout when switching from metrics.\n\nThe PR also moves layout local storage helpers to `@kbn/discover-utils`,\nsimplifies how panel toggle actions are wired into unified histogram,\nand adds test coverage for the new layout reset behaviour.\n\n### Testing notes\n\nSetup:\n- Ingest some logs and metrics, e.g. with `node scripts/synthtrace\nlogs_and_metrics.ts`.\n- Create a new o11y space in Kibana.\n- Use a clean local storage (e.g. private window).\n\nThe table should shown in this case (same as before):\n1. Start with `FROM logs-*` (by refreshing the page once you run that\nquery)\n2. `TS metrics-*`\n3. `FROM logs-*`\n\nThe table should now also be shown in this case (new to this PR):\n1. Start with `TS metrics-*` (by refreshing the page once you run that\nquery)\n2. `FROM logs-*`\n\nResolves #266092.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [x] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"09769b0ba1f9e46e84dedbb7e89b2a0bd0984a81"}}]}] BACKPORT--> Co-authored-by: Lukas Olson <lukas@elastic.co>
Summary
This PR fixes the layout carry-over that happens when Discover switches between profiles. Currently when resolving the metrics profile first and then switching to logs, the collapsed table state from metrics leaks into logs even though logs doesn't define that as a profile default:

Now Discover treats initial profile resolution differently from later profile switches. On profile change, Discover restores saved state for that profile when it exists, otherwise it resets layout state back to the shared local storage values before applying profile defaults. This keeps metrics defaults intact while preventing logs from inheriting the table layout when switching from metrics.
The PR also moves layout local storage helpers to
@kbn/discover-utils, simplifies how panel toggle actions are wired into unified histogram, and adds test coverage for the new layout reset behaviour.Testing notes
Setup:
node scripts/synthtrace logs_and_metrics.ts.The table should shown in this case (same as before):
FROM logs-*(by refreshing the page once you run that query)TS metrics-*FROM logs-*The table should now also be shown in this case (new to this PR):
TS metrics-*(by refreshing the page once you run that query)FROM logs-*Resolves #266092.
Checklist
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesbackport:*labels.