-
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
[UnifiedDocViewer] Filter on field name and value in expanded document view #192299
[UnifiedDocViewer] Filter on field name and value in expanded document view #192299
Conversation
/ci |
…to 163275-search-by-field-value
…eld-value # Conflicts: # packages/kbn-unified-data-table/src/types.ts
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
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.
LGTM for the Threat Hunting Investigations team
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.
It looks good from the design perspective.
Note: With the highlighting we do on fields we also add a background color, but with values we can't just highlight only the exact letters, and adding a background color will create too much visual noise. Highlighting only the exact letters would be better, as we do with the Fields but we understand the technical limitations to do so now.
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.
I see Laura already reviewed... I'll add her to kibana-design.
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.
Still looking through the code, but in my testing I noticed a couple of strange behaviors:
When I search for something more than a few characters long, I can swap out the first character without it actually applying to the results:
Screen.Recording.2024-09-17.at.3.33.12.PM.mov
Also, when I type *
once, it doesn't affect results, but if I add **
then it appears to highlight the entire entry:
Screen.Recording.2024-09-17.at.3.35.19.PM.mov
Contrast this with the behaviors on main (first character matters, only single *
is required to highlight the whole field):
Screen.Recording.2024-09-17.at.3.36.03.PM.mov
isDetails={isDetails} | ||
isHighLighted={Boolean( |
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.
isHighLighted={Boolean( | |
isHighlighted={Boolean( |
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.
Updated via b20c54e
Hi @lukasolson, Thanks for reviewing and testing! I think the points you are describing are working as intended according to #186894 and the length of the entered search terms. cc @mbondyra In this PR I (hopefully) didn't change the filtering by field name, only added filtering by value. And for value filtering I used simply |
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.
LGTM! Made just one very minor nit below.
|
||
// format as html in a lazy way | ||
public get formattedAsHtml(): string | undefined { | ||
if (!this.#isFormattedAsHtml) { |
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.
Nit: Do we need a separate variable for this.#isFormattedAsHtml
(and this.#isFormattedAsText
) or can we just check if this.#formattedAsHtml
is 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.
@lukasolson I added it so we don't run the formatter again if it returns an empty result. Would it be okay to keep it this way?
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.
Makes sense, yeah we can leave as is.
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @jughosta |
…t view (elastic#192299) - Closes elastic#163275 ## Summary With this PR users can search by field value (raw and formatted) in DocViewer flyout. <img width="569" alt="Screenshot 2024-09-06 at 20 49 57" src="https://github.com/user-attachments/assets/27e89017-4b8f-437b-9e00-b9a24eb88184"> <img width="544" alt="Screenshot 2024-09-06 at 20 50 09" src="https://github.com/user-attachments/assets/5086af87-df97-4fdb-84c3-f30c547678e0"> <img width="536" alt="Screenshot 2024-09-12 at 18 45 11" src="https://github.com/user-attachments/assets/abdfe31d-85a5-41de-bf05-88ecf0ac2b18"> ### Checklist - [x] [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 - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 7b76160)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ocument view (#192299) (#194094) # Backport This will backport the following commits from `main` to `8.x`: - [[UnifiedDocViewer] Filter on field name and value in expanded document view (#192299)](#192299) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Julia Rechkunova","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-26T07:11:34Z","message":"[UnifiedDocViewer] Filter on field name and value in expanded document view (#192299)\n\n- Closes https://github.com/elastic/kibana/issues/163275\r\n\r\n## Summary\r\n\r\nWith this PR users can search by field value (raw and formatted) in\r\nDocViewer flyout.\r\n\r\n<img width=\"569\" alt=\"Screenshot 2024-09-06 at 20 49 57\"\r\nsrc=\"https://github.com/user-attachments/assets/27e89017-4b8f-437b-9e00-b9a24eb88184\">\r\n<img width=\"544\" alt=\"Screenshot 2024-09-06 at 20 50 09\"\r\nsrc=\"https://github.com/user-attachments/assets/5086af87-df97-4fdb-84c3-f30c547678e0\">\r\n<img width=\"536\" alt=\"Screenshot 2024-09-12 at 18 45 11\"\r\nsrc=\"https://github.com/user-attachments/assets/abdfe31d-85a5-41de-bf05-88ecf0ac2b18\">\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [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- [x] 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: kibanamachine <[email protected]>","sha":"7b76160d68af1535c14343725c8280933ae0e937","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","Team:DataDiscovery","backport:prev-minor","Project:OneDiscover"],"title":"[UnifiedDocViewer] Filter on field name and value in expanded document view","number":192299,"url":"https://github.com/elastic/kibana/pull/192299","mergeCommit":{"message":"[UnifiedDocViewer] Filter on field name and value in expanded document view (#192299)\n\n- Closes https://github.com/elastic/kibana/issues/163275\r\n\r\n## Summary\r\n\r\nWith this PR users can search by field value (raw and formatted) in\r\nDocViewer flyout.\r\n\r\n<img width=\"569\" alt=\"Screenshot 2024-09-06 at 20 49 57\"\r\nsrc=\"https://github.com/user-attachments/assets/27e89017-4b8f-437b-9e00-b9a24eb88184\">\r\n<img width=\"544\" alt=\"Screenshot 2024-09-06 at 20 50 09\"\r\nsrc=\"https://github.com/user-attachments/assets/5086af87-df97-4fdb-84c3-f30c547678e0\">\r\n<img width=\"536\" alt=\"Screenshot 2024-09-12 at 18 45 11\"\r\nsrc=\"https://github.com/user-attachments/assets/abdfe31d-85a5-41de-bf05-88ecf0ac2b18\">\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [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- [x] 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: kibanamachine <[email protected]>","sha":"7b76160d68af1535c14343725c8280933ae0e937"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192299","number":192299,"mergeCommit":{"message":"[UnifiedDocViewer] Filter on field name and value in expanded document view (#192299)\n\n- Closes https://github.com/elastic/kibana/issues/163275\r\n\r\n## Summary\r\n\r\nWith this PR users can search by field value (raw and formatted) in\r\nDocViewer flyout.\r\n\r\n<img width=\"569\" alt=\"Screenshot 2024-09-06 at 20 49 57\"\r\nsrc=\"https://github.com/user-attachments/assets/27e89017-4b8f-437b-9e00-b9a24eb88184\">\r\n<img width=\"544\" alt=\"Screenshot 2024-09-06 at 20 50 09\"\r\nsrc=\"https://github.com/user-attachments/assets/5086af87-df97-4fdb-84c3-f30c547678e0\">\r\n<img width=\"536\" alt=\"Screenshot 2024-09-12 at 18 45 11\"\r\nsrc=\"https://github.com/user-attachments/assets/abdfe31d-85a5-41de-bf05-88ecf0ac2b18\">\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [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- [x] 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: kibanamachine <[email protected]>","sha":"7b76160d68af1535c14343725c8280933ae0e937"}}]}] BACKPORT--> Co-authored-by: Julia Rechkunova <[email protected]>
Summary
With this PR users can search by field value (raw and formatted) in DocViewer flyout.
Checklist