patricktran / react-table-hoc-draggable-columns

ReactTable HOC for draggable columns
https://patricktran.github.io/react-table-hoc-draggable-columns/
MIT License
28 stars 24 forks source link

Using custom accessor throws PropType console error #11

Closed while-rising-ltd closed 4 years ago

while-rising-ltd commented 4 years ago

Hi Patrick,

Cheers for this repo, it works well however it would be great if you could resolve the following issue. React tables (currently using v6) supports custom accessors which are functions, see the documentation:

https://www.npmjs.com/package/react-table-v6#example

Unfortunately your code base assumes accessors will always be strings when setting propTypes and therefore throws a propType console error when custom ones are used. Please change your propTypes account for react table's core functionality.

Here's what you have for line 534 of index.js

/** array of column accessors to allow drag and drop */ draggable: PropTypes.arrayOf(PropTypes.string),

This fix does the trick:

draggable: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.func])),

It does appear that the rest of your code base incidentally supports customs accessors so i don't forsee any additional changes

patricktran commented 4 years ago

Hi @while-rising-ltd, can you please provide some example code?

You are allowed to use a custom accessor as long as you have an id string value defined on the column. You would use the id string value in the draggable prop array.

chidz-1 commented 4 years ago

Nice one @patricktran,

That works great. Do you mind updating your documentation to specify that you can do that? Here would be cool: https://www.npmjs.com/package/react-table-hoc-draggable-columns#draggablecolumns-prop

Thanks

patricktran commented 4 years ago

Updated on the github repo documentation here: https://github.com/patricktran/react-table-hoc-draggable-columns

I did not update on NPMJS because it will require me to publish a new version... I do not want to do this since there are no code changes. Rest assured, the next release will push the updated documentation to NPMJS