Skip to content

Conversation

chunming-c
Copy link
Member

fix #235

This PR aims to support customized className and id on BootstrapTable.

@chunming-c
Copy link
Member Author

chunming-c commented Mar 10, 2018

Hi @AllenFang,

For previous customized classes, take rowClasses or example, it accepted String and Function. However, callback function might take a couple of params like row and rowIndex and I thought it might improper and unnecessary for the table. That's the reason why I only support string here.

@chunming-c
Copy link
Member Author

Besides, should I update the docs as well?

@AllenFang
Copy link
Member

Document should be patched only for dev branch, I will update offical docs

return (
<div className="react-bootstrap-table">
<table className={ tableClass }>
<table id={ id } className={ tableClass }>
Copy link
Member

Choose a reason for hiding this comment

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

About #235, I think we need to clarify the class/id is on outter div or table

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Let's discuss further. What I think is that the benefit of customizing className or id on the table directly is that it not only allows users to pick up whatever they want but also supports classNames from boostrap.

For example: bootstrap4#tables

You can also invert the colors—with light text on dark backgrounds—with .table-dark.

<table class="table table-dark">
  { ... }
</table>

@AllenFang
Copy link
Member

Thanks @Chun-MingChen

@AllenFang AllenFang merged commit 94f1a5e into react-bootstrap-table:develop Mar 18, 2018
@AllenFang
Copy link
Member

Patch docs c11539b

@waynebrantley
Copy link

@AllenFang
Why create a classes attribute, instead of using the standard className attribute and merging it in?
Just makes easier to self discover API.

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.

3 participants