material-react-table icon indicating copy to clipboard operation
material-react-table copied to clipboard

MRT_ShowHideColumnsMenu showAll / hideAll is poorly implemented

Open bigabig opened this issue 1 year ago • 3 comments

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

  • [X] 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.

bigabig avatar Oct 23 '24 08:10 bigabig

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.

bigabig avatar Oct 23 '24 08:10 bigabig

@bigabig , You can try this way:

  React.useEffect(() => {
    dispatch(
      setReduxAction({
        key: 'rowSelection',
        value: table.getState().rowSelection,
      }),
    );
  }, [table.getState().rowSelection]);

dangkhoa99 avatar Nov 05 '24 09:11 dangkhoa99

I encountered an issue and managed to resolve it quickly using Zustand. However, I'm still wondering why wouldn't optimize the process to reproduce the change only once, rather than triggering it multiple times with each update. :)

GabbereX avatar Jan 12 '25 13:01 GabbereX