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

Improve aggregate footer cell display #9124

Merged
merged 18 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { StyledHeaderDropdownButton } from '@/ui/layout/dropdown/components/StyledHeaderDropdownButton';
import styled from '@emotion/styled';
import { AppTooltip, Tag, TooltipDelay } from 'twenty-ui';
import { formatNumber } from '~/utils/format/number';

const StyledButton = styled(StyledHeaderDropdownButton)`
padding: 0;
Expand All @@ -19,10 +18,7 @@ export const RecordBoardColumnHeaderAggregateDropdownButton = ({
return (
<div id={dropdownId}>
<StyledButton>
<Tag
text={value ? formatNumber(Number(value)) : '-'}
color={'transparent'}
/>
<Tag text={value ? value.toString() : '-'} color={'transparent'} />
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: removing formatNumber() means numbers won't be properly formatted (e.g. 1000000 will show as '1000000' instead of '1M' or '1,000,000'). Consider keeping number formatting for better readability

<AppTooltip
anchorSelect={`#${dropdownId}`}
content={tooltip}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const useAggregateRecordsForRecordBoardColumn = () => {
skip: !isAggregateQueryEnabled,
});

const { value, label } = computeAggregateValueAndLabel({
const { value, labelWithFieldName } = computeAggregateValueAndLabel({
data,
objectMetadataItem,
fieldMetadataId: recordIndexKanbanAggregateOperation?.fieldMetadataId,
Expand All @@ -84,6 +84,6 @@ export const useAggregateRecordsForRecordBoardColumn = () => {

return {
aggregateValue: isAggregateQueryEnabled ? value : recordCount,
aggregateLabel: isDefined(value) ? label : undefined,
aggregateLabel: isDefined(value) ? labelWithFieldName : undefined,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,81 @@ describe('computeAggregateValueAndLabel', () => {
});

expect(result).toEqual({
value: 2,
label: 'Sum of amount',
value: '2',
label: 'Sum',
labelWithFieldName: 'Sum of amount',
});
});

it('should handle number field as percentage', () => {
const mockObjectMetadataWithPercentageField: ObjectMetadataItem = {
id: '123',
fields: [
{
id: MOCK_FIELD_ID,
name: 'percentage',
type: FieldMetadataType.Number,
settings: {
type: 'percentage',
},
} as FieldMetadataItem,
],
} as ObjectMetadataItem;

const mockData = {
percentage: {
[AGGREGATE_OPERATIONS.avg]: 0.3,
},
};

const result = computeAggregateValueAndLabel({
data: mockData,
objectMetadataItem: mockObjectMetadataWithPercentageField,
fieldMetadataId: MOCK_FIELD_ID,
aggregateOperation: AGGREGATE_OPERATIONS.avg,
fallbackFieldName: MOCK_KANBAN_FIELD_NAME,
});

expect(result).toEqual({
value: '30%',
label: 'Average',
labelWithFieldName: 'Average of percentage',
});
});

it('should handle number field with decimals', () => {
const mockObjectMetadataWithDecimalsField: ObjectMetadataItem = {
id: '123',
fields: [
{
id: MOCK_FIELD_ID,
name: 'decimals',
type: FieldMetadataType.Number,
settings: {
decimals: 2,
},
} as FieldMetadataItem,
],
} as ObjectMetadataItem;

const mockData = {
decimals: {
[AGGREGATE_OPERATIONS.sum]: 0.009,
},
};

const result = computeAggregateValueAndLabel({
data: mockData,
objectMetadataItem: mockObjectMetadataWithDecimalsField,
fieldMetadataId: MOCK_FIELD_ID,
aggregateOperation: AGGREGATE_OPERATIONS.sum,
fallbackFieldName: MOCK_KANBAN_FIELD_NAME,
});

expect(result).toEqual({
value: '0.01',
label: 'Sum',
labelWithFieldName: 'Sum of decimals',
});
});

Expand Down Expand Up @@ -86,8 +159,9 @@ describe('computeAggregateValueAndLabel', () => {
});

expect(result).toEqual({
value: undefined,
label: 'Sum of amount',
value: '-',
label: 'Sum',
labelWithFieldName: 'Sum of amount',
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { getAggregateOperationLabel } from '@/object-record/record-board/record-
import { AGGREGATE_OPERATIONS } from '@/object-record/record-table/constants/AggregateOperations';
import isEmpty from 'lodash.isempty';
import { FieldMetadataType } from '~/generated-metadata/graphql';
import { formatAmount } from '~/utils/format/formatAmount';
import { formatNumber } from '~/utils/format/number';
import { isDefined } from '~/utils/isDefined';

export const computeAggregateValueAndLabel = ({
Expand Down Expand Up @@ -42,18 +44,40 @@ export const computeAggregateValueAndLabel = ({

const aggregateValue = data[field.name]?.[aggregateOperation];

const value =
isDefined(aggregateValue) && field.type === FieldMetadataType.Currency
? Number(aggregateValue) / 1_000_000
: aggregateValue;
let value;

const label =
if (aggregateOperation === AGGREGATE_OPERATIONS.count) {
value = aggregateValue;
} else if (!isDefined(aggregateValue)) {
value = '-';
Comment on lines +51 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

style: consider using null instead of '-' to maintain consistent typing across the codebase

} else {
value = Number(aggregateValue);

switch (field.type) {
case FieldMetadataType.Currency: {
value = formatAmount(value / 1_000_000);
break;
}

case FieldMetadataType.Number: {
const { decimals, type } = field.settings ?? {};
value =
type === 'percentage'
? `${formatNumber(value * 100, decimals)}%`
: formatNumber(value, decimals);
break;
}
}
}
const label = getAggregateOperationLabel(aggregateOperation);
const labelWithFieldName =
aggregateOperation === AGGREGATE_OPERATIONS.count
? `${getAggregateOperationLabel(AGGREGATE_OPERATIONS.count)}`
: `${getAggregateOperationLabel(aggregateOperation)} of ${field.name}`;

return {
value,
label,
labelWithFieldName,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import { ViewComponentInstanceContext } from '@/views/states/contexts/ViewCompon
export const recordIndexRecordGroupHideComponentState =
createComponentStateV2<boolean>({
key: 'recordIndexRecordGroupHideComponentState',
defaultValue: true,
defaultValue: false,
componentInstanceContext: ViewComponentInstanceContext,
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { RecordTableNoRecordGroupBody } from '@/object-record/record-table/recor
import { RecordTableNoRecordGroupBodyEffect } from '@/object-record/record-table/record-table-body/components/RecordTableNoRecordGroupBodyEffect';
import { RecordTableRecordGroupBodyEffects } from '@/object-record/record-table/record-table-body/components/RecordTableRecordGroupBodyEffects';
import { RecordTableRecordGroupsBody } from '@/object-record/record-table/record-table-body/components/RecordTableRecordGroupsBody';
import { RecordTableFooter } from '@/object-record/record-table/record-table-footer/components/RecordTableFooter';
import { RecordTableAggregateFooter } from '@/object-record/record-table/record-table-footer/components/RecordTableAggregateFooter';
import { RecordTableHeader } from '@/object-record/record-table/record-table-header/components/RecordTableHeader';
import { isRecordTableInitialLoadingComponentState } from '@/object-record/record-table/states/isRecordTableInitialLoadingComponentState';
import { recordTablePendingRecordIdComponentState } from '@/object-record/record-table/states/recordTablePendingRecordIdComponentState';
Expand Down Expand Up @@ -88,7 +88,7 @@ export const RecordTable = () => {
<RecordTableEmptyState />
) : (
<>
<StyledTable className="entity-table-cell" ref={tableBodyRef}>
<StyledTable ref={tableBodyRef}>
<RecordTableHeader />
{!hasRecordGroups ? (
<RecordTableNoRecordGroupBody />
Expand All @@ -97,7 +97,7 @@ export const RecordTable = () => {
)}
<RecordTableStickyEffect />
{isAggregateQueryEnabled && !hasRecordGroups && (
<RecordTableFooter />
<RecordTableAggregateFooter endOfTableSticky />
)}
</StyledTable>
<DragSelect
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,17 @@
import { useCurrentRecordGroupId } from '@/object-record/record-group/hooks/useCurrentRecordGroupId';
import { recordIndexRecordIdsByGroupComponentFamilyState } from '@/object-record/record-index/states/recordIndexRecordIdsByGroupComponentFamilyState';
import { recordIndexAllRecordIdsComponentSelector } from '@/object-record/record-index/states/selectors/recordIndexAllRecordIdsComponentSelector';
import { RecordTableFooter } from '@/object-record/record-table/record-table-footer/components/RecordTableFooter';
import { RecordTablePendingRecordGroupRow } from '@/object-record/record-table/record-table-row/components/RecordTablePendingRecordGroupRow';
import { RecordTableRow } from '@/object-record/record-table/record-table-row/components/RecordTableRow';
import { RecordTableRecordGroupSectionAddNew } from '@/object-record/record-table/record-table-section/components/RecordTableRecordGroupSectionAddNew';
import { RecordTableRecordGroupSectionLoadMore } from '@/object-record/record-table/record-table-section/components/RecordTableRecordGroupSectionLoadMore';
import { isRecordGroupTableSectionToggledComponentState } from '@/object-record/record-table/record-table-section/states/isRecordGroupTableSectionToggledComponentState';
import { useRecoilComponentFamilyValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentFamilyValueV2';
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled';
import { useMemo } from 'react';
import { isDefined } from '~/utils/isDefined';

export const RecordTableRecordGroupRows = () => {
const isAggregateQueryEnabled = useIsFeatureEnabled(
'IS_AGGREGATE_QUERY_ENABLED',
);
const currentRecordGroupId = useCurrentRecordGroupId();

const allRecordIds = useRecoilComponentValueV2(
Expand Down Expand Up @@ -63,12 +58,6 @@ export const RecordTableRecordGroupRows = () => {
})}
<RecordTablePendingRecordGroupRow />
<RecordTableRecordGroupSectionAddNew />
{isAggregateQueryEnabled && (
<RecordTableFooter
key={currentRecordGroupId}
currentRecordGroupId={currentRecordGroupId}
/>
)}
<RecordTableRecordGroupSectionLoadMore />
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const RecordTableBodyDroppable = ({
isDropDisabled,
}: RecordTableBodyDroppableProps) => {
const [v4Persistable] = useState(v4());
const recordTableBodyId = `record-table-body${recordGroupId ? '-' + recordGroupId : ''}`;

return (
<Droppable
Expand All @@ -23,7 +24,7 @@ export const RecordTableBodyDroppable = ({
>
{(provided) => (
<RecordTableBody
id="record-table-body"
id={recordTableBodyId}
ref={provided.innerRef}
// eslint-disable-next-line react/jsx-props-no-spreading
{...provided.droppableProps}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ import { RecordTableRecordGroupRows } from '@/object-record/record-table/compone
import { RecordTableBodyDroppable } from '@/object-record/record-table/record-table-body/components/RecordTableBodyDroppable';
import { RecordTableBodyLoading } from '@/object-record/record-table/record-table-body/components/RecordTableBodyLoading';
import { RecordTableBodyRecordGroupDragDropContextProvider } from '@/object-record/record-table/record-table-body/components/RecordTableBodyRecordGroupDragDropContextProvider';
import { RecordTableAggregateFooter } from '@/object-record/record-table/record-table-footer/components/RecordTableAggregateFooter';
import { RecordTableRecordGroupEmptyRow } from '@/object-record/record-table/record-table-section/components/RecordTableRecordGroupEmptyRow';
import { RecordTableRecordGroupSection } from '@/object-record/record-table/record-table-section/components/RecordTableRecordGroupSection';
import { isRecordTableInitialLoadingComponentState } from '@/object-record/record-table/states/isRecordTableInitialLoadingComponentState';
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled';

export const RecordTableRecordGroupsBody = () => {
const isAggregateQueryEnabled = useIsFeatureEnabled(
'IS_AGGREGATE_QUERY_ENABLED',
);
const allRecordIds = useRecoilComponentValueV2(
recordIndexAllRecordIdsComponentSelector,
);
Expand All @@ -29,21 +34,29 @@ export const RecordTableRecordGroupsBody = () => {
}

return (
<RecordTableBodyRecordGroupDragDropContextProvider>
{visibleRecordGroupIds.map((recordGroupId, index) => (
<RecordTableRecordGroupBodyContextProvider
key={recordGroupId}
recordGroupId={recordGroupId}
>
<RecordGroupContext.Provider value={{ recordGroupId }}>
<RecordTableBodyDroppable recordGroupId={recordGroupId}>
{index > 0 && <RecordTableRecordGroupEmptyRow />}
<RecordTableRecordGroupSection />
<RecordTableRecordGroupRows />
</RecordTableBodyDroppable>
</RecordGroupContext.Provider>
</RecordTableRecordGroupBodyContextProvider>
))}
</RecordTableBodyRecordGroupDragDropContextProvider>
<>
<RecordTableBodyRecordGroupDragDropContextProvider>
{visibleRecordGroupIds.map((recordGroupId, index) => (
<RecordTableRecordGroupBodyContextProvider
key={recordGroupId}
recordGroupId={recordGroupId}
>
<RecordGroupContext.Provider value={{ recordGroupId }}>
<RecordTableBodyDroppable recordGroupId={recordGroupId}>
{index > 0 && <RecordTableRecordGroupEmptyRow />}
<RecordTableRecordGroupSection />
<RecordTableRecordGroupRows />
</RecordTableBodyDroppable>
{isAggregateQueryEnabled && (
<RecordTableAggregateFooter
key={recordGroupId}
currentRecordGroupId={recordGroupId}
/>
)}
</RecordGroupContext.Provider>
</RecordTableRecordGroupBodyContextProvider>
))}
</RecordTableBodyRecordGroupDragDropContextProvider>
</>
);
};
Loading
Loading