Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the Flutter application to null safety and updates dependencies to their latest versions, including Flutter SDK 2.12+ requirement, updated packages (google_fonts, flutter_svg, flutter_local_notifications, etc.), and Android build tools. The changes include extensive code modifications to support null safety with proper nullable type annotations, deprecated API replacements (e.g., accentColor → hintColor, bodyText1 → bodyLarge), and refactored constructors with required parameters.
- Updated Flutter SDK requirement from
>=2.7.0 <3.0.0to>=2.12.0 <=3.0.6to support null safety - Migrated all code to null safety with proper nullable annotations (
?,late,!) - Replaced deprecated Flutter theme properties and text styles with current equivalents
- Updated Android build configuration for compatibility with newer tools
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pubspec.yaml | Updates SDK constraints and bumps all dependencies to null-safe versions |
| lib/theme.dart | Replaces deprecated theme properties (accentColor, backgroundColor, bodyText1/2) with current alternatives |
| lib/size_config.dart | Adds null safety with late keyword for static properties |
| lib/screens/*.dart | Migrates constructors to null safety with proper nullable parameters and required keywords |
| lib/comps/*.dart | Refactors component classes for null safety and constructor improvements |
| lib/notification_manager.dart | Updates notification API to match flutter_local_notifications v16 changes |
| lib/main.dart | Removes flutter_native_timezone dependency and implements alternative timezone configuration |
| android/* | Updates Gradle, Kotlin versions, and Android configuration for compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Future<image.Image> _getImageFromWidget() async { | ||
| RenderRepaintBoundary boundary = | ||
| _globalKey.currentContext.findRenderObject(); | ||
| RenderRepaintBoundary boundary = RenderRepaintBoundary(); |
There was a problem hiding this comment.
This creates a new empty RenderRepaintBoundary instead of retrieving it from the widget tree. The correct implementation should be _globalKey.currentContext!.findRenderObject() as RenderRepaintBoundary. The current code will fail because boundary.size will be null/invalid, and toImage() will not work on an unattached render object.
| startDate: DateTime.parse(pref.getString("startTime"))); | ||
| dailyCigarattes: pref.getInt("dailycigarattes") ?? 0, | ||
| pricePerCigaratte: pref.getDouble("pricePerCigaratte") ?? 0.0, | ||
| startDate: DateTime.parse(pref.getString("startTime") ?? '')); |
There was a problem hiding this comment.
Parsing an empty string with DateTime.parse('') will throw a FormatException. Should use DateTime.tryParse() with a fallback, e.g., startDate: DateTime.tryParse(pref.getString("startTime") ?? '') ?? DateTime.now().
| startDate: DateTime.parse(pref.getString("startTime") ?? '')); | |
| startDate: DateTime.tryParse(pref.getString("startTime") ?? '') ?? DateTime.now()); |
|
|
||
| particles = new ParticleSpawner(size: widget.widgetSize); | ||
| particles.createParticles(3); | ||
| particles = ParticleSpawner(size: widget.widgetSize)..createParticles(3); |
There was a problem hiding this comment.
[nitpick] Using cascade notation to call createParticles immediately after construction couples initialization logic with object creation. Consider moving createParticles(3) to a separate line or creating a factory method for better readability and separation of concerns.
| particles = ParticleSpawner(size: widget.widgetSize)..createParticles(3); | |
| particles = ParticleSpawner(size: widget.widgetSize); | |
| particles.createParticles(3); |
|
|
||
| environment: | ||
| sdk: ">=2.7.0 <3.0.0" | ||
| sdk: ">=2.12.0 <=3.0.6" |
There was a problem hiding this comment.
Using an upper bound constraint <=3.0.6 may prevent the package from working with newer Flutter SDK versions. Consider using <4.0.0 or <3.1.0 instead to allow for patch and minor version updates within the major version range.
| sdk: ">=2.12.0 <=3.0.6" | |
| sdk: ">=2.12.0 <4.0.0" |
| 0, | ||
| title, | ||
| body, | ||
| RepeatInterval.daily, | ||
| platformChannelSpecifics, |
There was a problem hiding this comment.
The periodicallyShow method parameters are missing type annotations. Consider adding them: periodicallyShow({required String title, required String body}) for better type safety and API clarity.
| @@ -11,7 +10,8 @@ const CHANNEL_DESC = "Reminders"; | |||
| class NotificationManager { | |||
| static sendNotification({title, body}) async { | |||
There was a problem hiding this comment.
Parameters title and body are missing type annotations and should be marked as required. Should be: static sendNotification({required String title, required String body}) async.
| ); | ||
| } | ||
|
|
||
| static dailyNotification({title, body}) async { |
There was a problem hiding this comment.
Parameters title and body are missing type annotations and should be marked as required. Should be: static dailyNotification({required String title, required String body}) async.
| static dailyNotification({title, body}) async { | |
| static dailyNotification({required String title, required String body}) async { |
| @@ -67,16 +71,16 @@ class NotificationManager { | |||
|
|
|||
| static scheduleNotification({title, body}) async { | |||
There was a problem hiding this comment.
Parameters title and body are missing type annotations and should be marked as required. Should be: static scheduleNotification({required String title, required String body}) async.
| final locations = tz.timeZoneDatabase.locations; | ||
| final timeZoneName = tz.getLocation(locations.keys.first); | ||
| tz.setLocalLocation(tz.getLocation(timeZoneName.name)); |
There was a problem hiding this comment.
Using locations.keys.first will set an arbitrary timezone instead of the device's actual local timezone. This breaks the app's notification scheduling functionality. Since flutter_native_timezone was removed, consider using an alternative like the timezone package's platform-specific methods or reinstating flutter_native_timezone.
No description provided.