-
-
Notifications
You must be signed in to change notification settings - Fork 464
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
MRT_ShowHideColumnsMenu showAll / hideAll is poorly implemented #1277
Comments
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 , You can try this way:
|
The functionality to show/hide all columns within the ShowHideColumnsMenu was implemented with a custom function which filters all columns and toggles the visibility of each column. This calls the `onColumnVisibilityChange` callback multiple times. In my opinion this can be refactored to use the tables `toggleAllColumnsVisible` API, which results in only one call of `onColumnVisibilityChange` and makes the code easier to understand. Fixes: KevinVandy#1277
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. :) |
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)
with useState this is fine, e.g.
const [rowSelectionModel, setRowSelectionModel] = useState<T>(reduxState);
with redux this is not fine, e.g.
Screenshots or Videos (Optional)
No response
Do you intend to try to help solve this bug with your own PR?
None
Terms
The text was updated successfully, but these errors were encountered: