feat: Added the screen for draw clipart functionality #976
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new screen for drawing clipart on badges. The implementation refactors the badge drawing logic to use a grid-based approach with a custom painter, adds state management for drawing and erasing, and integrates a new draw badge screen with a navigation drawer. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Jhalakupadhyay - I've reviewed your changes and they look great!
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.
lib/view/drawBadgeScreen.dart
Outdated
| Widget build(BuildContext context) { | ||
| InlineImageProvider drawToggle = Provider.of<InlineImageProvider>(context); | ||
| SystemChrome.setEnabledSystemUIMode(SystemUiMode.immersiveSticky); | ||
| SystemChrome.setPreferredOrientations([ |
There was a problem hiding this comment.
suggestion: Use OrientationBuilder for more robust orientation management
Instead of manually setting orientations, consider using OrientationBuilder for more robust and flexible orientation management across different parts of the app.
| SystemChrome.setPreferredOrientations([ | |
| return OrientationBuilder( | |
| builder: (context, orientation) { | |
| if (orientation == Orientation.landscape) { | |
| SystemChrome.setEnabledSystemUIMode(SystemUiMode.immersiveSticky); | |
| } | |
| return Scaffold( | |
| // Your existing widget tree here | |
| ); | |
| }, | |
| ); |
lib/view/drawBadgeScreen.dart
Outdated
| Container( | ||
| margin: EdgeInsets.symmetric(vertical: 20.h, horizontal: 20.w), | ||
| padding: EdgeInsets.all(10.dg), | ||
| height: 400.h, |
There was a problem hiding this comment.
suggestion: Use more flexible sizing instead of fixed dimensions
The use of fixed sizes with ScreenUtil might cause issues on different device sizes. Consider using more flexible sizing approaches, such as AspectRatio or LayoutBuilder, to ensure proper rendering across various devices.
| height: 400.h, | |
| height: MediaQuery.of(context).size.height * 0.5, |
| InlineImageProvider cellStateToggle = | ||
| Provider.of<InlineImageProvider>(context); | ||
| return CustomPaint( | ||
| size: const Size(400, 480), |
There was a problem hiding this comment.
suggestion (performance): Use available space instead of fixed size for CustomPaint
Using a fixed size for the CustomPaint widget could cause rendering issues on different screen sizes. Consider using the available space or a responsive sizing approach to ensure the badge scales properly across different devices.
| size: const Size(400, 480), | |
| return LayoutBuilder( | |
| builder: (context, constraints) { | |
| return CustomPaint( | |
| size: Size(constraints.maxWidth, constraints.maxHeight), | |
| painter: BadgePainter(grid: cellStateToggle.getGrid()), | |
| ); | |
| }, | |
| ); |
lib/virtualbadge/view/cell.dart
Outdated
| @override | ||
| State<Cell> createState() => _CellState(); | ||
| } | ||
| class BadgePainter extends CustomPainter { |
There was a problem hiding this comment.
Rename the class to badge_painter.dart
lib/view/draw_badge_screen.dart
Outdated
| appBar: AppBar( | ||
| leading: Builder(builder: (context) { | ||
| return IconButton( | ||
| onPressed: () { | ||
| Scaffold.of(context).openDrawer(); | ||
| }, | ||
| icon: const Icon( | ||
| Icons.menu, | ||
| color: Colors.white, | ||
| )); | ||
| }), | ||
| backgroundColor: Colors.red, | ||
| title: const Text( | ||
| 'Badge Magic', | ||
| style: TextStyle(color: Colors.white), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
You should create a common class which will encapsulate this logic, rather than duplicating it. We need to manage the drawer from one class. You can then have these hooks which inject the content and creates the entire page for you
#975 #978
Summary by Sourcery
This pull request introduces a new screen for drawing clipart, complete with a custom painter for rendering a grid-based badge. It also adds a navigation drawer to the home screen for easier navigation and refactors the badge rendering logic for improved flexibility.