-
Notifications
You must be signed in to change notification settings - Fork 236
fix: Refactor HomeScreen to use proper Provider pattern #1356
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
base: development
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR refactors state management to use Flutter’s Provider pattern by centralizing provider creation in main.dart and updating HomeScreen to consume providers via context, with lifecycle and listener improvements. Sequence diagram for HomeScreen provider access after refactorsequenceDiagram
participant HomeScreen
participant BuildContext
participant AnimationBadgeProvider
participant SpeedDialProvider
participant BadgeMessageProvider
participant InlineImageProvider
HomeScreen->>BuildContext: build(context)
HomeScreen->>AnimationBadgeProvider: context.watch<AnimationBadgeProvider>()
HomeScreen->>SpeedDialProvider: context.watch<SpeedDialProvider>()
HomeScreen->>BadgeMessageProvider: context.read<BadgeMessageProvider>()
HomeScreen->>InlineImageProvider: context.watch<InlineImageProvider>()
Class diagram for HomeScreen state management refactorclassDiagram
class HomeScreen {
+State<HomeScreen> createState()
}
class _HomeScreenState {
-TabController _tabController
-ImageUtils imageUtils
-InlineImageProvider inlineImageProvider
-TextEditingController inlineimagecontroller
-bool isPrefixIconClicked
-int textfieldLength
-String previousText
-bool isDialInteracting
-String errorVal
-bool _controllerListenerAdded
+void initState()
+void didChangeDependencies()
+void handleTextChange()
+void _controllerListner()
+void dispose()
+void _setPortraitOrientation()
+Future<void> _startImageCaching()
+Widget build(BuildContext context)
+bool get wantKeepAlive
}
HomeScreen -- _HomeScreenState : creates
_HomeScreenState ..> AnimationBadgeProvider : uses via context
_HomeScreenState ..> SpeedDialProvider : uses via context
_HomeScreenState ..> BadgeMessageProvider : uses via context
_HomeScreenState ..> InlineImageProvider : uses via context
_HomeScreenState ..> TextEditingController : uses
_HomeScreenState ..> TabController : uses
_HomeScreenState ..> ImageUtils : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @nope3472 - I've reviewed your changes - here's some feedback:
- In main.dart, use ChangeNotifierProvider for BadgeMessageProvider instead of a plain Provider to ensure UI consumers rebuild when its state changes.
- You’re still pulling InlineImageProvider via GetIt in HomeScreen—drop the GetIt calls and consistently use context.read/watch() now that it’s provided.
- The DefaultTabController wrapper is redundant since you’re passing your own _tabController to TabBar/TabBarView—remove one to simplify the tab setup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In main.dart, use ChangeNotifierProvider for BadgeMessageProvider instead of a plain Provider to ensure UI consumers rebuild when its state changes.
- You’re still pulling InlineImageProvider via GetIt in HomeScreen—drop the GetIt calls and consistently use context.read/watch<InlineImageProvider>() now that it’s provided.
- The DefaultTabController wrapper is redundant since you’re passing your own _tabController to TabBar/TabBarView—remove one to simplify the tab setup.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@nope3472 What is an issue have you created one ? |
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.
@nope3472 I saw you have removed didChangeAppLifecycleState
Have you tested the Refactored HomeScreen where animations and texts resumes automatically when the app is in background ?
@samruddhi-Rahegaonkar yeah will reconfirm the applifecycle state |
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/15994708458/artifacts/3439030634. Screenshots (Android)
Screenshots (iPhone)
Screenshots (iPad)
|
@@ -1,5 +1,8 @@ | |||
import 'package:badgemagic/providers/getitlocator.dart'; |
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.
@nope3472 having all the providers global can make the app slower, and logically the providers should be setup and should be concentarted to files where they are being used.
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.
Using global providers in our app ensures that shared state (like badge data and animations) stays consistent across all screens. It simplifies data flow, avoids duplication, and makes it easier to manage features that interact with each other. Plus, with Provider’s efficient rebuild mechanism, performance isn’t negatively impacted.
fixes (#1358 )
Overview
This PR refactors state management in the BadgeMagic Flutter app to follow Flutter’s recommended Provider pattern.
Changes
main.dart
usingMultiProvider
wantKeepAlive
getter to fix build errorsBenefits
Summary by Sourcery
Refactor state management to follow Flutter’s Provider pattern by centralizing provider setup in main.dart and updating HomeScreen to consume providers via context
Enhancements: