icflorescu / mantine-datatable

The table component for your Mantine data-rich applications, supporting asynchronous data loading, column sorting, custom cell data rendering, context menus, nesting, Gmail-style batch row selection, dark theme, and more.
https://icflorescu.github.io/mantine-datatable/
MIT License
949 stars 68 forks source link

New columns don't appear when added to `useDataTableColumns` #596

Closed techied closed 4 months ago

techied commented 6 months ago

Describe the bug When using the useDataTableColumns hook (as described in the Column Toggling Example), new columns added to the columns prop do not appear (in the table or in effectiveColumns) until resetColumnsOrder is called (or the local storage key ${key}-columns-order is cleared)

To Reproduce Steps to reproduce the behavior:

  1. Copy the Column Toggling Example
  2. Load the page
  3. Add a column to the columns prop (one which has not been used before)
  4. Observe that the new column does not appear

Expected behavior The column should appear

Screenshots Happy to provide a screen recording if helpful in reproducing the issue

Desktop (please complete the following information):

nicobermudez commented 5 months ago

Bump. I'm running into this same issue

icflorescu commented 5 months ago

Thanks for raising the issue. Due to some family issues I won't have any spare time over the next week, so I'd be more than grateful if anyone else is willing to take a look at this. @gfazioli, maybe? :-)

gfazioli commented 5 months ago

@nicobermudez Thank you for reporting. I'll try to replicate that and investigate

devkoller commented 5 months ago

I have same issue!

wating for @gfazioli pull request will merge to update mantine datatable.

gfazioli commented 5 months ago

I have same issue!

wating for @gfazioli pull request will merge to update mantine datatable.

PR ready, waiting for @icflorescu approval/merge/release

icflorescu commented 5 months ago

Thanks a lot for taking the time, @gfazioli, you're a star! This will do for now; I'm not yet able to get back to work, but everything seems to be working, so the fix went into 7.10.3.

techied commented 5 months ago

Thank you @gfazioli !

nicobermudez commented 5 months ago

Thank you for the work on this! Will really help us next few weeks 👍

techied commented 5 months ago

Hi @gfazioli , just got around to testing this and it seems like columns are appearing as intended, although the order of the columns passed in to useDataTableColumns is not respected until resetColumnsOrder is called.

My use case does not require column ordering

I wonder if these 2 separate concerns (toggling columns and ordering columns) would be better served by 2 separate hooks? That way the complexity could be reduced

icflorescu commented 4 months ago

Sorry everyone, this issue is now reopened due to a different regression introduced by the fix. Due to a personal issue - my mother recovering in the hospital after a stroke, I haven't slept properly for more than a week and I simply don't have any mental energy and resources left at the moment.

Hopefully @gfazioli or someone else will be able to come up with a new solution soon.

For the benefit of the project/community, I'd be more than grateful for any help maintaining this project over the next couple of weeks 😫🙏

mikkurogue commented 4 months ago

If I have time, I can also take a look at the issue if @gfazioli or @icflorescu do not have the time.

gfazioli commented 4 months ago

If I have time, I can also take a look at the issue if @gfazioli or @icflorescu do not have the time.

you mean https://github.com/icflorescu/mantine-datatable/issues/605? if yes, i was able to create a PR https://github.com/icflorescu/mantine-datatable/pull/608

icflorescu commented 4 months ago

Thanks again, @gfazioli! Your PR was released in 7.11.0. The code in useDataTableColumns.ts definitely needs some cleanup; it would be nice if you, @mikkurogue, could also take a look at it when you have some spare time.

Please make your PRs against the next branch.

Thank you all!

I'll close thisfor now, please feel free to reopen and come up with PRs if necessary.

mikkurogue commented 4 months ago

@icflorescu On first glance, after reading useDataTableColumns.ts, I can't necessarily see much "wrong" other than potential performance increases from the runtime but I'm doubtful this would provide much value in the grand scheme of things. I can still provide a PR later if you feel like looking over it