-
Notifications
You must be signed in to change notification settings - Fork 213
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
feat: Added the code for the left scroll animation. #997
feat: Added the code for the left scroll animation. #997
Conversation
…badge. feat: Added the modes support for left right top down fixed and animation support for flash and marquee. animation right
…badge. feat: Added the modes support for left right top down fixed and animation support for flash and marquee. animation right
Reviewer's Guide by SourceryThis pull request implements the left scroll animation for the badge display. It introduces new utility functions for byte array and binary conversions, updates the badge painting logic, and implements an animation system with speed control. File-Level Changes
Tips
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Jhalakupadhyay - I've reviewed your changes - here's some feedback:
Overall Comments:
- The new animation system in DrawBadgeProvider is a good improvement, but consider breaking it down into smaller, more focused classes to reduce complexity.
- Ensure that the new timer-based animation system is optimized for performance and battery life, especially on mobile devices.
- Consider implementing the missing animation types (UpAnimation, DownAnimation, etc.) to fully utilize the new BadgeAnimation abstract class.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
return binaryString; | ||
} | ||
|
||
List<List<int>> binaryStringTo2DList(String binaryString) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (performance): Simplify and optimize binaryStringTo2DList function
The current implementation uses complex nested loops. Consider rewriting this function to use a single loop with index manipulation for better efficiency and readability.
List<List<int>> binaryStringTo2DList(String binaryString) {
final int maxHeight = 11;
final int width = binaryString.length ~/ maxHeight;
return List.generate(maxHeight, (i) =>
binaryString.substring(i * width, (i + 1) * width).split('').map(int.parse).toList()
);
}
..color = Colors.grey.shade800 | ||
..style = PaintingStyle.fill; | ||
|
||
double radians = math.pi / 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (performance): Use compile-time constant for rotation
For better precision and potentially faster execution, consider using a compile-time constant for the rotation value instead of calculating it at runtime.
double radians = math.pi / 4; | |
const double radians = math.pi / 4; |
lib/view/homescreen.dart
Outdated
_startImageCaching(); | ||
super.initState(); | ||
|
||
_tabController = TabController(length: 3, vsync: this); | ||
} | ||
|
||
void _controllerListner() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (performance): Optimize animation updates in _controllerListner
Consider implementing debouncing for the animation calls to prevent excessive updates during rapid text changes. Also, review the relationship between this method and controllerListener to avoid potential circular dependencies or unnecessary double updates.
void _controllerListener() {
final debouncer = Debouncer(milliseconds: 300);
debouncer.run(() {
if (mounted) {
logger.d('Controller Listener: ${inlineImageProvider.getController().text}');
}
});
}
#995
Summary by Sourcery
Add new badge animation features, including a right scroll animation and a singleton token generator. Refactor the BadgePainter for optimized repainting and replace DrawBadgeProvider with BadgeViewProvider. Introduce constants for animation speed and remove unnecessary debug logging.
New Features:
Enhancements:
Chores: