feature/sm - #12
Conversation
- Introduced `flutter_bloc` and `get_it` packages in `pubspec.yaml` for state management and dependency injection. - Updated `main.dart` to set up the `ProductContainer` for service retrieval. - Refactored `FlightListPage` to utilize `BlocProvider` for managing flight data with `FlightListCubit`. - Enhanced `LoginPage` to implement `BlocProvider` and `BlocListener` for improved state management during user authentication. - Removed the outdated `LoginPageState` model and associated logic, streamlining the login process. - Updated UI components in the login page to utilize `ValueNotifier` for managing state without setState calls. - Improved README.md with additional documentation on the new architecture and state management approach.
- Updated the login page to utilize a mixin for managing controllers and lifecycle, improving separation of concerns. - Removed unnecessary ValueNotifiers from the mixin, allowing individual widgets to manage their own state with ValueListenableBuilder. - Implemented BlocSelector for optimized state listening in the login UI components, reducing unnecessary rebuilds. - Enhanced the documentation to reflect the new structure and usage patterns for the login feature.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request standardizes the project's architecture by introducing a pattern based on Cubit, Equatable states, and ValueListenableBuilders for local UI state. It includes comprehensive documentation, Mermaid diagrams, and a Claude skill to automate feature creation. Key refactors were applied to the login and flight list pages, and a global dependency injection container was implemented using GetIt. Feedback focuses on improving the robustness of the version comparison logic to avoid range errors, addressing potential lifecycle issues when managing Cubit instances within State objects, and moving business logic like logout from the view layer to services or cubits for better separation of concerns.
| bool _isVersionOlder(String current, String minimum) { | ||
| // Bu çok basit bir implementasyon - production'da semantic versioning kullanılmalı | ||
| List<int> currentParts = current.split('.').map(int.parse).toList(); | ||
| List<int> minimumParts = minimum.split('.').map(int.parse).toList(); | ||
| final currentParts = current.split('.').map(int.parse).toList(); | ||
| final minimumParts = minimum.split('.').map(int.parse).toList(); | ||
|
|
||
| for (int i = 0; i < 3; i++) { | ||
| for (var i = 0; i < 3; i++) { | ||
| if (currentParts[i] < minimumParts[i]) return true; | ||
| if (currentParts[i] > minimumParts[i]) return false; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The version comparison logic assumes that both version strings always contain at least three parts (major.minor.patch). If either string has fewer parts (e.g., "1.0"), accessing currentParts[i] or minimumParts[i] will result in a RangeError. It is safer to handle varying lengths to prevent potential crashes.
bool _isVersionOlder(String current, String minimum) {
final currentParts = current.split('.').map(int.parse).toList();
final minimumParts = minimum.split('.').map(int.parse).toList();
final maxLength = currentParts.length > minimumParts.length
? currentParts.length
: minimumParts.length;
for (var i = 0; i < maxLength; i++) {
final currentPart = i < currentParts.length ? currentParts[i] : 0;
final minimumPart = i < minimumParts.length ? minimumParts[i] : 0;
if (currentPart < minimumPart) return true;
if (currentPart > minimumPart) return false;
}
return false;
}| late final LoginCubit loginCubit = LoginCubit( | ||
| ProductContainer.instance.get<IAuthService>(), | ||
| ProductContainer.instance.get<IProductNetworkManager>(), | ||
| ); |
There was a problem hiding this comment.
Defining the LoginCubit as a field in the State while using BlocProvider(create: ...) can lead to lifecycle issues. BlocProvider's create constructor is intended to own the lifecycle and will automatically call close() on the cubit when the provider is disposed. If the State object outlives the BlocProvider (e.g., during certain widget tree rebuilds), the loginCubit field will point to a disposed instance.
To fix this, you should either create the cubit instance directly inside the create callback and access it via context.read<LoginCubit>(), or use BlocProvider.value and manually close the cubit in the State.dispose() method.
| Future<void> _logout() async { | ||
| final prefs = await SharedPreferences.getInstance(); | ||
|
|
||
| // Hard coded key'leri temizle - kötü praktik | ||
| await prefs.remove('user_token'); | ||
| await prefs.remove('user_email'); | ||
| await prefs.remove('user_name'); | ||
| await prefs.remove('user_id'); | ||
| await prefs.setBool('is_logged_in', false); | ||
|
|
||
| // Tüm cache'i temizle - agresif yaklaşım | ||
| // await prefs.clear(); // Bu da kötü pratik | ||
|
|
||
| if (!mounted) return; | ||
| Navigator.pushAndRemoveUntil( | ||
| context, | ||
| MaterialPageRoute(builder: (context) => LoginPage()), | ||
| (Route<dynamic> route) => false, | ||
| MaterialPageRoute<void>(builder: (_) => const LoginPage()), | ||
| (route) => false, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The logout logic, which involves clearing multiple keys from SharedPreferences and performing navigation, is currently implemented directly within the View layer. To improve maintainability, separation of concerns, and testability, this logic should be moved to a service (e.g., AuthService) or a central Cubit (e.g., ApplicationCubit).
1 participant
This pull request introduces comprehensive documentation and code updates to standardize and clarify the use of the Cubit + Equatable state + mixin + ValueListenable pattern for Flutter feature screens. The changes include new and improved guides, architectural diagrams, command documentation, and a concrete Cubit implementation. These updates aim to make it easier for developers to implement and maintain consistent, scalable feature screens using best practices for state management, dependency injection, and UI-local state.
Key changes:
1. Documentation and Guidance Enhancements
docs/prompt/flutter_cubit_feature_prompt.md) that standardizes the Cubit + mixin + ValueListenable pattern, including code templates, rules, and best practices for state, Cubit, mixin, and UI-local state..claude/skills/flutter-cubit-feature/SKILL.md) summarizing when and how to apply the pattern, with a checklist and explicit do's and don'ts./cubit-addto automate or scaffold the pattern on new or existing feature screens, referencing the main prompt and live code examples.2. Architectural Visualization
docs/prompt/flutter_cubit_architecture.mmd) visualizing the full architecture: ProductContainer DI, Cubit/State, BlocProvider, PageMixin, BlocSelector/BlocBuilder/BlocListener, and ValueListenableBuilder, with clear layer responsibilities and data flow.README.mdwith a box diagram illustrating the same architecture and the flow from DI to UI-local state.3. Reference Implementation
FlightListCubitimplementation (lib/feature/auth/flight/cubit/flight_list_cubit.dart) demonstrating the prescribed pattern: Equatable state with copyWith, async loading, error handling, and service injection.These updates provide both high-level and hands-on resources to ensure all feature screens follow a consistent, maintainable structure using modern Flutter state management patterns.