Enhancement/custom head render as child

[PasVV]

This feature adding customHeadLabelRender property

That property can accept FC or any function whose will be used as children
content of TableHeadCell component.

For example, now you can provide any icon as header:

Pros of this feature:

• less code than using customHeadRender
• more predictable and compatible in future than customHeadRender

[coveralls]

Coverage decreased (-0.04%) to 76.597% when pulling f5c7d22 on Lilliance:enhancement/custom-head-render-as-child into f0ea9e5 on gregnb:master.

[gabrielliwerant]

Hm, not sure I understand the use of this feature. It was already possible to use icons for headers before with customHeadRender which is a function that can return any component.

[PasVV]

Hm, not sure I understand the use of this feature. It was already possible to use icons for headers before with customHeadRender which is a function that can return any component.

But using customHeadRender you need reproduce some functionality, for example sort, hint etc. And after that you must maintain your code. I suppose, it can be redundant for most projects.

Please, correct me if i missed something.

[patorjk]

I concur that this is better than customHeadRender. I have something similar in a fork I'm using because customHeadRender requires you too write too much code to replace the header label. Though it might be better to name this function customHeadLabelRender since the content of the header remains the same (the sort arrow, hint, etc), it's just the label that gets overwritten.

Using customHeadRender also breaks the resizableColumns feature, so you can't have resizable columns and override a header label. Additionally, using customHeadRender would potentially break future functionality like column dragging to re-order columns.

[PasVV]

I concur that this is better than customHeadRender. I have something similar in a fork I'm using because customHeadRender requires you too write too much code to replace the header label. Though it might be better to name this function customHeadLabelRender since the content of the header remains the same (the sort arrow, hint, etc), it's just the label that gets overwritten.

Using customHeadRender also breaks the resizableColumns feature, so you can't have resizable columns and override a header label. Additionally, using customHeadRender would potentially break future functionality like column dragging to re-order columns.

Yeah, i agree about property name.
Did it and updated topic post.

[gabrielliwerant]

If I understand correctly, you're just looking to use a custom component for the label only, and keep everything else about the head intact? If so, then we should take a different approach. I'd like to see the label option be capable of rendering a component in addition to a string, as is done now for title in TableToolbar.js.

customHeadRender solves a different problem. It's not about better or worse; both have their place. I just don't think we need an entirely new option here, now that I understand the intent better.

[PasVV]

If I understand correctly, you're just looking to use a custom component for the label only, and keep everything else about the head intact? If so, then we should take a different approach. I'd like to see the label option be capable of rendering a component in addition to a string, as is done now for title in TableToolbar.js.

customHeadRender solves a different problem. It's not about better or worse; both have their place. I just don't think we need an entirely new option here, now that I understand the intent better.

I will try to add label capability to accept FC in addition to a string in next day.

As i see in first look, you found better solution for my problem. I did not inspect source code of your library very well, and i'm not sure how label property may affect to other parts of library. Anyway, i'll provide feedback soon.

[patorjk]

I actually still think a render method is better. Using label can lead to other problems, since the label is used in other places like in the TableViewCol component. You don't nescessarily want to render the same thing there as you do in the table header. Some examples:

• Let's say you want to keep your column header names short due to space (and maybe expand the names when the table window expands). So you use initials in the headers when the window is small, but you always spell out the full name in the TableViewCol options.
• You want some additional flair in the column headers but you don't want it in the TableViewCol list.

[PasVV]

If I understand correctly, you're just looking to use a custom component for the label only, and keep everything else about the head intact? If so, then we should take a different approach. I'd like to see the label option be capable of rendering a component in addition to a string, as is done now for title in TableToolbar.js.

customHeadRender solves a different problem. It's not about better or worse; both have their place. I just don't think we need an entirely new option here, now that I understand the intent better.

I inspected src code of your library, and i concur @patorjk about separated method. It's because you have dependencies of label property and we could miss something important when we will try to add React FC functionality to the label prop.

I added basic support func prop type to column label prop in this PR, but i'm not sure...

[gabrielliwerant]

@Lilliance and @patorjk I understand the objections, I'm thinking about it, and whether or not there's some other way to accomplish this goal with a different approach. I know it's inconvenient to work with custom label renders now, but it is possible, and I'm reluctant to keep adding features ad infinitum. Some things are just going to be possible but difficult, as it depends on the priorities of the library. I know it doesn't seem like a big thing to ask, but there's quite a bit of new features coming in which expands the area of the codebase drip by drip, making it difficult to keep everything tested and running smoothly especially if refactoring is needed.

But thinking about it.

[gabrielliwerant]

Hey @Lilliance, sorry it took a while, but I'm good with this approach. Let's add the option to the documentation and then I think it's good to go!

[PasVV]

I thought about use cases of this PR and noticed one thing.
You may use any React component as column label, like this:

{
  name: 'language',
  // @ts-ignore
  label: <TranslateIcon />,
},

But it's not documented functionality and we need use @ts-ignore, because MUIDataTableColumn type's label prop accept only strings. We should update MUIDataTableColumn and update docs. :)

Thank you for your help!

[gabrielliwerant]

Hey @Lilliance, yes I think for now we're good with label provided it is properly documented to accept a react element. There may be use cases for more complex labels however, that require a custom renderer and a component, but I think those would be uncommon. Any interest in adding the documentation to the other PR you have open?