Speed improvements and added more config #6
Conversation
The widget now uses animatedBuilders, responds to rebuilds with new parameters and adds two fields: curve and isAnimating. The curve is for the alpha and the isAnimating is a flag to animate the glow or not.
|
Thanks for opening this PR. Will review soon. |
Previously only created the animation if the isAnimating flag was true, but that would lead the AnimatedBuilder to throw since its animation was expected to not be null
didUpdateWidget will now only update the animation if animation related values are changed.
Had a mistake where I'd set the `controller.duration` to the `widget.controller`, but as that might not always be provided by, it'd go to null and error. The duration is now defaulted in the constructor list
lib/avatar_glow.dart
Outdated
| final BoxShape shape; | ||
| final Duration duration; | ||
| final bool repeat; | ||
| final bool isAnimating; |
There was a problem hiding this comment.
The field name can be changed to something more meaningful, like a normal final bool animate. Just my views!
There was a problem hiding this comment.
Changed to just animate 👍
There was a problem hiding this comment.
@Nolence Btw do we really need this animate field? As one will only use this package when he wants to show animation on his avatar. If this field is intended for play/pausing the animation, then I think the name should be changed again to something more meaningful.
There was a problem hiding this comment.
A previous issue wanted this feature and I currently use it for drawing attention to dragTargets that are willing to take an item (by toggling the GlowAvatar around them) so I don't think users will just use this package for avatars
I don't have a preference on naming so long as others can understand the field. I can also add doc strings to all the fields in case we need more accessibility
There was a problem hiding this comment.
@apgapg we can merge the changes now.
|
@apgapg Apart from that field name, LGTM 👍 |
Renamed `isAnimating` -> `animate`. Also removed the 2 assignments inside of the `build` method (e.g. `final glowColor = widget.glowColor;`). The issue must've been a local thing as their initial warnings are now gone.
|
Thanks @Nolence @xsahil03x for the contribution and feedback. Did a fantastic job |
|
|
Hi! I love the package. My changes started out from wanting to avoid calling setState after reading this article: https://medium.com/flutter-community/flutter-laggy-animations-how-not-to-setstate-f2dd9873b8fc . It now uses animatedBuilders, responds to rebuilds with new parameters via didUpdateWidget and adds two fields: curve and isAnimating. The curve is for the alpha and the isAnimating is a flag to animate the glow or not.