Skip to content
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] Change surrounding documents to use EUIDataGrid #97126

Closed
shaunmcgough opened this issue Apr 14, 2021 · 2 comments · Fixed by #99447
Closed

[Discover] Change surrounding documents to use EUIDataGrid #97126

shaunmcgough opened this issue Apr 14, 2021 · 2 comments · Fixed by #99447
Labels
enhancement New value added to drive a business result Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@shaunmcgough
Copy link

Currently, the EUIDataGrid is not part of View Surrounding Documents. It needs to be.
The MVP here is to swap out the old table and old code, and use EUIDataGrid.
Nice to haves are column sorts, filters, full-screen, context (where you came from), etc.
We should not lose any functionality, so comparisons, and a flyout are on the table for this.

@shaunmcgough shaunmcgough added Feature:Discover Discover Application enhancement New value added to drive a business result Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Apr 14, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@kertal
Copy link
Member

kertal commented Apr 30, 2021

Implementation notes:

The used EUI component

https://elastic.github.io/eui/#/tabular-content/data-grid

The context layout component

(https://github.com/elastic/kibana/blob/master/src/plugins/discover/public/application/components/context_app/context_app_legacy.tsx) needs to be extended to support both grids similar to the way it is done in the main layout component.

{isLegacy && rows && rows.length && (
<DocTableLegacyMemoized
columns={columns}
indexPattern={indexPattern}
minimumVisibleRows={minimumVisibleRows}
rows={rows}
sort={state.sort || []}
searchDescription={opts.savedSearch.description}
searchTitle={opts.savedSearch.lastSavedTitle}
onAddColumn={onAddColumn}
onBackToTop={onBackToTop}
onFilter={onAddFilter}
onMoveColumn={onMoveColumn}
onRemoveColumn={onRemoveColumn}
onSort={onSort}
sampleSize={opts.sampleSize}
useNewFieldsApi={useNewFieldsApi}
/>
)}
{!isLegacy && rows && rows.length && (
<div className="dscDiscoverGrid">
<DataGridMemoized
ariaLabelledBy="documentsAriaLabel"
columns={columns}
expandedDoc={expandedDoc}
indexPattern={indexPattern}
isLoading={fetchStatus === 'loading'}
rows={rows}
sort={(state.sort as SortPairArr[]) || []}
sampleSize={opts.sampleSize}
searchDescription={opts.savedSearch.description}
searchTitle={opts.savedSearch.lastSavedTitle}
setExpandedDoc={setExpandedDoc}
showTimeCol={
!config.get('doc_table:hideTimeColumn', false) &&
!!indexPattern.timeFieldName
}
services={services}
settings={state.grid}
onAddColumn={onAddColumn}
onFilter={onAddFilter as DocViewFilterFn}
onRemoveColumn={onRemoveColumn}
onSetColumns={onSetColumns}
onSort={onSort}
onResize={onResize}
useNewFieldsApi={useNewFieldsApi}
/>

which grid is displayed depends on doc_table:legacy

The discover grid component

https://github.com/elastic/kibana/blob/master/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx

  • Needs to be extended to highlight an anchor document, that's the document selected by the user to view in context with previous and following doc
  • Needs to be extended to offer no sorting (we could think of adding sorting if it makes sense later on, but the way it works today, is sorting by timefield)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants