[Discover] In-table search #206454

jughosta · 2025-01-13T14:57:45Z

Closes In-Table Search (CTRL+F "like") Functionality to Discover Table #192360

Summary

The default browser Find-in-page does not work great with the grid virtualization and our pagination and it can only find matches in rows which are currently displayed.

This PR adds in-table search support to the grid so users can find matches in all grid rows (up to 500 sample docs/rows by default) and jump between them with "Previous"/"Next" buttons.

The implementation is extracted in a new package @kbn/data-grid-in-table-search. This would allow to use in-table search with EuiDataGrid on other pages of Kibana too.

Cmd+F shortcut is overridden when one of grid elements is in focus otherwise we keep the browser default behaviour.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

…360-in-table-search

stratoula

Looks awesome, even in the inline editing flyout! I really like this feature. I would personally advertise it more (change the release label to feature)

The changes to the ES|QL team codebase is minimal and it works great but I have some questions I would love an answer first.

src/platform/packages/shared/kbn-data-grid-in-table-search/src/in_table_search_control.tsx

stratoula · 2025-01-29T09:18:05Z

src/platform/packages/shared/kbn-data-grid-in-table-search/src/in_table_search_input.tsx

So I type 200 and click enter to navigate to the next one. The number changes but not the highlight. Here is should highlight the 3rd row, no? (when I have selected the column works as expected)

Ah I think it is because it founds the number in hidden columns, this is a bit confusing tbh but I am not sure how we can make it better. Just mentioning it here for visibility

Yeah, if a match is cut off by the cell boundaries then it might be confusing. I added the yellow border to the active cell as a hint.

@jughosta thinking in how to solve this, could we move the content in the cell?

also, could we consider this issue as an edge case? not sure if users will find themselves in this situation too often 🤔

I don't think is an edge case tbh but is not something I consider as a blocker either

@jughosta and I already discussed this limitation and it is not a blocker. Users have the ability to increase row height to see what is matching. Let's launch and collect feedback, if it is a big user pain and the workaround of increasing row height is not enough, then we can take it from there.

Agreed that it may not be an edge case, but also not a blocker. It's a tricky problem, collecting feedback is probably a good first step.

EUI offers a way to override row height for individual rows using the rowHeights option. We may be able to temporarily override to auto when highlighting a cell for "find" to ensure all matches are visible, and backport as a bug fix if it works well. Not sure how that would feel UX wise or if it's oversimplifying though, so just thinking out loud for now.

l-suarez

Checked by design - LGTM

stratoula

ES|QL changes LGTM! I played with it in both Chrome and FF, works great. (I focused on the ES|QL mode and havent tested the dataview mode)

Great work Jul 🙌

dmlemeshko

test/functional/services/data_grid.ts changes LGTM

…360-in-table-search

tonyghiani

Codewise we are only involved for shared ownership on the cell_actions_popover, the rest has been deeply reviewed already.

I left a note regarding the change for the text truncation, but it's not a blocker as logically should work fine and I don't want to block this further.

...d/kbn-discover-contextual-components/src/data_types/logs/components/cell_actions_popover.tsx

michaelolo24 · 2025-01-29T20:39:21Z

...ckages/shared/kbn-unified-data-table/src/components/custom_toolbar/render_custom_toolbar.tsx

