onColumnVisibilityChange behaves strange when clicked the show menu column showAll or hideAll button
shakif95 opened this issue · 5 comments
material-react-table version
v2.12.1
react & react-dom versions
v18.2.0
Describe the bug and the steps to reproduce it
I want to save the user-defined visible columns in my server, hence every time changing the visibility change I want to make an API call. So I used a state variable to keep track of these changes. When changing a specific column, the updater parameter sends all the column names and their visibility status. All works well when it is being handled by the material react table.
However, when managed with a state, for N columns updated function is called N times with each column changes. For example, given an initial state as
{'name'; true, 'age'; true, 'salary': true}
The function is called 3 times with these parameters for HIDE_ALL:
{'name'; false, 'age'; true, 'salary': true}
{'name'; true, 'age'; false, 'salary': true}
{'name'; true, 'age'; true, 'salary': false}
And end up with only the last property hidden.
The expected parameter would be one call with the following object
{'name'; false, 'age'; false, 'salary': false}
Minimal, Reproducible Example - (Optional, but Recommended)
Here is an example using custom states.
https://stackblitz.com/edit/github-afi8ma?file=src%2FTS.tsx
Here is an example without using custom states.
https://stackblitz.com/github/KevinVandy/material-react-table/tree/v2/apps/material-react-table-docs/examples/external-toolbar/sandbox?file=src%2FTS.tsx
Screenshots or Videos (Optional)
Do you intend to try to help solve this bug with your own PR?
Maybe, I'll investigate and start debugging
Terms
- I understand that if my bug cannot be reliably reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
I've had this issue too, and it's due to how react batches state updates, it works well if you pass the prev from the setColumnVisibility to the updater function like this
setColumnVisibility((prev) => updater instanceof Function ? updater(prev) : updater)
Yeah, looks like there is nothing we can patch here in MRT, just have to recommend the ^above fix.
For the scenario where the "Hide all" button is clicked, the columns are toggled like this:
const handleToggleAllColumns = (value?: boolean) => {
getAllLeafColumns()
.filter((col) => col.columnDef.enableHiding !== false)
.forEach((col) => col.toggleVisibility(value));
};
I think it should be using table.toggleAllColumnsVisible
which triggers a single state update, rather than above which triggers multiple state updates for each column.
If that API respects the enable hiding column def option the same way, then sure.
It appears to respect it. https://github.com/TanStack/table/blob/5edd993959967e84766602a9bba908fc6deb4b70/packages/table-core/src/features/ColumnVisibility.ts#L276
table.toggleAllColumnsVisible = value => {
value = value ?? !table.getIsAllColumnsVisible()
table.setColumnVisibility(
table.getAllLeafColumns().reduce(
(obj, column) => ({
...obj,
[column.id]: !value ? !column.getCanHide?.() : value,
}),
{}
)
)
}
// getCanHide called when attempting to hide a column
column.getCanHide = () => {
return (
(column.columnDef.enableHiding ?? true) &&
(table.options.enableHiding ?? true)
)
}
EDIT: I now see that toggleAllColumnsVisible
and filtering out the columns where enableHiding
is false are not the same and switching toggleAllColumnsVisible
will break some edge cases.