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

[Issue-5772] Add sort feature on settings tables #5787

Merged
merged 21 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 133 additions & 0 deletions packages/twenty-front/src/hooks/__tests__/useSortedArray.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import React, { ReactNode } from 'react';
import { renderHook } from '@testing-library/react';
import { MutableSnapshot, RecoilRoot } from 'recoil';

import { tableSortFamilyState } from '@/activities/states/tableSortFamilyState';
import { OrderBy } from '@/object-metadata/types/OrderBy';
import {
mockedTableData as tableData,
tableDataSortedByFieldsCountInAscendingOrder,
tableDataSortedByFieldsCountInDescendingOrder,
tableDataSortedBylabelInAscendingOrder,
tableDataSortedBylabelInDescendingOrder,
} from '~/testing/mock-data/tableData';

import { useSortedArray } from '../useSortedArray';

interface WrapperProps {
children: ReactNode;
initializeState?: (mutableSnapshot: MutableSnapshot) => void;
}

const Wrapper: React.FC<WrapperProps> = ({ children, initializeState }) => (
<RecoilRoot initializeState={initializeState}>{children}</RecoilRoot>
);

describe('useSortedArray hook', () => {
const initializeState =
(fieldName: string, orderBy: OrderBy) =>
({ set }: MutableSnapshot) => {
set(
tableSortFamilyState({
tableId: 'SettingsObjectDetail',
initialFieldName: 'labelPlural',
}),
{
fieldName,
orderBy,
},
);
};

test('initial sorting behavior for string fields - Ascending', () => {
const { result } = renderHook(
() =>
useSortedArray(tableData, {
tableId: 'SettingsObjectDetail',
initialFieldName: 'labelPlural',
}),
{
wrapper: ({ children }: { children: ReactNode }) => (
<Wrapper
initializeState={initializeState('labelPlural', 'AscNullsLast')}
>
{children}
</Wrapper>
),
},
);

const sortedData = result.current;

expect(sortedData).toEqual(tableDataSortedBylabelInAscendingOrder);
});

test('initial sorting behavior for string fields - Descending', () => {
const { result } = renderHook(
() =>
useSortedArray(tableData, {
tableId: 'SettingsObjectDetail',
initialFieldName: 'labelPlural',
}),
{
wrapper: ({ children }: { children: ReactNode }) => (
<Wrapper
initializeState={initializeState('labelPlural', 'DescNullsLast')}
>
{children}
</Wrapper>
),
},
);

const sortedData = result.current;

expect(sortedData).toEqual(tableDataSortedBylabelInDescendingOrder);
});

test('initial sorting behavior for number fields - Ascending', () => {
const { result } = renderHook(
() =>
useSortedArray(tableData, {
tableId: 'SettingsObjectDetail',
initialFieldName: 'labelPlural',
}),
{
wrapper: ({ children }: { children: ReactNode }) => (
<Wrapper
initializeState={initializeState('fieldsCount', 'AscNullsLast')}
>
{children}
</Wrapper>
),
},
);

const sortedData = result.current;

expect(sortedData).toEqual(tableDataSortedByFieldsCountInAscendingOrder);
});

test('initial sorting behavior for number fields - Descending', () => {
const { result } = renderHook(
() =>
useSortedArray(tableData, {
tableId: 'SettingsObjectDetail',
initialFieldName: 'labelPlural',
}),
{
wrapper: ({ children }: { children: ReactNode }) => (
<Wrapper
initializeState={initializeState('fieldsCount', 'DescNullsLast')}
>
{children}
</Wrapper>
),
},
);

const sortedData = result.current;

expect(sortedData).toEqual(tableDataSortedByFieldsCountInDescendingOrder);
});
});
39 changes: 39 additions & 0 deletions packages/twenty-front/src/hooks/useSortedArray.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { useRecoilValue } from 'recoil';

import { tableSortFamilyState } from '@/activities/states/tableSortFamilyState';
import { OrderBy } from '@/object-metadata/types/OrderBy';