@@ -90,18 +91,28 @@ export const internalRenderCustomToolbar = (
        <EuiFlexItem grow={false}>
          <EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
            {Boolean(leftSide) && buttons}
-            {(keyboardShortcutsControl || displayControl || fullScreenControl) && (
+            {Boolean(


super nit: all the boolean calls for each of these can probably be done once in the body of the component and not the render,

True, probably some opportunity for future cleanup in this component.

michaelolo24

Really really awesome feature! Thank you. Will test out enabling this in security as well 😄

elasticmachine · 2025-01-29T22:32:50Z

💚 Build Succeeded

Buildkite Build
Commit: ad91ee5

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cloudSecurityPosture`	775	787	+12
`discover`	1076	1088	+12
`esqlDataGrid`	500	512	+12
`lens`	1797	1809	+12
`logsExplorer`	697	709	+12
`securitySolution`	6625	6637	+12
`slo`	984	996	+12
total			+84

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/data-grid-in-table-search`	-	21	+21

Async chunks

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

id	before	after	diff
`cloudSecurityPosture`	523.7KB	536.8KB	+13.1KB
`discover`	834.1KB	846.9KB	+12.7KB
`esqlDataGrid`	176.2KB	189.4KB	+13.2KB
`securitySolution`	21.4MB	21.4MB	+76.7KB
`slo`	885.8KB	898.5KB	+12.7KB
total			+128.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/data-grid-in-table-search`	-	3	+3

Page load bundle

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

id	before	after	diff
`esqlDataGrid`	9.4KB	9.4KB	+52.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/data-grid-in-table-search`	-	21	+21
`@kbn/unified-data-table`	185	186	+1
total			+22

History

💔 Build #271732 failed 860eff7
💔 Build #271673 failed 90673c8
💔 Build #271660 failed 665122f
💚 Build #271165 succeeded 2ec6cc0
💔 Build #270994 failed f24fbf0

cc @jughosta

davismcphee

Phew, that was a lot to get through and take in at once. But excellent work on this, and great creativity getting it to work for formatted values! A super useful feature, nicely encapsulated in a reusable package 👌 I also like how much you were able to test through RTL vs FTR (seems like there are plenty of tests too).

I tested locally as much as I could, and in the vast majority of cases it worked great! There was a significant performance/rendering hit when pushing the limits by searching 10,000 summary columns, but I imagine this is a rarer case, and it still eventually returned results without falling over. So definitely sufficient for an MVP, with some opportunity to investigate further optimizations later.

I left some comments, but mostly nits and questions, nothing blocking or that needs to be addressed immediately. Otherwise I just noticed a few things when testing that we should consider following up on:

When using the native Chrome find, I can hold down Enter to cycle through matches, while in ours I have to tap repeatedly. It might polish the UX a bit if we did the same.
When I was stepping through matches and encountered one that was truncated, I changed the row height, which triggered a new search and reset me back to 0. We might want to consider if it's possible to keep the user on their current match in this case.
When stepping through matches and reaching the last page, loading more results also triggered a new search and reset me back to 0. Another case where it may be helpful to track the current match, but this seems like it could be trickier.

With that said, I'm merging this thing! 🎉

davismcphee · 2025-01-29T17:03:31Z

src/platform/packages/shared/kbn-data-grid-in-table-search/src/constants.ts

+export const HIGHLIGHT_COLOR = '#e5ffc0'; // TODO: Use a named color token
+export const ACTIVE_HIGHLIGHT_COLOR = '#ffc30e'; // TODO: Use a named color token


Let's create a dedicated issue for tracking this and working with the EUI team to get the tokens.

davismcphee · 2025-01-29T17:07:51Z

src/platform/packages/shared/kbn-data-grid-in-table-search/src/in_table_search_control.tsx

+
+const innerCss = css`
+  .dataGridInTableSearch__matchesCounter {
+    font-variant-numeric: tabular-nums;


Just curious what this is doing? I've never used font-variant-numeric before.

@davismcphee It allows to make numbers the same width (even if the font face is not monospaced) so there are no layout shifts when a digit changes.

davismcphee · 2025-01-29T17:08:38Z

src/platform/packages/shared/kbn-data-grid-in-table-search/src/in_table_search_control.tsx

+
+  .dataGridInTableSearch__input {
+    /* to prevent the width from changing when entering the search term */
+    min-width: 210px;


Nit: could use EUI size var here.

davismcphee · 2025-01-29T17:10:51Z

src/platform/packages/shared/kbn-data-grid-in-table-search/src/in_table_search_control.tsx

+    background: none;
+  }
+
+  /* override borders style only if it's under the custom grid toolbar */


Nit: could make sense to override in Unified Data Table instead if it's specific to that.

It's also specific to the current in-table search layout 😀

davismcphee · 2025-01-29T17:18:12Z

src/platform/packages/shared/kbn-data-grid-in-table-search/src/in_table_search_control.tsx

+            border: 2px solid ${ACTIVE_HIGHLIGHT_COLOR} !important;
+            border-radius: 3px;


Nit: I also wonder if there are / should be EUI vars available for these values.

davismcphee · 2025-01-29T22:25:24Z

...ckages/shared/kbn-unified-data-table/src/components/custom_toolbar/render_custom_toolbar.tsx

@@ -90,18 +91,28 @@ export const internalRenderCustomToolbar = (
        <EuiFlexItem grow={false}>
          <EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
            {Boolean(leftSide) && buttons}
-            {(keyboardShortcutsControl || displayControl || fullScreenControl) && (
+            {Boolean(


True, probably some opportunity for future cleanup in this component.

davismcphee · 2025-01-29T22:36:32Z

src/platform/packages/shared/kbn-unified-data-table/src/components/data_table.tsx

@@ -993,11 +1031,15 @@ export const UnifiedDataTable = ({
            renderCustomToolbar({
              toolbarProps,
              gridProps: {
-                additionalControls,
+                additionalControls:


Our toolbar logic is kinda confusing, and I wonder if we could standardize it more at some point to avoid all the conditionals. Not something for now though.

davismcphee · 2025-01-29T23:24:36Z

src/platform/packages/shared/kbn-data-grid-in-table-search/src/matches/use_find_matches.tsx

+  return null;
+}
+
+let prevJumpTimer: NodeJS.Timeout | null = null;


Could sharing this reference potentially cause issues with multiple searchable grids on screen at once like in dashboards? Doesn't really seem like it from the code, but wanted to raise it regardless.

Potentially yes, it would cancel a jump inside one panel when a jump is triggered in another one. Not likely in reality as changing between panels would take longer when the jump itself.

Having the timeout helps with a flow when users presses prev/next buttons or the shortcut too quickly inside one panel. Less visual noise with the highlights.

davismcphee · 2025-01-29T23:32:08Z

...platform/packages/shared/kbn-data-grid-in-table-search/src/use_data_grid_in_table_search.tsx

+  const pageIndexRef = useRef<number>();
+  pageIndexRef.current = pagination?.pageIndex ?? 0;


Another spot where useLatest could come in handy.

davismcphee · 2025-01-29T23:34:52Z

...platform/packages/shared/kbn-data-grid-in-table-search/src/use_data_grid_in_table_search.tsx

+    const controlsCount = dataGridWrapper
+      ? dataGridWrapper.querySelectorAll('.euiDataGridHeaderCell--controlColumn').length
+      : 0;


Would also be nice to have a cleaner way of knowing this, but again a minor concern.

Looks like I was in the mood of using querySelector in many places here. So old school 😀

kibanamachine · 2025-01-29T23:52:28Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/13042681735

kibanamachine · 2025-01-29T23:57:49Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts You might need to backport the following PRs to 8.x: - [ML] Adds simple flyout based file upload to Search (#206864) - Add a warning callout when deleting managed assets (#207329)

Manual backport

To create the backport manually run:

node scripts/backport --pr 206454

Questions ?

Please refer to the Backport tool documentation

davismcphee · 2025-01-30T02:22:24Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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 `8.x`: - [[Discover] In-table search (#206454)](#206454)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: Julia Rechkunova <[email protected]> Co-authored-by: kibanamachine <[email protected]>

jughosta · 2025-01-30T13:36:55Z

Thanks for the review, @davismcphee!

I created two tickets per your suggestions:

[Discover] Initial implementation

9a50e4c

jughosta added the Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. label Jan 13, 2025

jughosta self-assigned this Jan 13, 2025

jughosta and others added 27 commits January 13, 2025 20:21

[Discover] Some updates

1b17280

[Discover] Calculate matches count

6907398

[Discover] Add scrolling to a visible row

bb3385c

[Discover] Change the approach

1c1a015

[Discover] Improve highlight inserts

949d18c

[Discover] Rename

a0ddc31

[Discover] Add highlights for the next active match

f7286db

[Discover] Exclude Summary column for now completely

27559ec

[Discover] Support Summary column too

dadb590

[Discover] Add debounce for the input

2520df7

[Discover] Revert some redundant changes

40ca7ab

[Discover] Add navigation between pages

9b3b390

[Discover] Improve search input

ee62e43

[Discover] Update data structure

52c9716

[Discover] Support shift+enter

d9a2a5f

[Discover] Skip converting html to string

b432336

[Discover] More updates

0e97a27

[CI] Auto-commit changed files from 'node scripts/notice'

b749d4f

[Discover] Reorganize code

ec80ce4

Merge remote-tracking branch 'origin/192360-in-table-search' into 192…

bdc9644

…360-in-table-search

[Discover] Fix the context issue

85c2ba8

[Discover] Reorganize the code

bb9110f

[Discover] Allow to circle though matches endlessly

286fbec

[Discover] Update styles

e181a0b

[Discover] Update search input interactions

2d9337e

[Discover] Opt-in in table search. Fix rendering.

5361661

[CI] Auto-commit changed files from 'node scripts/notice'

3eed799

stratoula reviewed Jan 29, 2025

View reviewed changes

jughosta added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels Jan 29, 2025

l-suarez approved these changes Jan 29, 2025

View reviewed changes

stratoula approved these changes Jan 29, 2025

View reviewed changes

jughosta requested a review from a team January 29, 2025 11:23

dmlemeshko approved these changes Jan 29, 2025

View reviewed changes

jughosta added 2 commits January 29, 2025 17:27

[Discover] Add more tests

179a6fd

Merge remote-tracking branch 'origin/192360-in-table-search' into 192…

665122f

…360-in-table-search

tonyghiani approved these changes Jan 29, 2025

View reviewed changes

...d/kbn-discover-contextual-components/src/data_types/logs/components/cell_actions_popover.tsx Show resolved Hide resolved

jughosta added 2 commits January 29, 2025 17:59

[Discover] Update tests

90673c8

Merge branch 'main' into 192360-in-table-search

860eff7

michaelolo24 reviewed Jan 29, 2025

View reviewed changes

michaelolo24 approved these changes Jan 29, 2025

View reviewed changes

Merge branch 'main' into 192360-in-table-search

ad91ee5

davismcphee approved these changes Jan 29, 2025

View reviewed changes

davismcphee merged commit 8ffb2ff into elastic:main Jan 29, 2025
8 checks passed

kibanamachine added the v9.0.0 label Jan 29, 2025

davismcphee mentioned this pull request Jan 30, 2025

[8.x] [Discover] In-table search (#206454) #208868

Merged

kibanamachine added the v8.18.0 label Jan 30, 2025

This was referenced Jan 30, 2025

Search among fetched results #70799

Closed

[Discover] Follow ups for in-table search #208939

Open

[Discover] Use named color tokens for in-table highlights #208944

Open

jughosta mentioned this pull request Jan 30, 2025

[Discover] Some cleanups for the new in-table search #208980

Draft

1 task

		export const HIGHLIGHT_COLOR = '#e5ffc0'; // TODO: Use a named color token
		export const ACTIVE_HIGHLIGHT_COLOR = '#ffc30e'; // TODO: Use a named color token

		border: 2px solid ${ACTIVE_HIGHLIGHT_COLOR} !important;
		border-radius: 3px;

		const pageIndexRef = useRef<number>();
		pageIndexRef.current = pagination?.pageIndex ?? 0;

[Discover] In-table search #206454

[Discover] In-table search #206454

Conversation

jughosta commented Jan 13, 2025 • edited by kibanamachine Loading

Summary

Checklist

stratoula left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l-suarez left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

dmlemeshko left a comment

Choose a reason for hiding this comment

tonyghiani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

elasticmachine commented Jan 29, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

davismcphee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Jan 29, 2025

kibanamachine commented Jan 29, 2025

💔 All backports failed

Manual backport

Questions ?

davismcphee commented Jan 30, 2025

💚 All backports created successfully

Questions ?

jughosta commented Jan 30, 2025

jughosta commented Jan 13, 2025 •

edited by kibanamachine

Loading