Conversation
severo
left a comment
There was a problem hiding this comment.
I'm not against allowing more control on how to render the headers, but the PR adds complexity... If we're in a hurry to add features, maybe we can merge, and come back later to it to make it a bit simpler (at least: one way of doing things, in either of the three cases)
|
|
||
| const headerContent = useMemo(() => { | ||
| const { headerComponent } = columnConfig | ||
| if (typeof headerComponent === 'function') { |
There was a problem hiding this comment.
detail: you can declare isFunctionalHeader above, and use it here
|
|
||
| // If the hightable user provides a custom header component, they can choose where to place these controls inside it | ||
| const controls = useMemo(() => | ||
| <div className={styles['ht-header-controls']}> |
There was a problem hiding this comment.
does this mean that we now have two modes? one where the sort indicator is done with pure CSS? and another one, if columnConfig.headerComponent is a function, where it's done with a span with text content (and thus the content cannot be changed with CSS)?
|
I talked to Kenny in person this morning, he said he thinks we should just merge this. Although it is a little complicated to be honest I can't really think of a better way to do it. |
This will fix the issue on Hyperparam.app where the prompt controls overlap the standard column controls. The idea is that the user of hightable can specify a custom function header component and can choose where to place the controls element (which contains sort and column options) within the header, or even choose to exclude it completely. See the Functional Header Component story for an example.