export const useSortedArray = <T>(
thomtrp marked this conversation as resolved.
Show resolved Hide resolved
arrayToSort: T[],
{ tableId, initialFieldName }: { tableId: string; initialFieldName: string },
): T[] => {
const sortConfig = useRecoilValue(
tableSortFamilyState({ tableId, initialFieldName }),
);

const sortTableData = (
data: T[],
thomtrp marked this conversation as resolved.
Show resolved Hide resolved
columnKey: keyof T,
thomtrp marked this conversation as resolved.
Show resolved Hide resolved
order: OrderBy | null,
): T[] => {
return data.sort((a: T, b: T) => {
if (typeof a[columnKey] === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't try to guess the type from JS since it can be unreliable. Instead we should give this hook metadata about our table so that it knows how to behave for each field

return order === 'AscNullsLast'
? (a[columnKey] as string).localeCompare(b[columnKey] as string)
: (b[columnKey] as string).localeCompare(a[columnKey] as string);
FelixMalfait marked this conversation as resolved.
Show resolved Hide resolved
} else if (typeof a[columnKey] === 'number') {
return order === 'AscNullsLast'
? (a[columnKey] as number) - (b[columnKey] as number)
: (b[columnKey] as number) - (a[columnKey] as number);
} else return 0;
lucasbordeau marked this conversation as resolved.
Show resolved Hide resolved
});
};

const sortedTableData = sortTableData(
[...arrayToSort],
sortConfig.fieldName as keyof T,
sortConfig.orderBy,
);

return sortedTableData;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { SerializableParam } from 'recoil';

import { OrderBy } from '@/object-metadata/types/OrderBy';
import { createFamilyState } from '@/ui/utilities/state/utils/createFamilyState';

interface TableSortState {
fieldName: string;
orderBy: OrderBy | null;
}

type TableSortFamilyKey = {
thomtrp marked this conversation as resolved.
Show resolved Hide resolved
tableId: string;
initialFieldName: string;
} & SerializableParam;

const getDefaultTableSortState = ({
thomtrp marked this conversation as resolved.
Show resolved Hide resolved
initialFieldName,
}: TableSortFamilyKey): TableSortState => ({
fieldName: initialFieldName,
orderBy: 'AscNullsLast' as OrderBy,
});

export const tableSortFamilyState = createFamilyState<
TableSortState,
TableSortFamilyKey
>({
key: 'tableSortFamilyState',
defaultValue: getDefaultTableSortState as TableSortState,
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ReactNode } from 'react';
import { ReactNode, useEffect } from 'react';
import { useTheme } from '@emotion/react';
import styled from '@emotion/styled';
import { useIcons } from 'twenty-ui';
Expand All @@ -10,10 +10,11 @@ import { getObjectTypeLabel } from '@/settings/data-model/utils/getObjectTypeLab
import { TableCell } from '@/ui/layout/table/components/TableCell';
import { TableRow } from '@/ui/layout/table/components/TableRow';

type SettingsObjectItemTableRowProps = {
export type SettingsObjectItemTableRowProps = {
action: ReactNode;
objectItem: ObjectMetadataItem;
to?: string;
updateInstanceCount: (id: string, instanceCount: number) => void;
};

export const StyledObjectTableRow = styled(TableRow)`
Expand All @@ -34,12 +35,18 @@ export const SettingsObjectItemTableRow = ({
action,
objectItem,
to,
updateInstanceCount,
}: SettingsObjectItemTableRowProps) => {
const theme = useTheme();

const { totalCount } = useFindManyRecords({
objectNameSingular: objectItem.nameSingular,
});
useEffect(() => {
//Hoist instance count to parent component state to determine sort order
Copy link
Member

Choose a reason for hiding this comment

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

Can you try getting rid of useEffect? https://react.dev/learn/you-might-not-need-an-effect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me find a way to do that. Probably I will have to create a global state to do that, as we have to store the data somewhere to actually use it higher up the React tree.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can use a recoil family here also

updateInstanceCount(objectItem.id, totalCount || 0);
}, [objectItem.id, totalCount, updateInstanceCount]);

const { getIcon } = useIcons();
const Icon = getIcon(objectItem.icon);
const objectTypeLabel = getObjectTypeLabel(objectItem);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import styled from '@emotion/styled';

const StyledTableHeader = styled.div<{ align?: 'left' | 'center' | 'right' }>`
const StyledTableHeader = styled.div<{
align?: 'left' | 'center' | 'right';
onClick?: () => void;
}>`
align-items: center;
border-bottom: 1px solid ${({ theme }) => theme.border.color.light};
color: ${({ theme }) => theme.font.color.tertiary};
Expand All @@ -15,6 +18,7 @@ const StyledTableHeader = styled.div<{ align?: 'left' | 'center' | 'right' }>`
: 'flex-start'};
padding: 0 ${({ theme }) => theme.spacing(2)};
text-align: ${({ align }) => align ?? 'left'};
cursor: ${({ onClick }) => (onClick ? 'pointer' : 'default')};
`;

export { StyledTableHeader as TableHeader };
Loading
Loading