Skip to content

Hidden columns and table search - breaking change #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
staminaloops opened this issue Mar 3, 2016 · 3 comments
Closed

Hidden columns and table search - breaking change #310

staminaloops opened this issue Mar 3, 2016 · 3 comments

Comments

@staminaloops
Copy link
Contributor

Hello @AllenFang
Regarding this commit 1f3798b

This implemented a breaking change.

Let's say we have an array of objects with

const data = [{
id: 1,
name: 'Sofia',
surname: 'Silva',
message: 'This will only show when I click on the row, but I want to be able to search its contents'
},...]

I would do something like:

            <BootstrapTable
              search
              data={data}
              selectRow={selectRowProp}
            >
              <TableHeaderColumn dataField="id" isKey hidden>Id</TableHeaderColumn>
              <TableHeaderColumn dataField="name">Name</TableHeaderColumn>
              <TableHeaderColumn dataField="surname">Surname</TableHeaderColumn>
              <TableHeaderColumn dataField="message" hidden>Message</TableHeaderColumn>
            </BootstrapTable>

The hidden prop just made sense if you where using it to search that hidden column, since if you didnt include it, it wouldnt show anyway. One exception to this was the isKey column.

We can approach this in 2 different ways (and thats why im putting in discussion and not making a PR):

  • Make the hidden fields searchable again. This way, if you want to specify what columns you wants to change, you add them as hidden, if not, you just dont include them on ReactBootstrap table.
  • Or we can make a prop searchable to TableHeaderColumn, and we can specify what columns are searchable.

We have to be careful with the changes that we made, because some may break existing code (like in this case), and should had a major version bump instead of a minor.

What do you think?

@AllenFang
Copy link
Owner

I prefer to take second solution, it's more better and I have planned to make search on hidden column configurable by this discussion

And version issue is a little bit tricky, because I have planned to bump a major version after new table structure(coming soon). if fixing this issue will bump major again. no idea, how do you think? ^^

@staminaloops
Copy link
Contributor Author

Yes, the second solution its better. I was saying that commit 1f3798b was in fact a breaking change - my code stop working as it used to, and we should be careful when we do this kind of changes on minor versions :)

MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards-compatible manner, and
PATCH version when you make backwards-compatible bug fixes.

@staminaloops
Copy link
Contributor Author

I'll do a PR

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

No branches or pull requests

2 participants