-
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
[RAC] EuiDataGrid pagination #109269
[RAC] EuiDataGrid pagination #109269
Changes from 8 commits
d66f5c2
98d4125
091b592
06d112c
0b642ed
2efd63f
5a17864
dd2cc6f
567afb4
55d6707
abd30d3
372abfe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { useState, useLayoutEffect } from 'react'; | ||
|
||
// That could be different from security and observability. Get it as parameter? | ||
const INITIAL_DATA_GRID_HEIGHT = 967; | ||
|
||
// It will recalculate DataGrid height after this time interval. | ||
const TIME_INTERVAL = 50; | ||
|
||
/** | ||
* You are probably asking yourself "Why 3?". But that is the wrong mindset. You should be asking yourself "why not 3?". | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😂 |
||
* 3 (three) is a number, numeral and digit. It is the natural number following 2 and preceding 4, and is the smallest | ||
* odd prime number and the only prime preceding a square number. It has religious or cultural significance in many societies. | ||
*/ | ||
const MAGIC_GAP = 3; | ||
|
||
/** | ||
* HUGE HACK!!! | ||
* DataGrtid height isn't properly calculated when the grid has horizontal scroll. | ||
* https://github.com/elastic/eui/issues/5030 | ||
* | ||
* In order to get around this bug we are calculating `DataGrid` height here and setting it as a prop. | ||
* | ||
* Please delete me and allow DataGrid to calculate its height when the bug is fixed. | ||
*/ | ||
export const useDataGridHeightHack = (pageSize: number, rowCount: number) => { | ||
const [height, setHeight] = useState(INITIAL_DATA_GRID_HEIGHT); | ||
|
||
useLayoutEffect(() => { | ||
setTimeout(() => { | ||
const gridVirtualized = document.querySelector('#body-data-grid .euiDataGrid__virtualized'); | ||
|
||
if ( | ||
gridVirtualized && | ||
gridVirtualized.children[0].clientHeight !== gridVirtualized.clientHeight // check if it has vertical scroll | ||
) { | ||
setHeight( | ||
height + | ||
gridVirtualized.children[0].clientHeight - | ||
gridVirtualized.clientHeight + | ||
MAGIC_GAP | ||
); | ||
} | ||
}, TIME_INTERVAL); | ||
}, [pageSize, rowCount, height]); | ||
|
||
return height; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,10 +66,11 @@ describe('Body', () => { | |
excludedRowRendererIds: [], | ||
id: 'timeline-test', | ||
isSelectAllChecked: false, | ||
isLoading: false, | ||
itemsPerPageOptions: [], | ||
loadingEventIds: [], | ||
loadPage: jest.fn(), | ||
querySize: 25, | ||
pageSize: 25, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the initial amount shown in the table? Would it make sense to have this be at least 50? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @mdefazio , Yes the default size is 50 items in security solutions and observability. This is just a test, we are good 👍 |
||
renderCellValue: TestCellRenderer, | ||
rowRenderers: [], | ||
selectedEventIds: {}, | ||
|
@@ -78,7 +79,6 @@ describe('Body', () => { | |
showCheckboxes: false, | ||
tabType: TimelineTabs.query, | ||
tableView: 'gridView', | ||
totalPages: 1, | ||
totalItems: 1, | ||
leadingControlColumns: [], | ||
trailingControlColumns: [], | ||
|
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.
This function is defined twice (timelines and security solutions). Where can we move it to reuse on both places?
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.
since this is already importing from the timeslines plugin, i think we can import it from there
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.
okay I exported it from timeline public, it is imported by both o11y and securitySol.