KevinVandy / material-react-table

A fully featured Material UI V5 implementation of TanStack React Table V8, written from the ground up in TypeScript
https://material-react-table.com
MIT License
1.51k stars 436 forks source link

MRT_ShowHideColumnsMenu showAll / hideAll is poorly implemented #1277

Open bigabig opened 1 day ago

bigabig commented 1 day ago

material-react-table version

3.0.1

react & react-dom versions

18.3.1

Describe the bug and the steps to reproduce it

Hi, I noticed that clicking the showAll or hideAll button of the MRT_ShowHideColumnsMenu is not optimized.

If I have 30 columns, it will call the corresponding onRowSelectionChange 30 times. This should be optimized to be only one single call, updating the state only once.

Actually, this is not a problem, when you are using React's useState and provide the setState function. However, with the current implementation, I encounter problems when trying to synchronize with global state (in my case redux), as this will dispatch 30 redux actions (instead of just one).

I have not checked the source code of MRT_ShowHideColumnsMenu, but I think this should be fairly easy to fix / improve? If you do not intend to address this, can you guide me on how to implement MRT_ShowHideColumsnMenu myself?

Thank you!

Minimal, Reproducible Example - (Optional, but Recommended)

const table = useMaterialReactTable({
    data: searchSpace,
    columns: columns,
    state: {
      rowSelection: rowSelectionModel,
    },
    onRowSelectionChange: setRowSelectionModel,
});

with useState this is fine, e.g. const [rowSelectionModel, setRowSelectionModel] = useState<T>(reduxState);

with redux this is not fine, e.g.

const rowSelectionModel = useAppSelector(reduxStateSelectorFn);
const setRowSelectionModel = useCallback(
  (updater: MRT_Updater<T>) => {
      const newState = updater instanceof Function ? updater(rowSelectionModel) : updater;
      dispatch(reduxAction(newState));
    }
  },
  [dispatch, reduxAction],
);

Screenshots or Videos (Optional)

No response

Do you intend to try to help solve this bug with your own PR?

None

Terms

bigabig commented 1 day ago

the reason why it is not fine with redux is that the rowSelectionModel only changes after re-render. As a consequence, only the last of the 30 state updates comes through.