Skip to content

Feat/817 independant tagname - #821

Closed
Dangoo wants to merge 2 commits into
bvaughn:masterfrom
Dangoo:feat/817-independant-tagname
Closed

Dangoo wants to merge 2 commits into
bvaughn:masterfrom
Dangoo:feat/817-independant-tagname

Conversation

@Dangoo

@Dangoo Dangoo commented Sep 25, 2017

Copy link
Copy Markdown

This PR refers to #817 and just improves component Table for now due to good input in #819.

Work includes:

  • added new prop columnHeaderRenderer
  • extracting columnHeaderRenderer as own component and added as default for columnHeaderRenderer
  • renamed _createHeader -> _createColumnHeader
  • provided typings and documentation

Notes:

  • API changes are backward compatible
  • test succede
  • little adjustment to eslint config for indent

For further details pls. see issue.

@bvaughn bvaughn left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance- this looks reasonable. But can we get at least one unit test that verifies this behavior? It would be nice if there was a test that rendered the <table> HTML structure you want so that we could ensure that (a) it works and (b) it doesn't get broken with a future update.

@bvaughn

bvaughn commented Nov 26, 2017

Copy link
Copy Markdown
Owner

Closing due to lack of response to PR feedback.

If you decide to pick this back up I'll be happy to look at it again. 😄 Cheers!

@bvaughn bvaughn closed this Nov 26, 2017
@bvaughn

bvaughn commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Sorry for the very long delay in getting back to you about this!

I've just finished a large update for version 10 of react-virtualized (#1900): the library has been rewritten in TypeScript, every component is now a function component (React 18+ is required), and the source moved from source/ to lib/. Because of that, this PR can't be merged as-is. Version 10's Table still renders each header cell as a plain <div>, so there's still no way to customize that wrapper.

If you're still interested in this change, I'd welcome a new PR with it rebased onto the version-10 branch (or onto master once version 10 has been merged).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants