Address PR comments and add APK workflow #2
Conversation
- Added `.github/workflows/flutter_build.yml` for CI/CD. - Replaced `SearchAnchor` with `SearchBar` in `WeatherHomePage` to remove unreachable code. - Added `intl` package for better date formatting in `WeatherHomePage`. - Improved null safety and error handling for images and data parsing in `ForecastPage` and `WeatherHomePage`. - Enforced theme colors by removing hardcoded `Colors.blue`. - Handled empty hourly forecast state gracefully.
There was a problem hiding this comment.
Pull request overview
This PR addresses previous code review comments and adds CI/CD automation for Flutter APK builds. The changes focus on improving code quality, null safety, error handling, and theme consistency.
- Replaced
SearchAnchorwithSearchBarto eliminate unreachable suggestion builder code - Enhanced error handling with fallback icons for network images and safer date parsing
- Improved theme consistency by replacing hardcoded colors with theme-based colors
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/flutter_build.yml |
Adds CI/CD workflow for building and testing Flutter app, including formatting, analysis, tests, and APK artifact upload |
pubspec.yaml |
Adds intl package dependency for improved date formatting |
lib/screens/weather_home_page.dart |
Refactors SearchAnchor to SearchBar, adds date formatting with intl, and includes error handling for weather icon images |
lib/screens/forecast_page.dart |
Improves null safety with DateTime.tryParse, adds error builders for images, enforces theme colors, and handles empty hourly forecast state |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (t == null) return false; | ||
| return t.isAfter(now); | ||
| }).take(12).map<Map<String, dynamic>>((hour) => { | ||
| 'time': hour['time'].split(' ')[1], |
There was a problem hiding this comment.
After filtering out hours where DateTime.tryParse returns null, the code directly accesses hour['time'].split(' ')[1] in the map function. However, if the hour passed the tryParse check but has an unexpected time format that doesn't contain a space, calling .split(' ')[1] will throw an IndexError. Consider adding validation or using a safer approach like checking the split result length before accessing index 1.
| 'time': hour['time'].split(' ')[1], | |
| 'time': (hour['time'] is String && hour['time'].split(' ').length > 1) | |
| ? hour['time'].split(' ')[1] | |
| : hour['time'], |
|
@copilot fix build workflow |
[WIP] Address PR comments and add APK workflow
.github/workflows/flutter_build.ymlfor CI/CD.SearchAnchorwithSearchBarinWeatherHomePageto remove unreachable code.intlpackage for better date formatting inWeatherHomePage.ForecastPageandWeatherHomePage.Colors.blue.