Skip to content
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

Converted component to a controlled version #9

Merged
merged 1 commit into from
Sep 12, 2018
Merged

Converted component to a controlled version #9

merged 1 commit into from
Sep 12, 2018

Conversation

StanleySathler
Copy link
Contributor

This PR convert the component's code to a controlled version, according to the request on #6. However it is still being discussed.

@willianjusten
Copy link
Contributor

I didn't tested properly yet, but it would be nice to add tests to check the states and see if it works normally.

@StanleySathler
Copy link
Contributor Author

You're definitely right. I'll be providing them soon.

@StanleySathler
Copy link
Contributor Author

StanleySathler commented Sep 1, 2018

@willianjusten I added a few specs in order to test checkbox's checked prop. Do you think is there any other way to make it even better?

Edit
Actually, it looks like CI thrown an issue regarding the defaultChecked removal. I'll check it further and update it properly.

@StanleySathler
Copy link
Contributor Author

@willianjusten I've updated the missing unit tests. Also, an important note: I changed the old Git history for this Pull Request, so, if you have pulled it, just remove the local branch and pull it again. The Git history became really ugly while applying some updates.

@willianjusten
Copy link
Contributor

I'll take a look tomorrow morning, please don't let me forget this.

@StanleySathler
Copy link
Contributor Author

@willianjusten just reminding you again 😋

@willianjusten
Copy link
Contributor

Just tested here, everything works, the code looks fine as well.
I'm not working with this component anymore, but I feel that this will be useful to more people =)

@willianjusten willianjusten merged commit e3bbc4c into lyef:master Sep 12, 2018
@willianjusten
Copy link
Contributor

Release on 2.0.1

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