[Discover] [Metrics] Fix: metrics grid titles do not update on order change#250963
Merged
justinkambic merged 1 commit intoelastic:mainfrom Jan 29, 2026
Conversation
Contributor
|
Pinging @elastic/obs-exploration-team (Team:obs-exploration) |
Contributor
|
Starting backport for target branches: 9.3 |
Contributor
💚 Build Succeeded
Metrics [docs]
|
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jan 29, 2026
…change (elastic#250963) ## Summary Resolves elastic#250687. We are using the index of the items in the list for the grid. This is not specific enough for a `key` value, because when the list is reordered, or items are eliminated, the `index` can remain the same (the 0th item is different, but its index is unchanged), and React will choose not to unmount the existing component. This change reuses the existing `id` value we already generate, which is a composite of the index and the metric's name. This makes the list's render process resilient to item changes where the index value remains the same. ## Verification The linked issue lists several cases where this is broken, and I believe the fix addresses all of them. I have included some recordings. The issue has detailed repro instructions, so I recommend you review it for testing. But essentially, if you load any metrics data and perform filtering or narrowing your ES|QL query to cause the list to re-order without re-rendering the parent content, you should be able to verify the fix against `main`. ### Narrowing ES|QL via WHERE clause  ### Searching for title  ### Checking breakdown -> details matches title  (cherry picked from commit ddeb3ec)
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
Jan 29, 2026
…order change (#250963) (#250988) # Backport This will backport the following commits from `main` to `9.3`: - [[Discover] [Metrics] Fix: metrics grid titles do not update on order change (#250963)](#250963) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Justin Kambic","email":"jk@elastic.co"},"sourceCommit":{"committedDate":"2026-01-29T21:02:34Z","message":"[Discover] [Metrics] Fix: metrics grid titles do not update on order change (#250963)\n\n## Summary\n\nResolves #250687.\n\nWe are using the index of the items in the list for the grid. This is\nnot specific enough for a `key` value, because when the list is\nreordered, or items are eliminated, the `index` can remain the same (the\n0th item is different, but its index is unchanged), and React will\nchoose not to unmount the existing component.\n\nThis change reuses the existing `id` value we already generate, which is\na composite of the index and the metric's name. This makes the list's\nrender process resilient to item changes where the index value remains\nthe same.\n\n## Verification\n\nThe linked issue lists several cases where this is broken, and I believe\nthe fix addresses all of them. I have included some recordings. The\nissue has detailed repro instructions, so I recommend you review it for\ntesting. But essentially, if you load any metrics data and perform\nfiltering or narrowing your ES|QL query to cause the list to re-order\nwithout re-rendering the parent content, you should be able to verify\nthe fix against `main`.\n\n### Narrowing ES|QL via WHERE clause\n\n\n\n\n### Searching for title\n\n\n\n\n### Checking breakdown -> details matches title\n\n\n","sha":"ddeb3ec62fca1486c91c0d6264ccbf6dbf71416a","branchLabelMapping":{"^v9.4.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","backport:version","v9.3.0","Team:obs-exploration","v9.4.0"],"title":"[Discover] [Metrics] Fix: metrics grid titles do not update on order change","number":250963,"url":"https://github.com/elastic/kibana/pull/250963","mergeCommit":{"message":"[Discover] [Metrics] Fix: metrics grid titles do not update on order change (#250963)\n\n## Summary\n\nResolves #250687.\n\nWe are using the index of the items in the list for the grid. This is\nnot specific enough for a `key` value, because when the list is\nreordered, or items are eliminated, the `index` can remain the same (the\n0th item is different, but its index is unchanged), and React will\nchoose not to unmount the existing component.\n\nThis change reuses the existing `id` value we already generate, which is\na composite of the index and the metric's name. This makes the list's\nrender process resilient to item changes where the index value remains\nthe same.\n\n## Verification\n\nThe linked issue lists several cases where this is broken, and I believe\nthe fix addresses all of them. I have included some recordings. The\nissue has detailed repro instructions, so I recommend you review it for\ntesting. But essentially, if you load any metrics data and perform\nfiltering or narrowing your ES|QL query to cause the list to re-order\nwithout re-rendering the parent content, you should be able to verify\nthe fix against `main`.\n\n### Narrowing ES|QL via WHERE clause\n\n\n\n\n### Searching for title\n\n\n\n\n### Checking breakdown -> details matches title\n\n\n","sha":"ddeb3ec62fca1486c91c0d6264ccbf6dbf71416a"}},"sourceBranch":"main","suggestedTargetBranches":["9.3"],"targetPullRequestStates":[{"branch":"9.3","label":"v9.3.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.4.0","branchLabelMappingKey":"^v9.4.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/250963","number":250963,"mergeCommit":{"message":"[Discover] [Metrics] Fix: metrics grid titles do not update on order change (#250963)\n\n## Summary\n\nResolves #250687.\n\nWe are using the index of the items in the list for the grid. This is\nnot specific enough for a `key` value, because when the list is\nreordered, or items are eliminated, the `index` can remain the same (the\n0th item is different, but its index is unchanged), and React will\nchoose not to unmount the existing component.\n\nThis change reuses the existing `id` value we already generate, which is\na composite of the index and the metric's name. This makes the list's\nrender process resilient to item changes where the index value remains\nthe same.\n\n## Verification\n\nThe linked issue lists several cases where this is broken, and I believe\nthe fix addresses all of them. I have included some recordings. The\nissue has detailed repro instructions, so I recommend you review it for\ntesting. But essentially, if you load any metrics data and perform\nfiltering or narrowing your ES|QL query to cause the list to re-order\nwithout re-rendering the parent content, you should be able to verify\nthe fix against `main`.\n\n### Narrowing ES|QL via WHERE clause\n\n\n\n\n### Searching for title\n\n\n\n\n### Checking breakdown -> details matches title\n\n\n","sha":"ddeb3ec62fca1486c91c0d6264ccbf6dbf71416a"}}]}] BACKPORT--> Co-authored-by: Justin Kambic <jk@elastic.co>
kpatticha
reviewed
Jan 30, 2026
| focusedCell.rowIndex === rowIndex && focusedCell.colIndex === colIndex; | ||
|
|
||
| return ( | ||
| <EuiFlexItem key={index}> |
Contributor
There was a problem hiding this comment.
nice catch! @justinkambic what can we do to prevent this from happening again?
Contributor
Author
There was a problem hiding this comment.
Explanation
@kpatticha this is a pretty basic React bug. It's important to make sure that the key is truly a 1:1 value tying the element in the list to the item, so if the list is re-ordered React will know to break the memoize and re-mount.
This bug was already present in the code but didn't happen in practice before, because previously the parent element was re-rendered anytime data changed. When we switched to native Lens it seems it makes better use of memo optimization. Since the index is a value that will never change for the 0th element, and any following element positions that remain when filtering is applied, the titles will never update.
Example: [0: 'a', 1: 'b', 2: 'c', 3: 'd', 4: 'e',] -> apply filtering -> [0: 'b', 1: 'c', 2: 'd']. If we reference only index, the title will still say 'a, b, c' for the remaining items, even though the actual filtered list is 'b, c, d'.
Future prevention
There are a lot of gates that could catch something like this. I'll list a few:
- Seeing a
keyprop added to the return of an iterative call likemapshould be an automatic flag to check when writing/reviewing code, but it's easy to overlook in larger diffs, which is probably the kind of change that merged this in the first place. - An advanced linting rule could catch this bug - we should never reference the map function's
indexas the sole value for thekeyprop. In fact,eslint-plugin-reactactually has the no-array-index rule, which would flag code like this:
// this is the bug
things.map((thing, index) => (
<Hello key={index} />
));
// This plugin is not currently a dependency in Kibana though,
// so it could be a moderate lift to put it in.- E2E tests that check the titles change when filtering is applied would have discovered the bug
- Even RTL unit tests with Lens mocked might have been able to flag the problem
- Lastly, I didn't notice that this showed up when I made the change, so it slipped through a second code review and practical testing. It's easy to miss but we should also do a bit better looking out for this and being very granular with our smoke testing during reviews.
Contributor
There was a problem hiding this comment.
wow, that's a very thoughtful explanation 😮
I hadn't noticed that either when I was using the metrics experience, so I'm wondering if it's worth opening a ticket to add some coverage ? wdyt?
mbondyra
added a commit
to mbondyra/kibana
that referenced
this pull request
Jan 30, 2026
…iew_cps * commit '5f7fec57cb01883038810bd735a0666683b49904': (116 commits) [Security Solution][Attacks/Alerts][Setup and miscellaneous] Advanced setting to control feature visibility (elastic#250157) (elastic#250830) Fix synthtrace `fetch` usage (elastic#250950) [APM] Add Nodes and Edges components and selection logic (elastic#250937) [Docs] Update alerting-settings.md and add serverless value for one parameter (elastic#250842) [Agent Builder] filestore: initial implementation (elastic#250043) [CPS] Support CPS in Vega ESQL (elastic#250693) Adjustments to cascade document esql helpers (elastic#250560) [Security Solutions] Trial Companion - adds ai chat and elastic agent detectors (elastic#250908) [Obs Presentation] Code Scanning Alert Fixes (elastic#250858) [performance] add return and refresh render scenarios to dashboard journeys (elastic#250939) skip failing test suite (elastic#245458) Add Cloud Forwarder onboarding tile to O11y Solution (elastic#250325) [Traces] Remove APM unified trace waterall embeddable registration (elastic#250808) [Discover] [Metrics] Fix: metrics grid titles do not update on order change (elastic#250963) [a11y] Fix Eui modal title annoucment (elastic#250459) [Cloud Security] [Fleet] Add cloud connector access scope for input or package level credential definitions (elastic#250280) [WorkplaceAI] SharePoint Online stack connector (elastic#248737) [Response Ops][Task Manager] Update functions do not handle API key invalidation (elastic#249109) [Osquery] Remove @kbn/timelines-plugin dependency from osquery plugin (elastic#250055) [One Discover][Logs UX] Update OpenTelemetry Semantic Conventions (elastic#250346) ...
hannahbrooks
pushed a commit
to hannahbrooks/kibana
that referenced
this pull request
Jan 30, 2026
…change (elastic#250963) ## Summary Resolves elastic#250687. We are using the index of the items in the list for the grid. This is not specific enough for a `key` value, because when the list is reordered, or items are eliminated, the `index` can remain the same (the 0th item is different, but its index is unchanged), and React will choose not to unmount the existing component. This change reuses the existing `id` value we already generate, which is a composite of the index and the metric's name. This makes the list's render process resilient to item changes where the index value remains the same. ## Verification The linked issue lists several cases where this is broken, and I believe the fix addresses all of them. I have included some recordings. The issue has detailed repro instructions, so I recommend you review it for testing. But essentially, if you load any metrics data and perform filtering or narrowing your ES|QL query to cause the list to re-order without re-rendering the parent content, you should be able to verify the fix against `main`. ### Narrowing ES|QL via WHERE clause  ### Searching for title  ### Checking breakdown -> details matches title 
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
5 participants
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
Resolves #250687.
We are using the index of the items in the list for the grid. This is not specific enough for a
keyvalue, because when the list is reordered, or items are eliminated, theindexcan remain the same (the 0th item is different, but its index is unchanged), and React will choose not to unmount the existing component.This change reuses the existing
idvalue we already generate, which is a composite of the index and the metric's name. This makes the list's render process resilient to item changes where the index value remains the same.Verification
The linked issue lists several cases where this is broken, and I believe the fix addresses all of them. I have included some recordings. The issue has detailed repro instructions, so I recommend you review it for testing. But essentially, if you load any metrics data and perform filtering or narrowing your ES|QL query to cause the list to re-order without re-rendering the parent content, you should be able to verify the fix against
main.Narrowing ES|QL via WHERE clause
Searching for title
Checking breakdown -> details matches title