-
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
Integrating EuiDataGrid into Discover #51531
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
420b50c
to
7abb1b1
Compare
💔 Build Failed
|
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.
💔 Build Failed
|
For the cell popover with the 2 filter options, here is a screenshot from this prototype: I will work on the flyout portion in Figma to see about cleaning up some of it. |
src/legacy/core_plugins/kibana/public/discover/angular/directives/table.tsx
Outdated
Show resolved
Hide resolved
@myasonik nice progress on the data grid
*The expanded view always to only shows the expanded view without the ability to use the main Discover screen. The existing discover allows users to open and interact with discover, expanding multiple documents at once. I understand it is difficult to replicate it, however, we should at least let users see the discover main view while looking at the expanded document. This view also can use less of the screen with a smaller width
Happy tp play with it again whenever it makes sense, I know there is a lot of work thank you @myasonik |
💔 Build Failed
History
To update your PR or re-run it, just comment with: |
5440b05
to
e6671e5
Compare
@elasticmachine merge upstream |
merge conflict between base and head |
💔 Build FailedTest FailuresKibana Pipeline / kibana-oss-agent / Accessibility Tests.test/accessibility/apps/discover·ts.Discover main viewStandard Out
Stack Trace
Kibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/context/_discover_navigation·js.context app context link in discover "before all" hook for "should open the context view with the selected document as anchor"Standard Out
Stack Trace
Kibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/context/_discover_navigation·js.context app context link in discover "before all" hook for "should open the context view with the selected document as anchor"Standard Out
Stack Trace
and 5 more failures, only showing the first 3. History
To update your PR or re-run it, just comment with: |
} else { | ||
setVisibleColumns(dataGridColumns.map(obj => obj.id)); | ||
} | ||
}, [dataGridColumns.length]); // eslint-disable-line react-hooks/exhaustive-deps |
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, when adding the first column, there is no change in length of columns, it switches between ['_source']
and ['myLovelyFirstColumn']
, so the visible columns aren't set. The following code should fix this (using isEqual
of lodash)
useEffect(() => {
// every time a column is added, make it visible
if (!mounted.current) {
mounted.current = true;
} else {
const newColumns = dataGridColumns.map(obj => obj.id);
if (!isEqual(newColumns, visibleColumns)) {
setVisibleColumns(newColumns);
}
}
}, [dataGridColumns, visibleColumns]);
classing in favor of #67259 |
Summary
Gets
EuiDataGrid
into Discover! 🎉Resolves #737
Resolves #38982
Resolves #58204
Resolves #56529
Resolves #54345
Resolves #696
Resolves #8706
Part of #38646
TODO
🐲 There be dragons here still!
EuiFlyout
intable.tsx
should probably be broken out into its own file_score
which isn't automatically added to the table. We either need to show that in the UI somehow or add_score
to be automatically added and show the sort as normal. (Related Discover: Sort automatically by _score if it makes sense #54362) [Adding it to the UI is a nightmare. Instead, rolling with a simpler change of removing_score
sort when a new sort is added.]Pain points with EUI
Tracked in elastic/projects/323
To further investigate
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers