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

fix(selection): update logic to fix selection of disabled rows #5401

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
12 changes: 6 additions & 6 deletions examples/react/row-selection/src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function App() {
<IndeterminateCheckbox
{...{
checked: table.getIsAllRowsSelected(),
indeterminate: table.getIsSomeRowsSelected(),
indeterminate: table.getIsSomeRowsSelected(true),
onChange: table.getToggleAllRowsSelectedHandler(),
}}
/>
Expand Down Expand Up @@ -110,8 +110,8 @@ function App() {
state: {
rowSelection,
},
enableRowSelection: true, //enable row selection for all rows
// enableRowSelection: row => row.original.age > 18, // or enable row selection conditionally per row
// enableRowSelection: true, //enable row selection for all rows
enableRowSelection: row => row.original.age > 18, // or enable row selection conditionally per row
onRowSelectionChange: setRowSelection,
getCoreRowModel: getCoreRowModel(),
getFilteredRowModel: getFilteredRowModel(),
Expand Down Expand Up @@ -180,7 +180,7 @@ function App() {
<IndeterminateCheckbox
{...{
checked: table.getIsAllPageRowsSelected(),
indeterminate: table.getIsSomePageRowsSelected(),
indeterminate: table.getIsSomePageRowsSelected(true),
onChange: table.getToggleAllPageRowsSelectedHandler(),
}}
/>
Expand Down Expand Up @@ -344,9 +344,9 @@ function IndeterminateCheckbox({

React.useEffect(() => {
if (typeof indeterminate === 'boolean') {
ref.current.indeterminate = !rest.checked && indeterminate
ref.current.indeterminate = indeterminate
}
}, [ref, indeterminate])
}, [ref, indeterminate, rest.checked])

return (
<input
Expand Down
62 changes: 42 additions & 20 deletions packages/table-core/src/features/RowSelection.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TableFeature } from '../core/table'
import { OnChangeFn, Table, Row, RowModel, Updater, RowData } from '../types'
import { OnChangeFn, Row, RowData, RowModel, Table, Updater } from '../types'
import { getMemoOptions, makeStateUpdater, memo } from '../utils'

export type RowSelectionState = Record<string, boolean>
Expand Down Expand Up @@ -118,7 +118,7 @@ export interface RowSelectionInstance<TData extends RowData> {
* @link [API Docs](https://tanstack.com/table/v8/docs/api/features/row-selection#getisallpagerowsselected)
* @link [Guide](https://tanstack.com/table/v8/docs/guide/row-selection)
*/
getIsAllPageRowsSelected: () => boolean
getIsAllPageRowsSelected: (ignoreCanSelect?: boolean) => boolean
/**
* Returns whether or not all rows in the table are selected.
* @link [API Docs](https://tanstack.com/table/v8/docs/api/features/row-selection#getisallrowsselected)
Expand All @@ -130,7 +130,7 @@ export interface RowSelectionInstance<TData extends RowData> {
* @link [API Docs](https://tanstack.com/table/v8/docs/api/features/row-selection#getissomepagerowsselected)
* @link [Guide](https://tanstack.com/table/v8/docs/guide/row-selection)
*/
getIsSomePageRowsSelected: () => boolean
getIsSomePageRowsSelected: (ignoreCanSelect?: boolean) => boolean
/**
* Returns whether or not any rows in the table are selected.
* @link [API Docs](https://tanstack.com/table/v8/docs/api/features/row-selection#getissomerowsselected)
Expand Down Expand Up @@ -178,7 +178,10 @@ export interface RowSelectionInstance<TData extends RowData> {
* @link [API Docs](https://tanstack.com/table/v8/docs/api/features/row-selection#toggleallpagerowsselected)
* @link [Guide](https://tanstack.com/table/v8/docs/guide/row-selection)
*/
toggleAllPageRowsSelected: (value?: boolean) => void
toggleAllPageRowsSelected: (
value?: boolean,
ignoreCanSelect?: boolean
) => void
/**
* Selects/deselects all rows in the table.
* @link [API Docs](https://tanstack.com/table/v8/docs/api/features/row-selection#toggleallrowsselected)
Expand Down Expand Up @@ -238,22 +241,24 @@ export const RowSelection: TableFeature = {
})
} else {
preGroupedFlatRows.forEach(row => {
if (!row.getCanSelect()) {
return
}
delete rowSelection[row.id]
})
}

return rowSelection
})
}
table.toggleAllPageRowsSelected = value =>
table.toggleAllPageRowsSelected = (value, ignoreCanSelect) =>
table.setRowSelection(old => {
const resolvedValue =
typeof value !== 'undefined'
? value
: !table.getIsAllPageRowsSelected()
: !table.getIsAllPageRowsSelected(ignoreCanSelect)

const rowSelection: RowSelectionState = { ...old }

table.getRowModel().rows.forEach(row => {
mutateRowIsSelected(rowSelection, row.id, resolvedValue, true, table)
})
Expand Down Expand Up @@ -405,17 +410,20 @@ export const RowSelection: TableFeature = {
return isAllRowsSelected
}

table.getIsAllPageRowsSelected = () => {
const paginationFlatRows = table
.getPaginationRowModel()
.flatRows.filter(row => row.getCanSelect())
table.getIsAllPageRowsSelected = (ignoreCanSelect?: boolean) => {
const paginationFlatRows = table.getPaginationRowModel().flatRows

const filterPaginationFlatRows = ignoreCanSelect
? paginationFlatRows
: paginationFlatRows.filter(row => row.getCanSelect())

const { rowSelection } = table.getState()

let isAllPageRowsSelected = !!paginationFlatRows.length
let isAllPageRowsSelected = !!filterPaginationFlatRows.length

if (
isAllPageRowsSelected &&
paginationFlatRows.some(row => !rowSelection[row.id])
filterPaginationFlatRows.some(row => !rowSelection[row.id])
) {
isAllPageRowsSelected = false
}
Expand All @@ -433,13 +441,25 @@ export const RowSelection: TableFeature = {
)
}

table.getIsSomePageRowsSelected = () => {
table.getIsSomePageRowsSelected = (ignoreCanSelect?: boolean) => {
const paginationFlatRows = table.getPaginationRowModel().flatRows
return table.getIsAllPageRowsSelected()

const getIsSomeSelectedByCondition = () => {
return ignoreCanSelect
? paginationFlatRows.some(
d => d.getIsSelected() || d.getIsSomeSelected()
) &&
paginationFlatRows.some(
d => !d.getIsSelected() || !d.getIsSomeSelected()
)
: paginationFlatRows
.filter(row => row.getCanSelect())
.some(d => d.getIsSelected() || d.getIsSomeSelected())
}
Comment on lines +447 to +458

Choose a reason for hiding this comment

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

I think the readability is still lacking.
How about separating this part into an independent function or clearly splitting it with if-else statements?


return table.getIsAllPageRowsSelected(ignoreCanSelect)
? false
: paginationFlatRows
.filter(row => row.getCanSelect())
.some(d => d.getIsSelected() || d.getIsSomeSelected())
: getIsSomeSelectedByCondition()
}

table.getToggleAllRowsSelectedHandler = () => {
Expand Down Expand Up @@ -552,14 +572,16 @@ const mutateRowIsSelected = <TData extends RowData>(
// !isGrouped ||
// (isGrouped && table.options.enableGroupingRowSelection)
// ) {

const canSelect = row.getCanSelect()
if (value) {
if (!row.getCanMultiSelect()) {
Object.keys(selectedRowIds).forEach(key => delete selectedRowIds[key])
}
if (row.getCanSelect()) {
if (canSelect) {
selectedRowIds[id] = true
}
} else {
} else if (canSelect) {
delete selectedRowIds[id]
}
// }
Expand Down