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

Data Skeleton Loading on Indexes #5828

Merged
merged 13 commits into from
Jun 19, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const StyledBoardCardWrapper = styled.div`
width: 100%;
`;

const StyledBoardCardHeader = styled.div<{
export const StyledBoardCardHeader = styled.div<{
showCompactView: boolean;
}>`
align-items: center;
Expand All @@ -89,7 +89,7 @@ const StyledBoardCardHeader = styled.div<{
}
`;

const StyledBoardCardBody = styled.div`
export const StyledBoardCardBody = styled.div`
display: flex;
flex-direction: column;
gap: ${({ theme }) => theme.spacing(0.5)};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import Skeleton, { SkeletonTheme } from 'react-loading-skeleton';
import { useTheme } from '@emotion/react';
import styled from '@emotion/styled';

import {
StyledBoardCardBody,
StyledBoardCardHeader,
} from '@/object-record/record-board/record-board-card/components/RecordBoardCard';

const StyledSkeletonIconAndText = styled.div`
display: flex;
gap: ${({ theme }) => theme.spacing(1)};
`;

const StyledSkeletonTitle = styled.div`
padding-left: ${({ theme }) => theme.spacing(2)};
`;

const StyledSeparator = styled.div`
height: ${({ theme }) => theme.spacing(2)};
`;

export const RecordBoardColumnCardContainerSkeletonLoader = ({
titleSkeletonWidth,
isCompactModeActive,
}: {
titleSkeletonWidth: number;
isCompactModeActive: boolean;
}) => {
const theme = useTheme();
const skeletonItems = Array.from({ length: 6 }).map((_, index) => ({
id: `skeleton-item-${index}`,
}));
return (
<SkeletonTheme
baseColor={theme.background.tertiary}
highlightColor={theme.background.transparent.lighter}
borderRadius={4}
>
<StyledBoardCardHeader showCompactView={isCompactModeActive}>
<StyledSkeletonTitle>
<Skeleton width={titleSkeletonWidth} height={16} />
</StyledSkeletonTitle>
</StyledBoardCardHeader>
<StyledSeparator />
{!isCompactModeActive &&
skeletonItems.map(({ id }) => (
<StyledBoardCardBody key={id}>
<StyledSkeletonIconAndText>
<Skeleton width={16} height={16} />
<Skeleton width={151} height={16} />
</StyledSkeletonIconAndText>
</StyledBoardCardBody>
))}
</SkeletonTheme>
);
};
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import React, { useContext } from 'react';
import styled from '@emotion/styled';
import { Draggable, DroppableProvided } from '@hello-pangea/dnd';
import { useRecoilValue } from 'recoil';

import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
import { RecordBoardContext } from '@/object-record/record-board/contexts/RecordBoardContext';
import { useRecordBoardStates } from '@/object-record/record-board/hooks/internal/useRecordBoardStates';
import { RecordBoardColumnCardContainerSkeletonLoader } from '@/object-record/record-board/record-board-column/components/RecordBoardColumnCardContainerSkeletonLoader';
import { RecordBoardColumnCardsMemo } from '@/object-record/record-board/record-board-column/components/RecordBoardColumnCardsMemo';
import { RecordBoardColumnFetchMoreLoader } from '@/object-record/record-board/record-board-column/components/RecordBoardColumnFetchMoreLoader';
import { RecordBoardColumnNewButton } from '@/object-record/record-board/record-board-column/components/RecordBoardColumnNewButton';
import { RecordBoardColumnNewOpportunityButton } from '@/object-record/record-board/record-board-column/components/RecordBoardColumnNewOpportunityButton';
import { RecordBoardColumnContext } from '@/object-record/record-board/record-board-column/contexts/RecordBoardColumnContext';
import { isRecordIndexBoardColumnLoadingFamilyState } from '@/object-record/states/isRecordBoardColumnLoadingFamilyState';

const StyledColumnCardsContainer = styled.div`
display: flex;
Expand All @@ -20,6 +24,17 @@ const StyledNewButtonContainer = styled.div`
padding-bottom: ${({ theme }) => theme.spacing(4)};
`;

const StyledSkeletonCardContainer = styled.div`
background-color: ${({ theme }) => theme.background.secondary};
border: 1px solid ${({ theme }) => theme.background.quaternary};
border-radius: ${({ theme }) => theme.border.radius.md};
box-shadow:
0px 4px 8px 0px rgba(0, 0, 0, 0.08),
0px 0px 4px 0px rgba(0, 0, 0, 0.08);
color: ${({ theme }) => theme.font.color.primary};
margin-bottom: ${({ theme }) => theme.spacing(2)};
`;

type RecordBoardColumnCardsContainerProps = {
recordIds: string[];
droppableProvided: DroppableProvided;
Expand All @@ -32,13 +47,51 @@ export const RecordBoardColumnCardsContainer = ({
const { columnDefinition } = useContext(RecordBoardColumnContext);
const { objectMetadataItem } = useContext(RecordBoardContext);

const columnId = columnDefinition.id;

const isRecordIndexBoardColumnLoading = useRecoilValue(
isRecordIndexBoardColumnLoadingFamilyState(columnId),
);

const { isCompactModeActiveState } = useRecordBoardStates();

const isCompactModeActive = useRecoilValue(isCompactModeActiveState);

const getNumberOfSkeletons = (position: number): number => {
const skeletonCounts: Record<number, number> = {
0: 2,
1: 1,
2: 3,
3: 0,
4: 1,
};

return skeletonCounts[position] || 0;
};

return (
<StyledColumnCardsContainer
ref={droppableProvided?.innerRef}
// eslint-disable-next-line react/jsx-props-no-spreading
{...droppableProvided?.droppableProps}
>
<RecordBoardColumnCardsMemo recordIds={recordIds} />
{isRecordIndexBoardColumnLoading ? (
Array.from(
{ length: getNumberOfSkeletons(columnDefinition.position) },
(_, index) => (
<StyledSkeletonCardContainer
key={`${columnDefinition.id}-${index}`}
>
<RecordBoardColumnCardContainerSkeletonLoader
titleSkeletonWidth={isCompactModeActive ? 72 : 54}
isCompactModeActive={isCompactModeActive}
/>
</StyledSkeletonCardContainer>
),
)
) : (
<RecordBoardColumnCardsMemo recordIds={recordIds} />
)}
<RecordBoardColumnFetchMoreLoader />
<Draggable
draggableId={`new-${columnDefinition.id}`}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { useEffect } from 'react';
import { useRecoilState } from 'recoil';
import { useRecoilState, useSetRecoilState } from 'recoil';

import { isRecordBoardFetchingRecordsByColumnFamilyState } from '@/object-record/record-board/states/isRecordBoardFetchingRecordsByColumnFamilyState';
import { recordBoardShouldFetchMoreInColumnComponentFamilyState } from '@/object-record/record-board/states/recordBoardShouldFetchMoreInColumnComponentFamilyState';
import { useLoadRecordIndexBoardColumn } from '@/object-record/record-index/hooks/useLoadRecordIndexBoardColumn';
import { isRecordIndexBoardColumnLoadingFamilyState } from '@/object-record/states/isRecordBoardColumnLoadingFamilyState';
import { getScopeIdFromComponentId } from '@/ui/utilities/recoil-scope/utils/getScopeIdFromComponentId';

export const RecordIndexBoardColumnLoaderEffect = ({
Expand Down Expand Up @@ -34,7 +35,7 @@ export const RecordIndexBoardColumnLoaderEffect = ({
}),
);

const { fetchMoreRecords, loading, hasNextPage } =
const { fetchMoreRecords, loading, records, hasNextPage } =
useLoadRecordIndexBoardColumn({
objectNameSingular,
recordBoardId,
Expand All @@ -43,6 +44,14 @@ export const RecordIndexBoardColumnLoaderEffect = ({
columnId,
});

const setIsRecordIndexLoading = useSetRecoilState(
isRecordIndexBoardColumnLoadingFamilyState(columnId),
);

useEffect(() => {
setIsRecordIndexLoading(loading && records.length === 0);
}, [records, loading, setIsRecordIndexLoading]);

useEffect(() => {
const run = async () => {
if (!loading && shouldFetchMore && hasNextPage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type UseLoadRecordIndexBoardProps = {
viewBarId: string;
recordBoardId: string;
};

// this is not used anywhere! Whew!
export const useLoadRecordIndexBoard = ({
objectNameSingular,
viewBarId,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useRecoilValue } from 'recoil';

import { RecordTableBodyFetchMoreLoader } from '@/object-record/record-table/components/RecordTableBodyFetchMoreLoader';
import { RecordTableBodyLoading } from '@/object-record/record-table/components/RecordTableBodyLoading';
import { RecordTableRow } from '@/object-record/record-table/components/RecordTableRow';
import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates';
import { DraggableTableBody } from '@/ui/layout/draggable-list/components/DraggableTableBody';
Expand All @@ -14,10 +15,19 @@ export const RecordTableBody = ({
objectNameSingular,
recordTableId,
}: RecordTableBodyProps) => {
const { tableRowIdsState } = useRecordTableStates();
const { tableRowIdsState, isRecordTableInitialLoadingState } =
useRecordTableStates();

const tableRowIds = useRecoilValue(tableRowIdsState);

const isRecordTableInitialLoading = useRecoilValue(
isRecordTableInitialLoadingState,
);

if (isRecordTableInitialLoading && tableRowIds.length === 0) {
lucasbordeau marked this conversation as resolved.
Show resolved Hide resolved
return <RecordTableBodyLoading />;
}

return (
<>
<DraggableTableBody
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { useRecoilValue } from 'recoil';

import { CheckboxCell } from '@/object-record/record-table/components/CheckboxCell';
import { RecordTableCellFieldContextWrapper } from '@/object-record/record-table/components/RecordTableCellFieldContextWrapper';
import { RecordTableCellContext } from '@/object-record/record-table/contexts/RecordTableCellContext';
import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates';

export const RecordTableBodyLoading = () => {
const { visibleTableColumnsSelector } = useRecordTableStates();
const visibleTableColumns = useRecoilValue(visibleTableColumnsSelector());
const skeletonColumnsWithSmallWidth = ['Domain Name', 'ICP', 'ARR', 'X'];

return (
<tbody>
{Array.from({ length: 8 }).map((_, rowIndex) => (
<tr
data-testid={`row-id-${rowIndex}`}
data-selectable-id={`row-id-${rowIndex}`}
>
<td>
<CheckboxCell />
</td>
{visibleTableColumns.map((column, columnIndex) => (
<RecordTableCellContext.Provider
value={{
columnDefinition: column,
columnIndex,
}}
key={column.fieldMetadataId}
>
<RecordTableCellFieldContextWrapper
skeletonWidth={
skeletonColumnsWithSmallWidth.includes(column.label)
? 108
: 132
}
/>
</RecordTableCellContext.Provider>
))}
</tr>
))}
</tbody>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import { RelationPickerHotkeyScope } from '@/object-record/relation-picker/types
import { SelectFieldHotkeyScope } from '@/object-record/select/types/SelectFieldHotkeyScope';
import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull';

export const RecordTableCellFieldContextWrapper = () => {
export const RecordTableCellFieldContextWrapper = ({
skeletonWidth,
}: {
skeletonWidth?: number;
}) => {
const { objectMetadataItem } = useContext(RecordTableContext);
const { columnDefinition } = useContext(RecordTableCellContext);
const { recordId, pathToShowPage } = useContext(RecordTableRowContext);
Expand Down Expand Up @@ -49,7 +53,10 @@ export const RecordTableCellFieldContextWrapper = () => {
}),
}}
>
<RecordTableCell customHotkeyScope={{ scope: customHotkeyScope }} />
<RecordTableCell
customHotkeyScope={{ scope: customHotkeyScope }}
skeletonWidth={skeletonWidth}
/>
</FieldContext.Provider>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ import { HotkeyScope } from '@/ui/utilities/hotkey/types/HotkeyScope';

export const RecordTableCell = ({
customHotkeyScope,
skeletonWidth,
}: {
customHotkeyScope: HotkeyScope;
skeletonWidth?: number;
}) => {
const { onUpsertRecord, onMoveFocus, onCloseTableCell } =
useContext(RecordTableContext);
Expand Down Expand Up @@ -105,6 +107,7 @@ export const RecordTableCell = ({
/>
}
nonEditModeContent={<FieldDisplay />}
skeletonWidth={skeletonWidth}
/>
</FieldFocusContextProvider>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import React, { ReactElement, useContext, useEffect, useState } from 'react';
import { clsx } from 'clsx';
import { useRecoilValue } from 'recoil';

import { FieldContext } from '@/object-record/record-field/contexts/FieldContext';
import { useFieldFocus } from '@/object-record/record-field/hooks/useFieldFocus';
import { RecordTableContext } from '@/object-record/record-table/contexts/RecordTableContext';
import { RecordTableRowContext } from '@/object-record/record-table/contexts/RecordTableRowContext';
import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates';
import { RecordTableCellContainerSkeletonLoader } from '@/object-record/record-table/record-table-cell/components/RecordTableCellContainerSkeletonLoader';
import { RecordTableCellSoftFocusMode } from '@/object-record/record-table/record-table-cell/components/RecordTableCellSoftFocusMode';
import { useCurrentTableCellPosition } from '@/object-record/record-table/record-table-cell/hooks/useCurrentCellPosition';
import { useOpenRecordTableCellFromCell } from '@/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellFromCell';
Expand All @@ -26,6 +29,7 @@ export type RecordTableCellContainerProps = {
maxContentWidth?: number;
onSubmit?: () => void;
onCancel?: () => void;
skeletonWidth?: number;
};

const DEFAULT_CELL_SCOPE: HotkeyScope = {
Expand All @@ -36,9 +40,11 @@ export const RecordTableCellContainer = ({
editModeContent,
nonEditModeContent,
editHotkeyScope,
skeletonWidth,
}: RecordTableCellContainerProps) => {
const { setIsFocused } = useFieldFocus();
const { openTableCell } = useOpenRecordTableCellFromCell();
const { isRecordTableInitialLoadingState } = useRecordTableStates();

const { isSelected, recordId, isPendingRow } = useContext(
RecordTableRowContext,
Expand All @@ -53,7 +59,9 @@ export const RecordTableCellContainer = ({
const [isInEditMode, setIsInEditMode] = useState(shouldBeInitiallyInEditMode);

const cellPosition = useCurrentTableCellPosition();

const isRecordTableInitialLoading = useRecoilValue(
isRecordTableInitialLoadingState,
);
const handleContextMenu = (event: React.MouseEvent) => {
onContextMenu(event, recordId);
};
Expand Down Expand Up @@ -142,8 +150,16 @@ export const RecordTableCellContainer = ({
[styles.cellBaseContainerSoftFocus]: hasSoftFocus,
})}
>
{isInEditMode && (
<RecordTableCellEditMode>{editModeContent}</RecordTableCellEditMode>
{isRecordTableInitialLoading ? (
<RecordTableCellContainerSkeletonLoader
skeletonWidth={skeletonWidth}
/>
) : (
isInEditMode && (
<RecordTableCellEditMode>
{editModeContent}
</RecordTableCellEditMode>
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lucasbordeau
Here we need to render the individual loader for each cell during initial table loading, so we can't just move this loader to a new component (RecordTableLoading). However, this only shows during the initial load so shouldn't cause a performance issue.

What do you think?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out this has caused a performance hit of about 0.01s

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working on moving the loading logic out of the RecordTableCell

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks !

)}
{hasSoftFocus ? (
<RecordTableCellSoftFocusMode
Expand Down
Loading
Loading