-
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
[Discover] Add container for internal state #144149
Changes from 44 commits
5ab45e2
2557d20
67c1d23
838b95f
d74ab22
43c06f9
c2fe3f5
9404801
5e784c3
badaf79
0fd1b90
a01df77
2f482d7
ad88bff
95483dd
5843385
e8a8d9e
fc45043
16c1d01
9914d6f
80bb56c
9ddf6be
2fb845b
16f92b3
f49e221
9b8e176
4bbb236
ca718fc
27377bb
383ac1d
9d7a73e
5cab758
43bb325
ad6169b
c209394
4c929b8
7027786
857afed
74f5058
5091f46
8b75e85
6fe5cd2
06c6bb6
475033b
8c8e92e
177e1b3
ce9cc2a
fb71953
1ab6046
d0bc3d4
ca6926a
1c73ea9
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 |
---|---|---|
|
@@ -16,6 +16,7 @@ import { | |
import { FormattedMessage } from '@kbn/i18n-react'; | ||
import { DataView } from '@kbn/data-views-plugin/public'; | ||
import { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public'; | ||
import { useAppStateSelector } from '../../services/discover_app_state_container'; | ||
import { useDiscoverServices } from '../../../../hooks/use_discover_services'; | ||
import { DocViewFilterFn } from '../../../../services/doc_views/doc_views_types'; | ||
import { DiscoverGrid } from '../../../../components/discover_grid/discover_grid'; | ||
|
@@ -29,7 +30,7 @@ import { | |
} from '../../../../../common'; | ||
import { useColumns } from '../../../../hooks/use_data_grid_columns'; | ||
import { DataDocuments$, RecordRawType } from '../../hooks/use_saved_search'; | ||
import { AppState, GetStateReturn } from '../../services/discover_state'; | ||
import { DiscoverStateContainer } from '../../services/discover_state'; | ||
import { useDataState } from '../../hooks/use_data_state'; | ||
import { DocTableInfinite } from '../../../../components/doc_table/doc_table_infinite'; | ||
import { DocumentExplorerCallout } from '../document_explorer_callout'; | ||
|
@@ -45,9 +46,9 @@ const DataGridMemoized = React.memo(DiscoverGrid); | |
// export needs for testing | ||
export const onResize = ( | ||
colSettings: { columnId: string; width: number }, | ||
stateContainer: GetStateReturn, | ||
state: AppState | ||
stateContainer: DiscoverStateContainer | ||
) => { | ||
const state = stateContainer.appState.getState(); | ||
const grid = { ...(state.grid || {}) }; | ||
const newColumns = { ...(grid.columns || {}) }; | ||
newColumns[colSettings.columnId] = { | ||
|
@@ -64,7 +65,6 @@ function DiscoverDocumentsComponent({ | |
onAddFilter, | ||
savedSearch, | ||
setExpandedDoc, | ||
state, | ||
stateContainer, | ||
onFieldEdited, | ||
}: { | ||
|
@@ -75,49 +75,71 @@ function DiscoverDocumentsComponent({ | |
onAddFilter?: DocViewFilterFn; | ||
savedSearch: SavedSearch; | ||
setExpandedDoc: (doc?: DataTableRecord) => void; | ||
state: AppState; | ||
stateContainer: GetStateReturn; | ||
stateContainer: DiscoverStateContainer; | ||
onFieldEdited?: () => void; | ||
}) { | ||
const { capabilities, dataViews, uiSettings } = useDiscoverServices(); | ||
const [query, sort, rowHeight, rowsPerPage, grid, columns, index] = useAppStateSelector( | ||
(state) => { | ||
return [ | ||
state.query, | ||
state.sort, | ||
state.rowHeight, | ||
state.rowsPerPage, | ||
state.grid, | ||
state.columns, | ||
state.index, | ||
]; | ||
} | ||
); | ||
|
||
const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]); | ||
const hideAnnouncements = useMemo(() => uiSettings.get(HIDE_ANNOUNCEMENTS), [uiSettings]); | ||
const isLegacy = useMemo(() => uiSettings.get(DOC_TABLE_LEGACY), [uiSettings]); | ||
const sampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING), [uiSettings]); | ||
|
||
const documentState = useDataState(documents$); | ||
const isLoading = documentState.fetchStatus === FetchStatus.LOADING; | ||
const isPlainRecord = useMemo( | ||
() => getRawRecordType(state.query) === RecordRawType.PLAIN, | ||
[state.query] | ||
); | ||
// this is needed to prevent data view pushing onSort because there has been a state change with a new data view | ||
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. I'd like to walk through the code so you can show me this. It sounds like a good test of whether I understand this code. Also, it sounds like a flaw in the state model. 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. I've improved documentation why this is written that way .. generally it's fixing a situation when switching between data views with different timestamps, by showing the loading state for this cases. Not saying this is ideal. I've improved the documentation for this, and happy to have a chat about it for sure 👍 |
||
// but the data view in internal state was not set. due to the change of the timefield, this can lead to a change | ||
// of state, EuiDataGrid pushing an onSort event, because the next timefield is already used a sorting param | ||
// but it's not an available column | ||
const dataViewChanged = index && dataView.id && index !== dataView.id; | ||
const isPlainRecord = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); | ||
const rows = useMemo(() => documentState.result || [], [documentState.result]); | ||
|
||
const { columns, onAddColumn, onRemoveColumn, onMoveColumn, onSetColumns } = useColumns({ | ||
const { | ||
columns: currentColumns, | ||
onAddColumn, | ||
onRemoveColumn, | ||
onMoveColumn, | ||
onSetColumns, | ||
} = useColumns({ | ||
capabilities, | ||
config: uiSettings, | ||
dataView, | ||
dataViews, | ||
setAppState: stateContainer.setAppState, | ||
state, | ||
useNewFieldsApi, | ||
columns, | ||
sort, | ||
}); | ||
|
||
const onResizeDataGrid = useCallback( | ||
(colSettings) => onResize(colSettings, stateContainer, state), | ||
[stateContainer, state] | ||
(colSettings) => onResize(colSettings, stateContainer), | ||
[stateContainer] | ||
); | ||
|
||
const onUpdateRowsPerPage = useCallback( | ||
(rowsPerPage: number) => { | ||
stateContainer.setAppState({ rowsPerPage }); | ||
(nextRowsPerPage: number) => { | ||
stateContainer.setAppState({ rowsPerPage: nextRowsPerPage }); | ||
}, | ||
[stateContainer] | ||
); | ||
|
||
const onSort = useCallback( | ||
(sort: string[][]) => { | ||
stateContainer.setAppState({ sort }); | ||
(nextSort: string[][]) => { | ||
stateContainer.setAppState({ sort: nextSort }); | ||
}, | ||
[stateContainer] | ||
); | ||
|
@@ -138,8 +160,9 @@ function DiscoverDocumentsComponent({ | |
); | ||
|
||
if ( | ||
(!documentState.result || documentState.result.length === 0) && | ||
documentState.fetchStatus === FetchStatus.LOADING | ||
dataViewChanged || | ||
((!documentState.result || documentState.result.length === 0) && | ||
documentState.fetchStatus === FetchStatus.LOADING) | ||
) { | ||
return ( | ||
<div className="dscDocuments__loading"> | ||
|
@@ -163,10 +186,10 @@ function DiscoverDocumentsComponent({ | |
<> | ||
{!hideAnnouncements && <DocumentExplorerCallout />} | ||
<DocTableInfiniteMemoized | ||
columns={columns} | ||
columns={currentColumns} | ||
dataView={dataView} | ||
rows={rows} | ||
sort={state.sort || []} | ||
sort={sort || []} | ||
isLoading={isLoading} | ||
searchDescription={savedSearch.description} | ||
sharedItemTitle={savedSearch.title} | ||
|
@@ -190,30 +213,30 @@ function DiscoverDocumentsComponent({ | |
<div className="dscDiscoverGrid"> | ||
<DataGridMemoized | ||
ariaLabelledBy="documentsAriaLabel" | ||
columns={columns} | ||
columns={currentColumns} | ||
expandedDoc={expandedDoc} | ||
dataView={dataView} | ||
isLoading={isLoading} | ||
rows={rows} | ||
sort={(state.sort as SortOrder[]) || []} | ||
sort={(sort as SortOrder[]) || []} | ||
sampleSize={sampleSize} | ||
searchDescription={savedSearch.description} | ||
searchTitle={savedSearch.title} | ||
setExpandedDoc={!isPlainRecord ? setExpandedDoc : undefined} | ||
showTimeCol={showTimeCol} | ||
settings={state.grid} | ||
settings={grid} | ||
onAddColumn={onAddColumn} | ||
onFilter={onAddFilter as DocViewFilterFn} | ||
onRemoveColumn={onRemoveColumn} | ||
onSetColumns={onSetColumns} | ||
onSort={!isPlainRecord ? onSort : undefined} | ||
onResize={onResizeDataGrid} | ||
useNewFieldsApi={useNewFieldsApi} | ||
rowHeightState={state.rowHeight} | ||
rowHeightState={rowHeight} | ||
onUpdateRowHeight={onUpdateRowHeight} | ||
isSortEnabled={!isPlainRecord} | ||
isPlainRecord={isPlainRecord} | ||
rowsPerPageState={state.rowsPerPage} | ||
rowsPerPageState={rowsPerPage} | ||
onUpdateRowsPerPage={onUpdateRowsPerPage} | ||
onFieldEdited={onFieldEdited} | ||
savedSearchId={savedSearch.id} | ||
|
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.
There is
appStateContainer
in this definition. Is it correct?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's wrong, thx and changed, however it doesn't change the current broken state of storybook, it needs some ❤️ to unbreak it