Skip to content

Commit

Permalink
Fixed record table fetch more scroll bug (#6790)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
lucasbordeau and charlesBochet authored Aug 29, 2024
1 parent cd06ae2 commit b05361e
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const useLoadRecordIndexTable = (objectNameSingular: string) => {
totalCount,
fetchMoreRecords,
queryStateIdentifier,
hasNextPage,
} = useFindManyRecords({
...params,
recordGqlFields,
Expand All @@ -74,5 +75,6 @@ export const useLoadRecordIndexTable = (objectNameSingular: string) => {
fetchMoreRecords,
queryStateIdentifier,
setRecordTableData,
hasNextPage,
};
};
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
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 = () => {
const { tableRowIdsState } = useRecordTableStates();

const tableRowIds = useRecoilValue(tableRowIdsState);

return tableRowIds.map((recordId, rowIndex) => {
return (
<RecordTableRow key={recordId} recordId={recordId} rowIndex={rowIndex} />
);
});
return (
<>
{tableRowIds.map((recordId, rowIndex) => {
return (
<RecordTableRow
key={recordId}
recordId={recordId}
rowIndex={rowIndex}
/>
);
})}
<RecordTableBodyFetchMoreLoader />
</>
);
};
Original file line number Diff line number Diff line change
@@ -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(
Expand All @@ -32,7 +27,6 @@ export const RecordTableBody = () => {
<RecordTablePendingRow />
<RecordTableRows />
</RecordTableBodyDroppable>
<RecordTableBodyFetchMoreLoader objectNameSingular={objectNameSingular} />
</RecordTableBodyDragDropContext>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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(
Expand All @@ -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);
Expand Down Expand Up @@ -108,9 +113,7 @@ export const RecordTableBodyEffect = () => {
}
}, [
loading,
isFetchingMoreObjects,
lastShowPageRecordId,
fetchMoreObjects,
records,
scrollToPosition,
hasInitializedScroll,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);
Expand All @@ -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 (
<tbody ref={tbodyRef}>
{isFetchingMoreRecords && (
<tr>
<td colSpan={7}>
<StyledText>Loading more...</StyledText>
</td>
<td colSpan={7} />
</tr>
)}
</tbody>
<tr ref={tbodyRef}>
<td colSpan={7}>
<StyledText>Loading more...</StyledText>
</td>
<td colSpan={7} />
</tr>
);
};
Original file line number Diff line number Diff line change
@@ -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<boolean>({
key: 'hasRecordTableFetchedAllRecordsComponentStateV2',
componentContext: RecordTableScopeInternalContext,
defaultValue: false,
});

0 comments on commit b05361e

Please sign in to comment.