feat: add announcements feature#900
Conversation
Commands
|
There was a problem hiding this comment.
Hello @tatsutakein, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request. This PR introduces a new 'Announcements' feature to the application. It includes creating a dedicated feature module (feature_announcement), adding a new screen to list announcements, implementing a reusable component for displaying individual announcements with type indicators and a 'NEW' badge, and integrating this new screen into the navigation flow from the 'My Page' screen. The feature also includes initial localization support for English and Japanese.
Highlights
- New Feature Module: A new feature module,
feature_announcement, has been added to encapsulate the announcements functionality. - Announcements List Screen: A new screen (
AnnouncementListScreen) is introduced to display a list of announcements. It currently uses mock data but includes placeholders for integrating with a data provider and pull-to-refresh. - Reusable Announcement Card: A new
AnnouncementCardwidget is created to display individual announcements, supporting different types (info, warning, update) with visual styling and a 'NEW' badge. - Navigation Integration: An 'Announcements' menu item has been added to the 'My Page' screen, providing navigation to the new list screen via GoRouter.
- Localization: Localization support for the new feature and the My Page menu item has been added for English and Japanese.
Changelog
Click here to see the changelog
- app/mobile/lib/router/app_router.dart
- Imported the new
feature_announcementmodule. - Added a new GoRoute
/my/announcementsnamedAppRoutes.announcementsunder the My Page route, pointing toAnnouncementListScreen.
- Imported the new
- app/mobile/lib/router/app_routes.dart
- Added a new static constant
announcementsfor the route name.
- Added a new static constant
- app/mobile/pubspec.yaml
- Added
feature_announcementas a dependency.
- Added
- feature/announcement/.flutter-plugins-dependencies
- Added generated file listing plugin dependencies for the new module.
- feature/announcement/README.md
- Added README file describing the new feature module, its purpose, dependencies, components, navigation, state management approach (Riverpod), localization, and development steps.
- feature/announcement/l10n.yaml
- Added localization configuration file for the
feature_announcementmodule.
- Added localization configuration file for the
- feature/announcement/lib/feature_announcement.dart
- Added main export file for the module, exporting
AnnouncementListScreen.
- Added main export file for the module, exporting
- feature/announcement/lib/src/gen/l10n/l10n.dart
- Added generated localization code for the
feature_announcementmodule.
- Added generated localization code for the
- feature/announcement/lib/src/gen/l10n/l10n_en.dart
- Added generated English localization strings for the module.
- feature/announcement/lib/src/gen/l10n/l10n_ja.dart
- Added generated Japanese localization strings for the module.
- feature/announcement/lib/src/l10n/app_en.arb
- Added English ARB file with strings for announcement titles, empty state, types, and new badge.
- feature/announcement/lib/src/l10n/app_ja.arb
- Added Japanese ARB file with strings for announcement titles, empty state, types, and new badge.
- feature/announcement/lib/src/ui/component/announcement_card.dart
- Added
AnnouncementCardwidget to display individual announcements. - Includes styling based on announcement type (info, warning, update).
- Adds a 'NEW' badge for new announcements.
- Includes a TODO for navigating to a detail screen on tap.
- Added
- feature/announcement/lib/src/ui/screen/announcement_list_screen.dart
- Added
AnnouncementListScreenwidget. - Displays a list of
AnnouncementCardwidgets. - Uses mock data temporarily (with a TODO to replace).
- Includes a
RefreshIndicatorwith a placeholder refresh logic (with a TODO to implement).
- Added
- feature/announcement/pubspec.yaml
- Added
pubspec.yamlfor the newfeature_announcementmodule, defining its dependencies and dev dependencies.
- Added
- feature/my/lib/src/gen/l10n/l10n.dart
- Added generated localization code for the new 'Announcements' menu item in the My Page feature.
- feature/my/lib/src/gen/l10n/l10n_en.dart
- Added generated English localization string for the 'Announcements' menu item.
- feature/my/lib/src/gen/l10n/l10n_ja.dart
- Added generated Japanese localization string for the 'Announcements' menu item.
- feature/my/lib/src/l10n/app_en.arb
- Added English ARB string for the 'Announcements' menu item.
- feature/my/lib/src/l10n/app_ja.arb
- Added Japanese ARB string for the 'Announcements' menu item.
- feature/my/lib/src/ui/screen/my/my_screen.dart
- Added a new
ListTilefor 'Announcements' to the My Screen menu. - Configured the ListTile to navigate to the new announcements route.
- Added a new
- pubspec.yaml
- Added the new
feature/announcementmodule to the workspace definition.
- Added the new
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
New feature takes flight,
Announcements now in sight,
Code review awaits.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request effectively introduces the new announcements feature. The structure of the feature_announcement module is well-organized, including localization, routing, and basic UI components. The use of mock data for the initial implementation is clearly marked and understandable.
I've identified a few areas for improvement, primarily related to type safety, use of magic strings, and handling of generated files. Addressing these will enhance the robustness and maintainability of the new feature.
Overall, good work on setting up this new feature module!
Summary of Findings
- Version Control of Generated Files: The file
feature/announcement/.flutter-plugins-dependenciescontains user-specific paths and appears to be generated. It's recommended to add such files to.gitignore. - Type Safety in UI Components: In
AnnouncementCard, the use ofMap<String, dynamic>fortypeDataand subsequent casting can be error-prone. Consider using a dedicated class or record for better type safety. - Use of Magic Strings: String literals are used for announcement types in
AnnouncementCard. Using enums or string constants would improve maintainability and reduce the risk of typos. - Data Handling with Mock Data: In
AnnouncementListScreen, force unwrapping (!) is used when accessing mock data from a map. This could lead to runtime errors. Also, theisNewfield is stored as a string in mock data and converted to a boolean, which could be more robustly handled by storing it as a boolean directly or using a simple data model. - README Content: The README for
feature_announcementmentions features like 'Read/unread status' and 'Real-time updates' which are not yet implemented with the current mock data. This is acceptable for an initial feature overview but worth noting for future development alignment. (Low Severity - Not commented directly) - TODO Comments: Several TODO comments are present (e.g., navigating to detail, implementing refresh logic). These are good placeholders for future work. (Low Severity - Not commented directly)
Merge Readiness
The pull request lays a solid foundation for the announcements feature. However, there are several medium-severity issues related to type safety, magic strings, and version control of generated files that should be addressed to improve code quality and maintainability. I recommend making these changes before merging. As an AI, I am not authorized to approve pull requests; please ensure further review and approval from team members.
| @@ -0,0 +1 @@ | |||
| {"info":"This is a generated file; do not edit or check into version control.","plugins":{"ios":[{"name":"firebase_analytics","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_analytics-11.4.6/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"firebase_auth","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_auth-5.5.4/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"firebase_core","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_core-3.13.1/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"firebase_remote_config","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_remote_config-5.4.4/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"image_picker_ios","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/image_picker_ios-0.8.12+2/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"package_info_plus","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/package_info_plus-8.3.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"path_provider_foundation","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/path_provider_foundation-2.4.1/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false},{"name":"shared_preferences_foundation","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/shared_preferences_foundation-2.5.4/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false},{"name":"sqlite3_flutter_libs","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/sqlite3_flutter_libs-0.5.33/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false},{"name":"url_launcher_ios","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/url_launcher_ios-6.3.2/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"video_player_avfoundation","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/video_player_avfoundation-2.6.5/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false}],"android":[{"name":"dynamic_color","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/dynamic_color-1.7.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"firebase_analytics","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_analytics-11.4.6/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"firebase_auth","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_auth-5.5.4/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"firebase_core","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_core-3.13.1/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"firebase_remote_config","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_remote_config-5.4.4/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"flutter_plugin_android_lifecycle","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/flutter_plugin_android_lifecycle-2.0.28/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"image_picker_android","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/image_picker_android-0.8.12+23/","native_build":true,"dependencies":["flutter_plugin_android_lifecycle"],"dev_dependency":false},{"name":"package_info_plus","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/package_info_plus-8.3.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"path_provider_android","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/path_provider_android-2.2.15/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"shared_preferences_android","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/shared_preferences_android-2.4.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"sqlite3_flutter_libs","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/sqlite3_flutter_libs-0.5.33/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"url_launcher_android","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/url_launcher_android-6.3.14/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"video_player_android","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/video_player_android-2.7.17/","native_build":true,"dependencies":[],"dev_dependency":false}],"macos":[{"name":"dynamic_color","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/dynamic_color-1.7.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"file_selector_macos","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/file_selector_macos-0.9.4+3/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"firebase_analytics","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_analytics-11.4.6/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"firebase_auth","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_auth-5.5.4/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"firebase_core","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_core-3.13.1/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"firebase_remote_config","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_remote_config-5.4.4/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"image_picker_macos","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/image_picker_macos-0.2.1+2/","native_build":false,"dependencies":["file_selector_macos"],"dev_dependency":false},{"name":"package_info_plus","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/package_info_plus-8.3.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"path_provider_foundation","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/path_provider_foundation-2.4.1/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false},{"name":"shared_preferences_foundation","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/shared_preferences_foundation-2.5.4/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false},{"name":"sqlite3_flutter_libs","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/sqlite3_flutter_libs-0.5.33/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false},{"name":"url_launcher_macos","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/url_launcher_macos-3.2.2/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"video_player_avfoundation","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/video_player_avfoundation-2.6.5/","shared_darwin_source":true,"native_build":true,"dependencies":[],"dev_dependency":false}],"linux":[{"name":"dynamic_color","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/dynamic_color-1.7.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"file_selector_linux","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/file_selector_linux-0.9.3+2/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"image_picker_linux","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/image_picker_linux-0.2.1+2/","native_build":false,"dependencies":["file_selector_linux"],"dev_dependency":false},{"name":"package_info_plus","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/package_info_plus-8.3.0/","native_build":false,"dependencies":[],"dev_dependency":false},{"name":"path_provider_linux","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/path_provider_linux-2.2.1/","native_build":false,"dependencies":[],"dev_dependency":false},{"name":"shared_preferences_linux","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/shared_preferences_linux-2.4.1/","native_build":false,"dependencies":["path_provider_linux"],"dev_dependency":false},{"name":"sqlite3_flutter_libs","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/sqlite3_flutter_libs-0.5.33/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"url_launcher_linux","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/url_launcher_linux-3.2.1/","native_build":true,"dependencies":[],"dev_dependency":false}],"windows":[{"name":"dynamic_color","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/dynamic_color-1.7.0/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"file_selector_windows","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/file_selector_windows-0.9.3+4/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"firebase_auth","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_auth-5.5.4/","native_build":true,"dependencies":["firebase_core"],"dev_dependency":false},{"name":"firebase_core","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_core-3.13.1/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"image_picker_windows","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/image_picker_windows-0.2.1+1/","native_build":false,"dependencies":["file_selector_windows"],"dev_dependency":false},{"name":"package_info_plus","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/package_info_plus-8.3.0/","native_build":false,"dependencies":[],"dev_dependency":false},{"name":"path_provider_windows","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/path_provider_windows-2.3.0/","native_build":false,"dependencies":[],"dev_dependency":false},{"name":"shared_preferences_windows","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/shared_preferences_windows-2.4.1/","native_build":false,"dependencies":["path_provider_windows"],"dev_dependency":false},{"name":"sqlite3_flutter_libs","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/sqlite3_flutter_libs-0.5.33/","native_build":true,"dependencies":[],"dev_dependency":false},{"name":"url_launcher_windows","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/url_launcher_windows-3.1.3/","native_build":true,"dependencies":[],"dev_dependency":false}],"web":[{"name":"firebase_analytics_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_analytics_web-0.5.10+12/","dependencies":["firebase_core_web"],"dev_dependency":false},{"name":"firebase_auth_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_auth_web-5.14.3/","dependencies":["firebase_core_web"],"dev_dependency":false},{"name":"firebase_core_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_core_web-2.23.0/","dependencies":[],"dev_dependency":false},{"name":"firebase_remote_config_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/firebase_remote_config_web-1.8.4/","dependencies":["firebase_core_web"],"dev_dependency":false},{"name":"image_picker_for_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/image_picker_for_web-3.0.6/","dependencies":[],"dev_dependency":false},{"name":"package_info_plus","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/package_info_plus-8.3.0/","dependencies":[],"dev_dependency":false},{"name":"shared_preferences_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/shared_preferences_web-2.4.2/","dependencies":[],"dev_dependency":false},{"name":"url_launcher_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/url_launcher_web-2.3.3/","dependencies":[],"dev_dependency":false},{"name":"video_player_web","path":"/Users/tatsutakein/.pub-cache/hosted/pub.dev/video_player_web-2.3.3/","dependencies":[],"dev_dependency":false}]},"dependencyGraph":[{"name":"dynamic_color","dependencies":[]},{"name":"file_selector_linux","dependencies":[]},{"name":"file_selector_macos","dependencies":[]},{"name":"file_selector_windows","dependencies":[]},{"name":"firebase_analytics","dependencies":["firebase_analytics_web","firebase_core"]},{"name":"firebase_analytics_web","dependencies":["firebase_core","firebase_core_web"]},{"name":"firebase_auth","dependencies":["firebase_auth_web","firebase_core"]},{"name":"firebase_auth_web","dependencies":["firebase_core","firebase_core_web"]},{"name":"firebase_core","dependencies":["firebase_core_web"]},{"name":"firebase_core_web","dependencies":[]},{"name":"firebase_remote_config","dependencies":["firebase_core","firebase_remote_config_web"]},{"name":"firebase_remote_config_web","dependencies":["firebase_core","firebase_core_web"]},{"name":"flutter_plugin_android_lifecycle","dependencies":[]},{"name":"image_picker","dependencies":["image_picker_android","image_picker_for_web","image_picker_ios","image_picker_linux","image_picker_macos","image_picker_windows"]},{"name":"image_picker_android","dependencies":["flutter_plugin_android_lifecycle"]},{"name":"image_picker_for_web","dependencies":[]},{"name":"image_picker_ios","dependencies":[]},{"name":"image_picker_linux","dependencies":["file_selector_linux"]},{"name":"image_picker_macos","dependencies":["file_selector_macos"]},{"name":"image_picker_windows","dependencies":["file_selector_windows"]},{"name":"package_info_plus","dependencies":[]},{"name":"path_provider","dependencies":["path_provider_android","path_provider_foundation","path_provider_linux","path_provider_windows"]},{"name":"path_provider_android","dependencies":[]},{"name":"path_provider_foundation","dependencies":[]},{"name":"path_provider_linux","dependencies":[]},{"name":"path_provider_windows","dependencies":[]},{"name":"shared_preferences","dependencies":["shared_preferences_android","shared_preferences_foundation","shared_preferences_linux","shared_preferences_web","shared_preferences_windows"]},{"name":"shared_preferences_android","dependencies":[]},{"name":"shared_preferences_foundation","dependencies":[]},{"name":"shared_preferences_linux","dependencies":["path_provider_linux"]},{"name":"shared_preferences_web","dependencies":[]},{"name":"shared_preferences_windows","dependencies":["path_provider_windows"]},{"name":"sqlite3_flutter_libs","dependencies":[]},{"name":"url_launcher","dependencies":["url_launcher_android","url_launcher_ios","url_launcher_linux","url_launcher_macos","url_launcher_web","url_launcher_windows"]},{"name":"url_launcher_android","dependencies":[]},{"name":"url_launcher_ios","dependencies":[]},{"name":"url_launcher_linux","dependencies":[]},{"name":"url_launcher_macos","dependencies":[]},{"name":"url_launcher_web","dependencies":[]},{"name":"url_launcher_windows","dependencies":[]},{"name":"video_player","dependencies":["video_player_android","video_player_avfoundation","video_player_web"]},{"name":"video_player_android","dependencies":[]},{"name":"video_player_avfoundation","dependencies":[]},{"name":"video_player_web","dependencies":[]}],"date_created":"2025-05-29 23:11:20.104743","version":"3.32.0","swift_package_manager_enabled":{"ios":false,"macos":false}} No newline at end of file | |||
There was a problem hiding this comment.
This file appears to be a generated file containing user-specific paths (e.g., /Users/tatsutakein/...). Such files are generally not recommended for version control as they can lead to merge conflicts and unnecessary diffs for different developers.
Could this file be added to the project's .gitignore file to prevent it from being tracked?
| Map<String, dynamic> _getTypeData(BuildContext context) { | ||
| final theme = Theme.of(context); | ||
| final l10n = FeatureAnnouncementL10n.of(context); | ||
|
|
||
| switch (type) { | ||
| case 'info': | ||
| return { | ||
| 'icon': Icons.info_outline, | ||
| 'label': l10n?.announcementTypeInfo ?? 'Information', | ||
| 'color': theme.colorScheme.primaryContainer, | ||
| 'textColor': theme.colorScheme.onPrimaryContainer, | ||
| }; | ||
| case 'warning': | ||
| return { | ||
| 'icon': Icons.warning_amber_outlined, | ||
| 'label': l10n?.announcementTypeWarning ?? 'Important', | ||
| 'color': theme.colorScheme.errorContainer, | ||
| 'textColor': theme.colorScheme.onErrorContainer, | ||
| }; | ||
| case 'update': | ||
| return { | ||
| 'icon': Icons.system_update_outlined, | ||
| 'label': l10n?.announcementTypeUpdate ?? 'Update', | ||
| 'color': theme.colorScheme.tertiaryContainer, | ||
| 'textColor': theme.colorScheme.onTertiaryContainer, | ||
| }; | ||
| default: | ||
| return { | ||
| 'icon': Icons.info_outline, | ||
| 'label': l10n?.announcementTypeInfo ?? 'Information', | ||
| 'color': theme.colorScheme.surfaceContainerHighest, | ||
| 'textColor': theme.colorScheme.onSurfaceVariant, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
The _getTypeData method returns a Map<String, dynamic> and relies on string keys ('icon', 'label', 'color', 'textColor') and type casting (e.g., typeData['color'] as Color). This approach can be prone to runtime errors if keys are misspelled or types are incorrect.
To improve type safety and maintainability, have you considered defining a small helper class or a record to represent TypeData? This would provide compile-time checks and better autocompletion.
For example, a record could look like:
({IconData icon, String label, Color color, Color textColor}) _getTypeData(BuildContext context) {
// ...
switch (type) {
case 'info':
return (
icon: Icons.info_outline,
label: l10n?.announcementTypeInfo ?? 'Information',
color: theme.colorScheme.primaryContainer,
textColor: theme.colorScheme.onPrimaryContainer,
);
// ... other cases
}
}Then, accessing properties would be type-safe, e.g., typeData.icon, typeData.label.
| switch (type) { | ||
| case 'info': | ||
| return { | ||
| 'icon': Icons.info_outline, | ||
| 'label': l10n?.announcementTypeInfo ?? 'Information', | ||
| 'color': theme.colorScheme.primaryContainer, | ||
| 'textColor': theme.colorScheme.onPrimaryContainer, | ||
| }; | ||
| case 'warning': | ||
| return { | ||
| 'icon': Icons.warning_amber_outlined, | ||
| 'label': l10n?.announcementTypeWarning ?? 'Important', | ||
| 'color': theme.colorScheme.errorContainer, | ||
| 'textColor': theme.colorScheme.onErrorContainer, | ||
| }; | ||
| case 'update': | ||
| return { | ||
| 'icon': Icons.system_update_outlined, | ||
| 'label': l10n?.announcementTypeUpdate ?? 'Update', | ||
| 'color': theme.colorScheme.tertiaryContainer, | ||
| 'textColor': theme.colorScheme.onTertiaryContainer, | ||
| }; | ||
| default: |
There was a problem hiding this comment.
The switch (type) statement uses string literals like 'info', 'warning', and 'update' for different announcement types. This can be error-prone (typos) and makes it harder to manage the available types.
Would it be beneficial to define these types as an enum or at least as static const String constants within the AnnouncementCard or a dedicated constants file? This would improve type safety and make the code more self-documenting.
For example, using an enum:
enum AnnouncementType {
info,
warning,
update,
unknown // for the default case
}
// In the widget, parse the string to the enum
// final AnnouncementType announcementType = AnnouncementType.values.firstWhere(
// (e) => e.toString().split('.').last == type,
// orElse: () => AnnouncementType.unknown,
// );
// switch (announcementType) { ... }This would also make the default case in your _getTypeData more explicit if you map it to AnnouncementType.unknown.
| title: announcement['title']!, | ||
| description: announcement['description']!, | ||
| type: announcement['type']!, | ||
| date: announcement['date']!, | ||
| isNew: announcement['isNew'] == 'true', |
There was a problem hiding this comment.
Accessing values from the announcement map (which is Map<String, String>) uses the force unwrap operator (!), for example, announcement['title']!. While this might be acceptable for tightly controlled mock data, it can lead to runtime exceptions if a key is missing or misspelled.
To make this more robust, even for mock data, have you considered:
- Using
announcement['key'] ?? 'default_value'if a fallback is acceptable? - Defining a simple data class/model for
Announcement(even if it's just for the mock data initially) and then populating aList<Announcement>? This would provide type safety and better autocompletion.
For example, a simple class:
class _MockAnnouncement {
final String title;
final String description;
final String type;
final String date;
final bool isNew;
_MockAnnouncement({
required this.title,
required this.description,
required this.type,
required this.date,
required this.isNew,
});
}
// Then _getMockAnnouncements would return List<_MockAnnouncement>
// And you'd access properties like: announcement.titleThis also helps with the isNew field, which is currently a string 'true'/'false' in the map but is used as a boolean.
| } | ||
|
|
||
| // TODO: Remove mock data and use actual provider | ||
| List<Map<String, String>> _getMockAnnouncements() { |
There was a problem hiding this comment.
The _getMockAnnouncements method returns List<Map<String, String>>. However, the isNew field is stored as a string (e.g., 'true', 'false') within this map. In AnnouncementCard, this string is then converted to a boolean using announcement['isNew'] == 'true'.
This string representation for a boolean value can be a bit fragile. Would it be clearer to store isNew directly as a boolean in your mock data structure? If you adopt a simple model class for announcements as suggested in another comment, this would naturally be handled by defining isNew as a bool type in the class.
If sticking with Map<String, dynamic> for now, you could change the map to List<Map<String, dynamic>> and store isNew as a boolean:
List<Map<String, dynamic>> _getMockAnnouncements() {
return [
{
'title': 'New Feature: Quest Sharing',
'description': 'You can now share your quests with friends and family!',
'type': 'update',
'date': '2024-05-28',
'isNew': true, // Store as boolean
},
// ...
];
}
// Then in AnnouncementCard:
// isNew: announcement['isNew'] as bool? ?? false,- Add announcements menu item with notifications icon - Add localization strings for English and Japanese - Navigate to announcements route when tapped 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Create announcement list screen with mock data - Add announcement card component with type-based styling - Support info, warning, and update announcement types - Implement NEW badge for recent announcements - Export announcement screen in feature module 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add English translations for announcement UI - Add Japanese translations for announcement UI - Include strings for titles, empty state, types, and badges 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add announcements route under My page branch - Define route constant for type-safe navigation - Import announcement screen from feed feature 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Enable flutter generate flag in feed feature - Generate l10n files for my feature with new strings - Generate l10n files for feed feature with announcement strings 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
9b6e5e6 to
f13cf02
Compare
- Remove unnecessary null-aware operators on non-nullable l10n - Break long lines to fit 80-character limit - Add newlines at end of files
- Create proper Announcement model with Freezed - Use AnnouncementType enum instead of strings - Use DateTime for dates instead of strings - Update components to use typed model - Address review feedback for better maintainability
- Move model from feature/feed to core/model for proper architecture - Update imports to use core_model/feed.dart - Export announcement model through feed module index
Summary
Implementation Details
Test plan
Closes #279
🤖 Generated with Claude Code