Address comments

Author: amanmahajan7

src/TreeDataGrid.tsx

@@ -39,7 +39,7 @@ export interface TreeDataGridProps<R, SR = unknown, K extends Key = Key>
   ) => Record<string, readonly NoInfer<R>[]>;
   expandedGroupIds: ReadonlySet<unknown>;
   onExpandedGroupIdsChange: (expandedGroupIds: Set<unknown>) => void;
-  generateGroupId?: (groupKey: string, parentId?: string) => string;
+  groupIdGetter?: Maybe<(groupKey: string, parentId?: string) => string>;
 }
 
 type GroupByDictionary<TRow> = Record<
@@ -67,14 +67,15 @@ export function TreeDataGrid<R, SR = unknown, K extends Key = Key>({
   rowGrouper,
   expandedGroupIds,
   onExpandedGroupIdsChange,
-  generateGroupId,
+  groupIdGetter: rawGroupIdGetter,
   ...props
 }: TreeDataGridProps<R, SR, K>) {
   const defaultRenderers = useDefaultRenderers<R, SR>();
   const rawRenderRow = renderers?.renderRow ?? defaultRenderers?.renderRow ?? defaultRenderRow;
   const headerAndTopSummaryRowsCount = 1 + (props.topSummaryRows?.length ?? 0);
   const { leftKey, rightKey } = getLeftRightKey(props.direction);
   const toggleGroupLatest = useLatestFunc(toggleGroup);
+  const groupIdGetter = rawGroupIdGetter ?? defaultGroupIdGetter;
 
   const { columns, groupBy } = useMemo(() => {
     const columns = [...rawColumns].sort(({ key: aKey }, { key: bKey }) => {
@@ -147,14 +148,6 @@ export function TreeDataGrid<R, SR = unknown, K extends Key = Key>({
 
     const flattenedRows: Array<R | GroupRow<R>> = [];
 
-    const groupIdGenerator = (groupKey: string, parentId?: string) => {
-      if (generateGroupId !== undefined && typeof generateGroupId === 'function') {
-        return generateGroupId(groupKey, parentId);
-      }
-
-      return `${parentId}__${groupKey}`;
-    };
-
     const expandGroup = (
       rows: GroupByDictionary<R> | readonly R[],
       parentId: string | undefined,
@@ -165,7 +158,7 @@ export function TreeDataGrid<R, SR = unknown, K extends Key = Key>({
         return;
       }
       Object.keys(rows).forEach((groupKey, posInSet, keys) => {
-        const id = groupIdGenerator(groupKey, parentId);
+        const id = groupIdGetter(groupKey, parentId);
         const isExpanded = expandedGroupIds.has(id);
         const { childRows, childGroups, startRowIndex } = rows[groupKey];
 
@@ -195,7 +188,7 @@ export function TreeDataGrid<R, SR = unknown, K extends Key = Key>({
     function isGroupRow(row: R | GroupRow<R>): row is GroupRow<R> {
       return allGroupRows.has(row);
     }
-  }, [expandedGroupIds, groupedRows, rawRows, generateGroupId]);
+  }, [expandedGroupIds, groupedRows, rawRows, groupIdGetter]);
 
   const rowHeight = useMemo(() => {
     if (typeof rawRowHeight === 'function') {
@@ -455,6 +448,10 @@ export function TreeDataGrid<R, SR = unknown, K extends Key = Key>({
   );
 }
 
+function defaultGroupIdGetter(groupKey: string, parentId: string | undefined) {
+  return parentId !== undefined ? `${parentId}__${groupKey}` : groupKey;
+}
+
 function isReadonlyArray(arr: unknown): arr is readonly unknown[] {
   return Array.isArray(arr);
 }

test/browser/TreeDataGrid.test.tsx

@@ -84,13 +84,13 @@ const onCellPasteSpy = vi.fn(({ row }: { row: Row }) => row);
 function rowKeyGetter(row: Row) {
   return row.id;
 }
-
-interface TestGridProps {
+function TestGrid({
+  groupBy,
+  groupIdGetter
+}: {
   groupBy: string[];
-  generateGroupId: ((groupKey: string, parentId?: string) => string) | undefined;
-}
-
-function TestGrid({ groupBy, generateGroupId }: TestGridProps) {
+  groupIdGetter: ((groupKey: string, parentId?: string) => string) | undefined;
+}) {
   const [rows, setRows] = useState(initialRows);
   const [selectedRows, setSelectedRows] = useState((): ReadonlySet<number> => new Set());
   const [expandedGroupIds, setExpandedGroupIds] = useState(
@@ -113,7 +113,7 @@ function TestGrid({ groupBy, generateGroupId }: TestGridProps) {
       onRowsChange={setRows}
       onCellCopy={onCellCopySpy}
       onCellPaste={onCellPasteSpy}
-      {...(generateGroupId && { generateGroupId })}
+      groupIdGetter={groupIdGetter}
     />
   );
 }
@@ -124,11 +124,8 @@ function rowGrouper(rows: readonly Row[], columnKey: string) {
   return Object.groupBy(rows, (r) => r[columnKey]) as Record<string, readonly R[]>;
 }
 
-function setup(
-  groupBy: string[],
-  generateGroupId?: (groupKey: string, parentId?: string) => string
-) {
-  page.render(<TestGrid groupBy={groupBy} generateGroupId={generateGroupId} />);
+function setup(groupBy: string[], groupIdGetter?: (groupKey: string, parentId?: string) => string) {
+  page.render(<TestGrid groupBy={groupBy} groupIdGetter={groupIdGetter} />);
 }
 
 function getHeaderCellsContent() {
@@ -162,13 +159,23 @@ test('should group by multiple columns', async () => {
   expect(getRows()).toHaveLength(4);
 });
 
-test('should group by multiple columns when passing generateGroupId', () => {
-  setup(['country', 'year'], (groupKey, parentId) =>
+test('should use groupIdGetter when provided', async () => {
+  const groupIdGetter = vi.fn((groupKey: string, parentId?: string) =>
     parentId !== undefined ? `${groupKey}#${parentId}` : groupKey
   );
+  setup(['country', 'year'], groupIdGetter);
+  expect(groupIdGetter).toHaveBeenCalled();
   expect(getTreeGrid()).toHaveAttribute('aria-rowcount', '13');
   expect(getHeaderCellsContent()).toStrictEqual(['', 'Country', 'Year', 'Sport', 'Id']);
   expect(getRows()).toHaveLength(4);
+  groupIdGetter.mockClear();
+  await userEvent.click(page.getByRole('gridcell', { name: 'USA' }));
+  expect(getRows()).toHaveLength(6);
+  expect(groupIdGetter).toHaveBeenCalled();
+  await userEvent.click(page.getByRole('gridcell', { name: 'Canada' }));
+  expect(getRows()).toHaveLength(8);
+  await userEvent.click(page.getByRole('gridcell', { name: '2020' }));
+  expect(getRows()).toHaveLength(9);
 });
 
 test('should ignore duplicate groupBy columns', async () => {
@@ -194,20 +201,6 @@ test('should toggle group when group cell is clicked', async () => {
   expect(getRows()).toHaveLength(5);
 });
 
-test('should toggle group when group cell is clicked and is passing `generateGroupId` props', async () => {
-  setup(['country', 'year'], (groupKey, parentId) =>
-    parentId !== undefined ? `${groupKey}#${parentId}` : groupKey
-  );
-  expect(getTreeGrid()).toHaveAttribute('aria-rowcount', '13');
-  expect(getHeaderCellsContent()).toStrictEqual(['', 'Country', 'Year', 'Sport', 'Id']);
-  await userEvent.click(page.getByRole('gridcell', { name: 'USA' }));
-  expect(getRows()).toHaveLength(6);
-  await userEvent.click(page.getByRole('gridcell', { name: 'Canada' }));
-  expect(getRows()).toHaveLength(8);
-  await userEvent.click(page.getByRole('gridcell', { name: '2020' }));
-  expect(getRows()).toHaveLength(9);
-});
-
 test('should toggle group using keyboard', async () => {
   setup(['year']);
   expect(getRows()).toHaveLength(5);