-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add toggle all button #301
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
Conversation
Here's the code health analysis summary for commits Analysis Summary
|
@igoroctaviano FYI: It's impossible to satisfy all of the DeepSource requirements My first commit had "this.handleVisibilityChange.bind(this)" and it said I shouldn't have .bind because it would be recalculated on every re-render Then I changed it to (checked: boolean) => this.handleVisibilityChange(checked), and it said I shouldn't have a arrow function because of the same problem. Finally, I used the constructor to bind the handleVisibilityChange function on the instantiation of the object, but now it's complaining that I shouldn't use "this.handleVisibilityChange" without binding it. So it's impossible to satisfy all of its requirements |
@pedrokohler cau take a look at deep source check? |
@igoroctaviano have you seen my previous comment? |
@pedrokohler have you tried:
This does not require a constructor or bind, and it does not create an arrow function all the time, since it's a class assignment. |
|
@igoroctaviano everything is passing now. |
Fixes #288