imballinst / react-bs-datatable

Bootstrap datatable without jQuery. Features include: filter, sort, pagination, checkbox, and control customization.
https://imballinst.github.io/react-bs-datatable
MIT License
60 stars 20 forks source link

Add ability to reset checkboxes #168

Closed Thomas-Boi closed 1 year ago

Thomas-Boi commented 1 year ago

Hello,

Thank you for your work on this! I'd like to request a feature to reset the checkbox state to be all empty aka none-selected. I've been looking into Controlled components in the __stories__ folder to control the state of the checkbox.

image

I tried to replicate what you did but with a checkboxState and a onCheckboxChange: image

image

The checkboxState works as expected. However, the onCheckboxChange doesn't. When I click on a checkbox, there's no console.log. I tried with and without useCallback like the in story but nothing happened.

I'm a bit worried with implementing a custom onCheckboxChange as well. After looking at the code, it looks like the callback would override the default behaviour: image

I'm worried that once I override it, I won't be able to handle the state change since the code looks complex to me. Perhaps there is an easier way to reset the checkbox state? I can hook into the checkbox state via useDataTableWrapper. Perhaps we can expose a method there to set the checkbox state?

If the only way is to override onCheckboxChange, perhaps you can expose the default version of onCheckboxChange? I noticed these lines in DatatableWrapper.tsx but I'm not sure whether onCheckboxChange is the default handler or the one I've overridden. image

Thank you for your time and effort!

imballinst commented 1 year ago

hi @Thomas-Boi, just to confirm, I take it this is for controlled table, right? To be honest, I think I haven't verified it yet whether the current implementation is able to support controlled table checkbox, so this probably is a valid feedback.

Any chance you could provide the sandbox containing your use case? It would be great if I could have it so I could develop the thing with that as the test case. Thanks!

Thomas-Boi commented 1 year ago

Hi @imballinst , thanks for getting back to me so quickly!

I've made a sandbox here. My use case is pretty simple:

In the example, I've set initialState to all-selected and you can see that the checkbox in the header is selected. However, the onChange callback doesn't log anything to the console when I click on them.

I know that there's a BulkCheckboxControl. However, I'd like to have my own styled button to clear the selections.

If possible, I'd just like to have a function just for resetting the state. I've find that the default works great out of the box (store across data updates etc.) so I would prefer not to mess anything up by accident.

imballinst commented 1 year ago

Gotcha @Thomas-Boi, thanks for the thorough explanation and the sandbox! I think I understand it clearer now. I'll see what I can do this weekend then I'll get back to this issue with some PoCs.

Thomas-Boi commented 1 year ago

Thank you! I don't need anything fancy. If the best solution is to make it a controlled component, that's fine with me as long as I can still hook into the default functionality when needed.

imballinst commented 1 year ago

hi @Thomas-Boi, I have published https://www.npmjs.com/package/react-bs-datatable/v/3.9.0-beta.0 and I have made the sandbox here: https://codesandbox.io/s/white-microservice-s0bs6h?file=/src/App.js. Note that this example is uncontrolled table, so you don't actually need to make it controlled.

So I created this new function called useTableCheckboxState. It returns an object, one of the fields is createBulkCheckboxClickHandler. Here's how to use it:

const { createBulkCheckboxClickHandler } = useTableCheckboxState();

return (
  <>
    <button
      onClick={createBulkCheckboxClickHandler("add", {
        idProp: "id",
        checkboxProp: "checkbox"
      })}
    >
      Add all to selection
    </button>

    <button
      onClick={createBulkCheckboxClickHandler("remove", {
        idProp: "id",
        checkboxProp: "checkbox"
      })}
    >
      Clear selection
    </button>
  </>
);

Let me know what you think about it. Thanks!

EDIT: perhaps useTableCheckboxState is a poor name, maybe useCreateCheckboxHandlers is better?

Thomas-Boi commented 1 year ago

Hi @imballinst this looks and works like what I wanted 👍 . As for naming, useCreateCheckboxHandlers seems more specific compared to useTableCheckboxState since this is specifically about handling checkboxes. However, I think either one is fine.

imballinst commented 1 year ago

Sounds good, thanks for the confirmation @Thomas-Boi! useCreateCheckboxHandlers it is. I will tweak the PR, merge it, then publish 3.9.0.

Thomas-Boi commented 1 year ago

Thanks again on getting this done so quickly 😄

imballinst commented 1 year ago

Done! https://www.npmjs.com/package/react-bs-datatable/v/3.9.0. Tested in the sandbox as well for 3.9.0: https://codesandbox.io/s/zen-platform-pq0ud0.

Please let me know if you have other questions. Thanks!