From b05361e65095d5367f5e5ed667bcc8c92a490232 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Thu, 29 Aug 2024 18:22:17 +0200 Subject: [PATCH] Fixed record table fetch more scroll bug (#6790) Fetch more on the record table was causing a strange bug where it was auto scrolling to the bottom of the newly loaded chunk of rows. This was confusing because we lost our previous position in the record table. With this fix the table doesn't scroll when more rows are loaded. The fetch more row has been moved in the same tbody as the rest of the rows. We now only hide it when there is no more record to fetch. --------- Co-authored-by: Charles Bochet --- .../hooks/useLoadRecordIndexTable.ts | 2 + .../components/RecordTableRows.tsx | 20 ++++++-- .../components/RecordTableBody.tsx | 6 --- .../components/RecordTableBodyEffect.tsx | 29 ++++++++--- .../RecordTableBodyFetchMoreLoader.tsx | 48 +++++++++---------- ...dTableFetchedAllRecordsComponentStateV2.ts | 9 ++++ 6 files changed, 70 insertions(+), 44 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-table/states/hasRecordTableFetchedAllRecordsComponentStateV2.ts diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts index dbf4eada7..369470974 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexTable.ts @@ -53,6 +53,7 @@ export const useLoadRecordIndexTable = (objectNameSingular: string) => { totalCount, fetchMoreRecords, queryStateIdentifier, + hasNextPage, } = useFindManyRecords({ ...params, recordGqlFields, @@ -74,5 +75,6 @@ export const useLoadRecordIndexTable = (objectNameSingular: string) => { fetchMoreRecords, queryStateIdentifier, setRecordTableData, + hasNextPage, }; }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableRows.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableRows.tsx index 40db65bbb..688bfd0f0 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableRows.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableRows.tsx @@ -1,6 +1,7 @@ import { useRecoilValue } from 'recoil'; import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates'; +import { RecordTableBodyFetchMoreLoader } from '@/object-record/record-table/record-table-body/components/RecordTableBodyFetchMoreLoader'; import { RecordTableRow } from '@/object-record/record-table/record-table-row/components/RecordTableRow'; export const RecordTableRows = () => { @@ -8,9 +9,18 @@ export const RecordTableRows = () => { const tableRowIds = useRecoilValue(tableRowIdsState); - return tableRowIds.map((recordId, rowIndex) => { - return ( - - ); - }); + return ( + <> + {tableRowIds.map((recordId, rowIndex) => { + return ( + + ); + })} + + + ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBody.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBody.tsx index 2a6e46c21..c3adb621d 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBody.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBody.tsx @@ -1,21 +1,16 @@ import { useRecoilValue } from 'recoil'; import { RecordTableRows } from '@/object-record/record-table/components/RecordTableRows'; -import { RecordTableContext } from '@/object-record/record-table/contexts/RecordTableContext'; import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates'; import { RecordTableBodyDragDropContext } from '@/object-record/record-table/record-table-body/components/RecordTableBodyDragDropContext'; import { RecordTableBodyDroppable } from '@/object-record/record-table/record-table-body/components/RecordTableBodyDroppable'; -import { RecordTableBodyFetchMoreLoader } from '@/object-record/record-table/record-table-body/components/RecordTableBodyFetchMoreLoader'; import { RecordTableBodyLoading } from '@/object-record/record-table/record-table-body/components/RecordTableBodyLoading'; import { RecordTablePendingRow } from '@/object-record/record-table/record-table-row/components/RecordTablePendingRow'; -import { useContext } from 'react'; export const RecordTableBody = () => { const { tableRowIdsState, isRecordTableInitialLoadingState } = useRecordTableStates(); - const { objectNameSingular } = useContext(RecordTableContext); - const tableRowIds = useRecoilValue(tableRowIdsState); const isRecordTableInitialLoading = useRecoilValue( @@ -32,7 +27,6 @@ export const RecordTableBody = () => { - ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyEffect.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyEffect.tsx index d53d64990..9550dac39 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyEffect.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyEffect.tsx @@ -7,6 +7,7 @@ import { useLoadRecordIndexTable } from '@/object-record/record-index/hooks/useL import { ROW_HEIGHT } from '@/object-record/record-table/constants/RowHeight'; import { RecordTableContext } from '@/object-record/record-table/contexts/RecordTableContext'; import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates'; +import { hasRecordTableFetchedAllRecordsComponentStateV2 } from '@/object-record/record-table/states/hasRecordTableFetchedAllRecordsComponentStateV2'; import { isRecordTableScrolledLeftComponentState } from '@/object-record/record-table/states/isRecordTableScrolledLeftComponentState'; import { isRecordTableScrolledTopComponentState } from '@/object-record/record-table/states/isRecordTableScrolledTopComponentState'; import { isFetchingMoreRecordsFamilyState } from '@/object-record/states/isFetchingMoreRecordsFamilyState'; @@ -22,12 +23,13 @@ export const RecordTableBodyEffect = () => { const [hasInitializedScroll, setHasInitiazedScroll] = useState(false); const { - fetchMoreRecords: fetchMoreObjects, + fetchMoreRecords, records, totalCount, setRecordTableData, loading, queryStateIdentifier, + hasNextPage, } = useLoadRecordIndexTable(objectNameSingular); const isFetchingMoreObjects = useRecoilValue( @@ -43,6 +45,9 @@ export const RecordTableBodyEffect = () => { isRecordTableScrolledTopComponentState, ); + const setHasRecordTableFetchedAllRecordsComponents = + useSetRecoilComponentState(hasRecordTableFetchedAllRecordsComponentStateV2); + // TODO: move this outside because it might cause way too many re-renders for other hooks useEffect(() => { setIsRecordTableScrolledTop(scrollTop === 0); @@ -108,9 +113,7 @@ export const RecordTableBodyEffect = () => { } }, [ loading, - isFetchingMoreObjects, lastShowPageRecordId, - fetchMoreObjects, records, scrollToPosition, hasInitializedScroll, @@ -125,14 +128,26 @@ export const RecordTableBodyEffect = () => { const fetchMoreDebouncedIfRequested = useDebouncedCallback(async () => { // We are debouncing here to give the user some room to scroll if they want to within this throttle window - await fetchMoreObjects(); + await fetchMoreRecords(); }, 100); useEffect(() => { - if (!isFetchingMoreObjects && tableLastRowVisible) { - fetchMoreDebouncedIfRequested(); - } + const allRecordsHaveBeenFetched = !hasNextPage; + + setHasRecordTableFetchedAllRecordsComponents(allRecordsHaveBeenFetched); + }, [hasNextPage, setHasRecordTableFetchedAllRecordsComponents]); + + useEffect(() => { + (async () => { + if (!isFetchingMoreObjects && tableLastRowVisible && hasNextPage) { + await fetchMoreDebouncedIfRequested(); + } + })(); }, [ + hasNextPage, + records, + lastShowPageRecordId, + scrollToPosition, fetchMoreDebouncedIfRequested, isFetchingMoreObjects, tableLastRowVisible, diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFetchMoreLoader.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFetchMoreLoader.tsx index 32df8cf26..4e15bb733 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFetchMoreLoader.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBodyFetchMoreLoader.tsx @@ -1,17 +1,13 @@ import styled from '@emotion/styled'; import { useContext } from 'react'; import { useInView } from 'react-intersection-observer'; -import { useRecoilCallback, useRecoilValue } from 'recoil'; +import { useRecoilCallback } from 'recoil'; import { GRAY_SCALE } from 'twenty-ui'; -import { useLoadRecordIndexTable } from '@/object-record/record-index/hooks/useLoadRecordIndexTable'; import { useRecordTable } from '@/object-record/record-table/hooks/useRecordTable'; -import { isFetchingMoreRecordsFamilyState } from '@/object-record/states/isFetchingMoreRecordsFamilyState'; +import { hasRecordTableFetchedAllRecordsComponentStateV2 } from '@/object-record/record-table/states/hasRecordTableFetchedAllRecordsComponentStateV2'; import { RecordTableWithWrappersScrollWrapperContext } from '@/ui/utilities/scroll/contexts/ScrollWrapperContexts'; - -type RecordTableBodyFetchMoreLoaderProps = { - objectNameSingular: string; -}; +import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValue'; const StyledText = styled.div` align-items: center; @@ -23,16 +19,9 @@ const StyledText = styled.div` padding-left: ${({ theme }) => theme.spacing(2)}; `; -export const RecordTableBodyFetchMoreLoader = ({ - objectNameSingular, -}: RecordTableBodyFetchMoreLoaderProps) => { - const { queryStateIdentifier } = useLoadRecordIndexTable(objectNameSingular); +export const RecordTableBodyFetchMoreLoader = () => { const { setRecordTableLastRowVisible } = useRecordTable(); - const isFetchingMoreRecords = useRecoilValue( - isFetchingMoreRecordsFamilyState(queryStateIdentifier), - ); - const onLastRowVisible = useRecoilCallback( () => async (inView: boolean) => { setRecordTableLastRowVisible(inView); @@ -44,24 +33,31 @@ export const RecordTableBodyFetchMoreLoader = ({ RecordTableWithWrappersScrollWrapperContext, ); + const hasRecordTableFetchedAllRecordsComponents = useRecoilComponentValue( + hasRecordTableFetchedAllRecordsComponentStateV2, + ); + + const showLoadingMoreRow = !hasRecordTableFetchedAllRecordsComponents; + const { ref: tbodyRef } = useInView({ onChange: onLastRowVisible, + delay: 1000, rootMargin: '1000px', root: scrollWrapperRef?.ref.current?.querySelector( - '[data-overlayscrollbars-viewport="scrollbarHidden"]', + '[data-overlayscrollbars-viewport]', ), }); + if (!showLoadingMoreRow) { + return <>; + } + return ( - - {isFetchingMoreRecords && ( - - - Loading more... - - - - )} - + + + Loading more... + + + ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/states/hasRecordTableFetchedAllRecordsComponentStateV2.ts b/packages/twenty-front/src/modules/object-record/record-table/states/hasRecordTableFetchedAllRecordsComponentStateV2.ts new file mode 100644 index 000000000..f8b6716a8 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-table/states/hasRecordTableFetchedAllRecordsComponentStateV2.ts @@ -0,0 +1,9 @@ +import { RecordTableScopeInternalContext } from '@/object-record/record-table/scopes/scope-internal-context/RecordTableScopeInternalContext'; +import { createComponentStateV2 } from '@/ui/utilities/state/component-state/utils/createComponentStateV2'; + +export const hasRecordTableFetchedAllRecordsComponentStateV2 = + createComponentStateV2({ + key: 'hasRecordTableFetchedAllRecordsComponentStateV2', + componentContext: RecordTableScopeInternalContext, + defaultValue: false, + });