-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Discover] Fix document explorer cell popover rendering #123194
[Discover] Fix document explorer cell popover rendering #123194
Conversation
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
merge conflict between base and head |
…8-fix-document-explorer-rendering
@elasticmachine merge upstream |
src/plugins/discover/public/components/discover_grid/get_render_cell_value.tsx
Show resolved
Hide resolved
src/plugins/discover/public/components/discover_grid/get_render_cell_value.tsx
Show resolved
Hide resolved
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @kertal |
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.
Tested using a-la-carte. LTGM!
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.
Tested this in Chrome on Mac OSX, works as expected. I don't fully understand where the motivation for the change is coming from, I feel the JSON view is nicer, but I trust you that this was the correct decision.
) | ||
.join(', '); | ||
const pairs = highlights[key] ? highlightPairs : sourcePairs; | ||
if (displayKey) { |
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 know I wrote this code, but omg is it horrible 😅
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.
so I've got good news for your, since I refactured the structure of the file, but did not touch the function's logic ... now technically: I wrote this code :)
Summary
This PR fixes the way values are rendered in the cell popover when fetching using the fields API (which is the default). With this PR the JSON viewer is just shown when rendering the
Document
columns, and for rendering top level objects (likeproducts
of our ecommerce demo dataset) when fetching using_source
. In all other cases the value the field formatter returns is displayed.Before:
After:
Fixes #118162
Checklist