add possibility to inject header-row-renderer + typescript type declarations #600
Conversation
with this prop it is possible to inject a cusotm header row with higher order componencts (sortable)
|
Hi @kaoDev, Thank for submitting a PR. I'll be happy to review it as soon as I get the chance, though that might not be for a couple of days. I'm pretty swamped now. I did want to make a quick note that I'm not really willing to take on the maintenance burden of TypeScript type declarations. I understand why they are useful- but I'm not a TypeScript user and I have enough of a challenge maintaining this project on my own as it is. Unless they can be auto-generated as part of the build process- they'll need to live somewhere externally so they can be owned by the community. I hope you understand! |
|
Hey @bvaughn, |
the work on declarations will continue in the definitely typed repo
|
Much appreciated 😄 I'm a fan of TypeScript! I've used it in the past. I mostly stick with Flow these days though because it's easier to consolidate my tech stack with work and OSS projects. |
| </div> | ||
| this.props.headerRowRenderer({ | ||
| className: cn('ReactVirtualized__Table__headerRow', rowClass), | ||
| style: rowStyleObject, |
There was a problem hiding this comment.
I think the style parameter passed should have all of the values preset, just like we pre-append a custom rowClass. This is more inline with what we do for other *renderer props. Users can then override (if they want) specific styles or just pass them through as-is by default.
I'll make this change myself before merging, just commenting to let you know my rationale.
| @@ -25,7 +33,7 @@ export type HeaderRendererParams = { | |||
|
|
|||
| export type RowRendererParams = { | |||
| className: string, | |||
| columns: Array, | |||
| columns: any[], | |||
There was a problem hiding this comment.
Hm, why was this change required?
There was a problem hiding this comment.
hmm I think it just slipped in when I implemented the new types, or perhaps I tried to find an easy to implement more specific type information for this array ;).
This change on it's own, is more or less a matter of taste any[] and Array should be the same in flowType.
There was a problem hiding this comment.
Agreed! I just wanted to ask in case there was a difference I was unaware of. 😄
|
Merged with the change I mentioned. Thanks for the contribution! This will be released sometime this morning. |
This pull request enables the table to handle higher order components in the header row. It is needed to make the columns/headers sortable
Additionally I have added a declaration file for typescript to have better declarations than the @types/react-virtualized repo