From 753729d8ac6b7e9b96a0bbee4a4ca760b11df521 Mon Sep 17 00:00:00 2001 From: Joao Duarte Date: Sun, 14 Jan 2024 03:25:48 -0300 Subject: [PATCH 1/3] fix(table-core): Do not select subrows if enableSubRowSelection is set to false --- .../table-core/__tests__/RowSelection.test.ts | 69 +++++++++++++++++++ .../table-core/src/features/RowSelection.ts | 4 +- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/table-core/__tests__/RowSelection.test.ts b/packages/table-core/__tests__/RowSelection.test.ts index f8d2fc9279..1ed16cf4b0 100644 --- a/packages/table-core/__tests__/RowSelection.test.ts +++ b/packages/table-core/__tests__/RowSelection.test.ts @@ -1,5 +1,6 @@ import { ColumnDef, + TableState, createColumnHelper, createTable, getCoreRowModel, @@ -319,4 +320,72 @@ describe('RowSelection', () => { expect(result).toEqual('some') }) }) + + describe('toggleAllRowsSelected', () => { + test('it should not select subrows if enableSubRowSelection is set to false', () => { + const data = makeData(2, 1) + const columns = generateColumns(data) + + let state: Partial = { + rowSelection: {}, + } + + const table = createTable({ + enableRowSelection: true, + onStateChange() {}, + renderFallbackValue: '', + data, + columns, + getSubRows: row => row.subRows, + state, + onRowSelectionChange(updater) { + state.rowSelection = + typeof updater === 'function' + ? updater(state.rowSelection ?? {}) + : updater + }, + enableSubRowSelection: false, + getCoreRowModel: getCoreRowModel(), + }) + + table.toggleAllRowsSelected(true) + const allRows = table + .getCoreRowModel() + .flatRows.map(v => v.getIsSelected()) + expect(allRows).toEqual([true, false, true, false]) + }) + + test('it should select subrows if enableSubRowSelection is set to true', () => { + const data = makeData(2, 1) + const columns = generateColumns(data) + + let state: Partial = { + rowSelection: {}, + } + + const table = createTable({ + enableRowSelection: true, + onStateChange() {}, + renderFallbackValue: '', + data, + columns, + getSubRows: row => row.subRows, + state, + onRowSelectionChange(updater) { + state.rowSelection = + typeof updater === 'function' + ? updater(state.rowSelection ?? {}) + : updater + }, + enableSubRowSelection: true, + getCoreRowModel: getCoreRowModel(), + }) + + table.toggleAllRowsSelected(true) + const allRows = table + .getCoreRowModel() + .flatRows.map(v => v.getIsSelected()) + expect(allRows).toEqual([true, true, true, true]) + }) + }) }) diff --git a/packages/table-core/src/features/RowSelection.ts b/packages/table-core/src/features/RowSelection.ts index efb927600b..9dfedd57bc 100644 --- a/packages/table-core/src/features/RowSelection.ts +++ b/packages/table-core/src/features/RowSelection.ts @@ -225,7 +225,9 @@ export const RowSelection: TableFeature = { const rowSelection = { ...old } - const preGroupedFlatRows = table.getPreGroupedRowModel().flatRows + const preGroupedFlatRows = table.options.enableSubRowSelection + ? table.getPreGroupedRowModel().flatRows + : table.getPreGroupedRowModel().rows // We don't use `mutateRowIsSelected` here for performance reasons. // All of the rows are flat already, so it wouldn't be worth it From adc80c2f47ff136f9b33ac411011ecd6c8997c7f Mon Sep 17 00:00:00 2001 From: Joao Duarte Date: Sun, 14 Jan 2024 03:27:52 -0300 Subject: [PATCH 2/3] refactor: use const instead of let --- packages/table-core/__tests__/RowSelection.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/table-core/__tests__/RowSelection.test.ts b/packages/table-core/__tests__/RowSelection.test.ts index 1ed16cf4b0..9ac2a7bf7c 100644 --- a/packages/table-core/__tests__/RowSelection.test.ts +++ b/packages/table-core/__tests__/RowSelection.test.ts @@ -326,7 +326,7 @@ describe('RowSelection', () => { const data = makeData(2, 1) const columns = generateColumns(data) - let state: Partial = { + const state: Partial = { rowSelection: {}, } @@ -359,7 +359,7 @@ describe('RowSelection', () => { const data = makeData(2, 1) const columns = generateColumns(data) - let state: Partial = { + const state: Partial = { rowSelection: {}, } From eb630885498f5b1139ea5c6c835463819eee805a Mon Sep 17 00:00:00 2001 From: Joao Duarte Date: Sun, 14 Jan 2024 12:17:25 -0300 Subject: [PATCH 3/3] fix: Edge case when enableSubRowSelection is a function --- .../table-core/__tests__/RowSelection.test.ts | 39 +++++++++++++++ .../table-core/src/features/RowSelection.ts | 47 +++++++++++++------ 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/packages/table-core/__tests__/RowSelection.test.ts b/packages/table-core/__tests__/RowSelection.test.ts index 9ac2a7bf7c..c73ccf0524 100644 --- a/packages/table-core/__tests__/RowSelection.test.ts +++ b/packages/table-core/__tests__/RowSelection.test.ts @@ -355,6 +355,45 @@ describe('RowSelection', () => { expect(allRows).toEqual([true, false, true, false]) }) + test('it should not select subrows if enableSubRowSelection is a function and it evaluates to false', () => { + const data = makeData(2, 2, 1) + const columns = generateColumns(data) + + const state: Partial = { + rowSelection: {}, + } + + const table = createTable({ + onStateChange() {}, + renderFallbackValue: '', + data, + columns, + getSubRows: row => row.subRows, + state, + onRowSelectionChange(updater) { + state.rowSelection = + typeof updater === 'function' + ? updater(state.rowSelection ?? {}) + : updater + }, + enableRowSelection: true, + enableSubRowSelection: row => + row.id === '0' || row.id.startsWith('0.0'), + getCoreRowModel: getCoreRowModel(), + }) + + table.toggleAllRowsSelected(true) + const selectionState = table.getState().rowSelection + + expect(selectionState).toEqual({ + '0': true, + '0.0': true, + '0.0.0': true, + '0.1': true, + '1': true, + }) + }) + test('it should select subrows if enableSubRowSelection is set to true', () => { const data = makeData(2, 1) const columns = generateColumns(data) diff --git a/packages/table-core/src/features/RowSelection.ts b/packages/table-core/src/features/RowSelection.ts index 9dfedd57bc..cc199694ae 100644 --- a/packages/table-core/src/features/RowSelection.ts +++ b/packages/table-core/src/features/RowSelection.ts @@ -225,23 +225,40 @@ export const RowSelection: TableFeature = { const rowSelection = { ...old } - const preGroupedFlatRows = table.options.enableSubRowSelection - ? table.getPreGroupedRowModel().flatRows - : table.getPreGroupedRowModel().rows - - // We don't use `mutateRowIsSelected` here for performance reasons. - // All of the rows are flat already, so it wouldn't be worth it - if (value) { - preGroupedFlatRows.forEach(row => { - if (!row.getCanSelect()) { - return - } - rowSelection[row.id] = true + if (typeof table.options.enableSubRowSelection === 'function') { + // There's no easy workaround like using flat rows here in the case enableSubRowSelection is a function + // We need to recursively check if the current row/subrow can be selected so just + // using mutateRowIsSelected is easier + table.getFilteredRowModel().rows.forEach(row => { + mutateRowIsSelected( + rowSelection, + row.id, + value ?? false, + true, + table + ) }) } else { - preGroupedFlatRows.forEach(row => { - delete rowSelection[row.id] - }) + // Here we check if only the first level of rows should be selected (enableSubRowSelection is true.) + // If that's the case we just get the first level of rows, otherwise we get the flat rows. + const preGroupedFlatRows = table.options.enableSubRowSelection + ? table.getPreGroupedRowModel().flatRows + : table.getPreGroupedRowModel().rows + + // We don't use `mutateRowIsSelected` here for performance reasons. + // The rows are either flat or just the first level is being selected, so it wouldn't be worth it + if (value) { + preGroupedFlatRows.forEach(row => { + if (!row.getCanSelect()) { + return + } + rowSelection[row.id] = true + }) + } else { + preGroupedFlatRows.forEach(row => { + delete rowSelection[row.id] + }) + } } return rowSelection