Conversation
WalkthroughAdds a Roadster feature: network model/service/data source, repository and mapping, BLoC (events/states), detailed UI (screen/widgets), DI bindings and route, localization keys and ARB updates, fixtures and comprehensive tests; also updates generated Freezed outputs and minor formatting. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as RoadsterScreen
participant Bloc as RoadsterBloc
participant Repo as RoadsterRepository
participant DS as RoadsterDataSource
participant Svc as RoadsterService
participant API as SpaceX API v4
User->>UI: navigate to Routes.roadster
UI->>Bloc: dispatch RoadsterEvent.load()
Bloc->>Bloc: emit Loading
Bloc->>Repo: getRoadster()
Repo->>DS: getRoadster()
DS->>Svc: fetchRoadster()
Svc->>API: GET /roadster
API-->>Svc: 200 JSON
Svc-->>DS: NetworkRoadsterModel
DS-->>Repo: ApiResult.success(model)
Repo->>Repo: map to RoadsterResource
Repo-->>Bloc: RoadsterResource
Bloc->>Bloc: emit Success(roadster)
Bloc-->>UI: state = Success
UI->>UI: render details
alt Error path
API-->>Svc: Error
Svc-->>DS: throw
DS-->>Repo: ApiResult.error(msg)
Repo-->>Bloc: throw Exception
Bloc->>Bloc: emit Error
Bloc-->>UI: state = Error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
+ Coverage 83.80% 84.90% +1.10%
==========================================
Files 118 150 +32
Lines 2451 3194 +743
==========================================
+ Hits 2054 2712 +658
- Misses 397 482 +85 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (71)
lib/data/network/service/constants.dart (1)
1-2: Deprecate v3 and align naming/usageIf v3 is legacy for this app, mark it deprecated and nudge call sites to v4. Also keep trailing-slash convention consistent across services to avoid accidental double slashes in retrofit/dio joins.
-const String baseUrl = 'https://api.spacexdata.com/v3/'; +@Deprecated('Use baseUrlVersion4') +const String baseUrl = 'https://api.spacexdata.com/v3/';lib/features/roadster/model/orbital_data.dart (2)
3-9: Make the model truly immutable and const-constructibleAnnotate with @immutable and make the constructor const to enable const literals in widgets and safer reuse.
-import 'package:flutter/material.dart'; +import 'package:flutter/foundation.dart' show immutable; +import 'package:flutter/material.dart'; -class OrbitalData { - OrbitalData({required this.label, required this.value, required this.icon}); +@immutable +class OrbitalData { + const OrbitalData({required this.label, required this.value, required this.icon});
3-9: Value semantics for UI modelsConsider Equatable or overriding ==/hashCode for reliable comparisons in tests and state diffs.
lib/features/roadster/model/mission.dart (2)
1-6: Const constructor and immutability annotationSame rationale as OrbitalData: annotate and make the constructor const.
-class Mission { - Mission({required this.name, required this.isPrimary}); +import 'package:flutter/foundation.dart' show immutable; + +@immutable +class Mission { + const Mission({required this.name, required this.isPrimary});
1-6: Equality for list rendering/testsAdd Equatable or ==/hashCode to avoid duplicate chips and ease testing.
lib/features/roadster/widget/animated_star_widget.dart (1)
15-19: Avoid hardcoded 400/500; position relative to available sizeOn small/large screens the starfield may cluster or overflow. Use MediaQuery for screen size or pass bounds from parent.
- final top = random.nextDouble() * 500; - final left = random.nextDouble() * 400; + final screenSize = MediaQuery.sizeOf(context); + final top = random.nextDouble() * screenSize.height; + final left = random.nextDouble() * screenSize.width;lib/features/roadster/widget/animated_counter_widget.dart (1)
52-58: Use locale-aware number formattingRegex-based grouping won’t localize and can misplace separators. Prefer intl NumberFormat.
- : _animation.value.toInt().toString().replaceAllMapped( - RegExp(r'(\d{1,3})(?=(\d{3})+(?!\d))'), - (Match m) => '${m[1]},', - ), + : NumberFormat.decimalPattern().format(_animation.value.floor()),Add:
+import 'package:intl/intl.dart';lib/l10n/intl_de.arb (1)
243-248: Improve German term and add placeholders metadata
- “Orbitale Periode” is uncommon; “Umlaufzeit” is idiomatic.
- Add @launched metadata for typed placeholder support.
- "orbitalPeriod": "Orbitale Periode", + "orbitalPeriod": "Umlaufzeit", @@ - "launched": "Gestartet: {date}", + "launched": "Gestartet: {date}", + "@launched": { + "description": "Formatierter Starttermin", + "placeholders": { "date": {} } + },lib/models/roadster/roadster_resource.dart (1)
58-59: Use deep equality for lists and keep immutability semantics.Equatable compares lists by identity; this may break state equality when only
flickrImagescontents differ. Wrap withEquatableListViewinprops.@override List<Object?> get props => [ @@ - flickrImages, + flickrImages == null ? null : EquatableListView(flickrImages!), wikipedia, video, details, id, ]; + + @override + bool get stringify => kDebugMode;Also applies to: 64-94
lib/l10n/intl_uk.arb (1)
374-374: Adjust unit label for consistent usage across values.Prefer compact, number-agnostic form.
- "millionKm": "мільйон км", + "millionKm": "млн км",lib/l10n/app_localizations_pt.dart (2)
378-379: Portuguese grammar: add preposition in unit label.- String get millionKm => 'milhões km'; + String get millionKm => 'milhões de km';
392-394: Casing consistency in PT labels.Align with sentence case used elsewhere (e.g., “Detalhes da missão”).
- String get currentSpeed => 'Velocidade Atual'; + String get currentSpeed => 'Velocidade atual';lib/features/settings/settings_screen.dart (1)
118-126: Nice addition; consider matching icon style with others.Other settings use outlined icons; switch to outlined variant for consistency.
- icon: Icons.rocket_launch, + icon: Icons.rocket_launch_outlined,test/models/roadster/roadster_ext_test.dart (1)
7-22: Consider adding null and additional-field cases.Add cases for nulls and a couple of extra fields from NetworkRoadsterModel to guard future regressions in the extension mapping.
test/features/roadster/widget/launch_section_widget_test.dart (1)
13-24: Use pumpAndSettle to ensure l10n/frames are ready.Prevents flakiness on slower CI when loading delegates.
Apply:
await tester.pumpWidget( const MaterialApp( localizationsDelegates: appLocalizationsDelegates, supportedLocales: appSupportedLocales, home: Scaffold( body: LaunchSectionWidget( massKg: testMass, vehicle: testVehicle, ), ), ), ); + await tester.pumpAndSettle();lib/di/di_repository_module.dart (1)
14-15: Method name nit: “Accidents” vs Theme.Minor: rename for clarity.
- ThemeRepository provideAccidentsRepository(ThemeStorage themeStorage) => + ThemeRepository provideThemeRepository(ThemeStorage themeStorage) => ThemeRepositoryImpl(themeStorage);test/features/roadster/widget/animated_stat_card_widget_test.dart (1)
31-33: More robust finder for generic widget.byType with generics can be brittle; prefer a predicate.
-expect(find.byType(TweenAnimationBuilder<double>), findsOneWidget); +expect( + find.byWidgetPredicate((w) => w is TweenAnimationBuilder<double>), + findsOneWidget, +);test/features/roadster/widget/details_card_widget_test.dart (2)
39-43: Avoid redundant Chip count assertion inside loop.Assert count once outside the loop for clarity/perf.
- for (final mission in missions) { - expect(find.text(mission.name), findsOneWidget); - expect(find.byType(Chip), findsNWidgets(missions.length)); - } + for (final mission in missions) { + expect(find.text(mission.name), findsOneWidget); + } + expect(find.byType(Chip), findsNWidgets(missions.length));
45-47: Settle animations if assertions later depend on final state.Optional: settle frames for stability.
- await tester.pump(const Duration(seconds: 1)); + await tester.pumpAndSettle(const Duration(seconds: 1));test/features/roadster/bloc/roadster_bloc_test.dart (2)
27-41: Build a fresh Bloc instance inside blocTest for isolation.Returning the pre-created
blocis fine but less isolated. Prefer constructing a new bloc per test inbuild.Apply this diff:
@@ - build: () { - when(repository.getRoadster()).thenAnswer( - (_) async => mockRoadsterResource, - ); - return bloc; - }, + build: () { + when(repository.getRoadster()).thenAnswer( + (_) async => mockRoadsterResource, + ); + return RoadsterBloc(repository); + }, @@ - build: () { - when(repository.getRoadster()).thenThrow( - Exception('something went wrong'), - ); - return bloc; - }, + build: () { + when(repository.getRoadster()).thenThrow( + Exception('something went wrong'), + ); + return RoadsterBloc(repository); + },Also applies to: 43-57
7-9: Eliminate imports of tests from other test files
Multiple tests (e.g. rockets_screen_test.dart, roadster_bloc_test.dart, email_list_screen_test.dart) import other *_test.dart files. Extract any shared mocks or fixtures into a common helper (e.g. under test/fixtures/) and update imports to reference that instead.lib/models/roadster/roadster_ext.dart (1)
29-34: Defensive copy for list field to avoid downstream mutation.If
flickrImagesfrom the network layer is mutable, propagate an unmodifiable view to the domain model.Apply this diff:
- flickrImages: flickrImages, + flickrImages: List.unmodifiable(flickrImages),test/features/roadster/roadster_screen_test.dart (1)
41-51: Avoid brittle hard-coded UI text; assert via localization or keys.Hard-coding 'Try Again' will break on copy changes/locales. Prefer
find.byKey(const ValueKey('roadster_retry'))and add that key in the widget, or resolve the localized string in-test.Example (test side):
- expect(find.text('Try Again'), findsOneWidget); + expect(find.byKey(const ValueKey('roadster_retry')), findsOneWidget);And in the widget (outside this file), attach the key to the retry button/text.
test/repository/roadster_repository_impl_test.dart (3)
22-35: Also verify data source interactions.Assert the data source is called exactly once and nothing else.
Apply this diff:
final result = await repository.getRoadster(); expect(result, isA<RoadsterResource>()); expect(result.name, equals("Elon Musk's Tesla Roadster")); + verify(() => mockDataSource.getRoadster()).called(1); + verifyNoMoreInteractions(mockDataSource);
36-45: Tighten verification on error path.Add interaction verification for consistency.
Apply this diff:
expect( () => repository.getRoadster(), throwsA(isA<Exception>()), ); + verify(() => mockDataSource.getRoadster()).called(1); + verifyNoMoreInteractions(mockDataSource);
47-56: Ditto for loading path.Apply this diff:
expect( () => repository.getRoadster(), throwsA(isA<Exception>()), ); + verify(() => mockDataSource.getRoadster()).called(1); + verifyNoMoreInteractions(mockDataSource);lib/data/network/service/roadster/roadster_service.dart (1)
12-13: Nit: use a leading slash in the path for clarity.Prevents accidental double-join quirks if baseUrl changes.
Apply this diff:
- @GET('roadster') + @GET('/roadster')lib/features/roadster/bloc/roadster_bloc.dart (1)
12-20: Consider a restartable event transformer and richer error capture.Avoid stacking multiple fetches and preserve error context for observability.
Apply this diff:
+import 'package:bloc_concurrency/bloc_concurrency.dart' show restartable; @@ - on<RoadsterLoadEvent>((event, emit) async { + on<RoadsterLoadEvent>((event, emit) async { emit(const RoadsterState.loading()); try { final roadster = await _repository.getRoadster(); emit(RoadsterState.success(roadster: roadster)); - } catch (e) { - emit(const RoadsterState.error()); + } catch (e, st) { + // TODO: add logging if available, e.g., log(e, st) + emit(const RoadsterState.error()); } - }); + }, transformer: restartable());test/repository/roadster_repository_test.dart (2)
25-32: Prefer Exception over Error in async contract tests.Production code typically throws
Exception. UsingErrorhere can mislead and bypass catch blocks.Apply this diff:
- test('returns error', () { + test('returns exception', () { when(repository.getRoadster()).thenAnswer((_) => Future.error(Error())); expect( repository.getRoadster(), - throwsA(isA<Error>()), + throwsA(isA<Exception>()), ); });
37-72: Extract the sharedmockRoadsterResourceinto a fixture module.This constant is reused elsewhere; move it to
test/fixtures/roadster_resource.dart(see earlier comment) to avoid cross-importing test files.test/models/roadster/roadster_resource_test.dart (1)
41-42: UsehasLengthmatcher for clarity.Slightly clearer and null-safe.
- expect(resource.flickrImages?.length, 2); + expect(resource.flickrImages, hasLength(2));lib/repository/roadster_repository.dart (1)
19-25: Avoid throwing generic exceptions; treat unexpectedloadingdeterministically.Map errors to a typed repo exception and make the
loadingbranch an explicit unexpected state.return ApiResultWhen(result).when( success: (data) => data.toResource(), - error: (message) => throw Exception(message), - loading: () { - throw Exception('Loading'); - }, + error: (message) => throw RoadsterRepositoryException(message), + loading: () => throw StateError('Unexpected loading state from data source'), );Add this type to the file (outside the class):
class RoadsterRepositoryException implements Exception { final String message; const RoadsterRepositoryException(this.message); @override String toString() => 'RoadsterRepositoryException: $message'; }test/data/network/service/roadster/roadter_service_test.dart (2)
16-20: Drop unnecessary async and fallback registration insetUp.
fetchRoadster()takes no args;registerFallbackValue(Uri())is unused.setUpneedn’t beasync.-setUp(() async { - registerFallbackValue(Uri()); +setUp(() { service = MockRoadsterService(); dataSource = RoadsterNetworkDataSource(service); });
1-1: Typo in filename:roadter→roadster.Rename the test file for consistency and easier discovery.
lib/features/roadster/bloc/roadster_state.dart (2)
11-11: Carry an optional error message in state.Enables UI to display the failure reason without external state.
- const factory RoadsterState.error() = RoadsterErrorState; + const factory RoadsterState.error([String? message]) = RoadsterErrorState;
3-4: Style consistency: prefer@freezed(lowercase) like elsewhere.No functional change; improves consistency with common Freezed usage.
test/features/roadster/utils/roadster_utils_test.dart (2)
1-4: Lock locale to avoid date-formatting flakiness on CI.
DateFormatoutput varies by locale; pin toen_USfor stable expectations.import 'package:flutter/material.dart'; import 'package:flutter_bloc_app_template/features/roadster/utils/roadster_utils.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:intl/intl.dart';
5-6: Initialize default locale in tests.void main() { + setUpAll(() => Intl.defaultLocale = 'en_US'); group('NumberFormatter', () {lib/features/roadster/widget/launch_section_widget.dart (2)
67-71: Non-nullable color access and consistency.Use
.shade400over index for non-null color access.- style: TextStyle( - color: Colors.orange[400], + style: TextStyle( + color: Colors.orange.shade400, fontSize: 18, fontWeight: FontWeight.bold, ),(repeat the same change for the
vehicletext style)Also applies to: 86-90
18-99: Theming and accessibility (optional).Consider deriving colors from
Theme.of(context).colorSchemeand addingSemantics/excludeFromSemanticsas appropriate for better contrast and screen-reader hints.test/features/roadster/widget/animated_counter_widget_test.dart (1)
72-73: Scope theTextlookup to this widget to avoid false positives.If additional
Textwidgets are introduced, this remains stable.- final textWidget = tester.widget<Text>(find.byType(Text)); - final value = int.tryParse(textWidget.data!.replaceAll(',', '')); + final textFinder = find.descendant( + of: find.byType(AnimatedCounterWidget), + matching: find.byType(Text), + ); + final textWidget = tester.widget<Text>(textFinder); + final value = int.tryParse(textWidget.data!.replaceAll(',', ''));lib/features/roadster/widget/links_section_widget.dart (1)
15-19: Use the existing “Links & Resources” key for consistencyPrefer l10n.linksResources over l10n.learnMore to match localization surface elsewhere.
- l10n.learnMore, + l10n.linksResources,lib/features/roadster/widget/orbital_section_widget.dart (1)
43-101: Avoid rebuilding heavy child each tickPass the tile as TweenAnimationBuilder.child and only animate the transform.
- return TweenAnimationBuilder<double>( - tween: Tween(begin: 0, end: 1), - duration: Duration(milliseconds: 800 + (index * 100)), - curve: Curves.easeOutBack, - builder: (context, animation, child) { - return Transform.scale( - scale: animation, - child: Container( + return TweenAnimationBuilder<double>( + tween: Tween(begin: 0, end: 1), + duration: Duration(milliseconds: 800 + (index * 100)), + curve: Curves.easeOutBack, + child: Container( padding: const EdgeInsets.all(12), decoration: BoxDecoration( color: Theme.of(context) .colorScheme .surface .withValues(alpha: 0.5), borderRadius: BorderRadius.circular(12), border: Border.all( color: Theme.of(context) .colorScheme .primary .withValues(alpha: 0.3), ), ), child: Row( children: [ Icon( data.icon, color: Theme.of(context).colorScheme.secondary, size: 20, ), const SizedBox(width: 8), Expanded( child: Column( crossAxisAlignment: CrossAxisAlignment.start, mainAxisAlignment: MainAxisAlignment.center, children: [ Text( data.label, - style: const TextStyle( - color: Colors.white60, - fontSize: 11, - ), + style: TextStyle( + color: Theme.of(context) + .colorScheme + .onSurface + .withOpacity(0.6), + fontSize: 11, + ), ), Text( data.value, style: TextStyle( color: Theme.of(context).colorScheme.primary, fontSize: 14, fontWeight: FontWeight.bold, ), ), ], ), ), ], ), - ), - ); - }, + ), + builder: (context, animation, child) => + Transform.scale(scale: animation, child: child), );lib/features/roadster/widget/details_card_widget.dart (1)
68-79: Avoid rebuilding the icon every frameUse AnimatedBuilder.child to keep the Icon constant.
- AnimatedBuilder( - animation: _rotationController, - builder: (context, child) { - return Transform.rotate( - angle: _rotationController.value * 2 * math.pi, - child: Icon( - Icons.satellite_alt, - color: Theme.of(context).colorScheme.primary, - size: 32, - ), - ); - }, - ), + AnimatedBuilder( + animation: _rotationController, + child: Icon( + Icons.satellite_alt, + color: Theme.of(context).colorScheme.primary, + size: 32, + ), + builder: (context, child) => Transform.rotate( + angle: _rotationController.value * 2 * math.pi, + child: child, + ), + ),test/data/network/model/roadster/network_roadster_model_test.dart (2)
69-70: Use matcher semantics for inequalityCleaner test style.
- expect(model != copy, true); // Original and copy are different + expect(copy, isNot(equals(model))); // Original and copy are different
88-96: Avoid shadowing outerjsonvariableRename the inner JSON map for clarity.
- test('can be deserialized from JSON', () { - final json = { + test('can be deserialized from JSON', () { + final minimalJson = { 'name': 'Roadster', 'launch_date_utc': '2018-02-06T20:45:00.000Z', 'launch_mass_kg': 1350, 'flickr_images': ['url1', 'url2'], }; - final model = NetworkRoadsterModel.fromJson(json); + final model = NetworkRoadsterModel.fromJson(minimalJson);Also applies to: 96-102
lib/l10n/intl_en.arb (1)
383-385: Add metadata for placeholderdateHelps gen_l10n produce clearer API/docs.
"unitKph": "km/h", "launched": "Launched: {date}", + "@launched": { + "description": "Label for launched date", + "placeholders": { "date": {} } + }, "roadsterTitle": "Roadster",lib/features/roadster/widget/distance_card_widget.dart (1)
81-84: Optional: guard negative distancesClamp to 0 to avoid odd UI if upstream sends negatives.
- AnimatedCounterWidget( - value: distance / 1000000, + AnimatedCounterWidget( + value: + (distance / 1000000).clamp(0, double.infinity), duration: Duration(milliseconds: 2000 + delay), decimals: 1, ),lib/features/roadster/utils/roadster_utils.dart (4)
4-4: Fix typo in extension name for consistency
RenameNumberFormatterEXt→NumberFormatterExt.-extension NumberFormatterEXt on double { +extension NumberFormatterExt on double {
5-13: Locale flexibility for number formatting
Allow caller-supplied locale; keep current default. Optionally consider caching if this is hot.- String formatSpeed() { - final formatter = NumberFormat('#,##0.00', 'en_US'); + String formatSpeed({String locale = 'en_US'}) { + final formatter = NumberFormat('#,##0.00', locale); return formatter.format(this); } - String formatPeriod() { - final formatter = NumberFormat('0.0', 'en_US'); + String formatPeriod({String locale = 'en_US'}) { + final formatter = NumberFormat('0.0', locale); return formatter.format(this); }
17-25: Avoid exceptions during parsing
UseDateTime.tryParseto skip exceptions and keep the nice fallback.String toFormattedDate({String locale = 'en_US'}) { - try { - final dateTime = DateTime.parse(this).toLocal(); - final formatter = DateFormat.yMMMd(locale); - return formatter.format(dateTime); - } catch (e) { - return this; // fallback if parsing fails - } + final dt = DateTime.tryParse(this); + if (dt == null) return this; + final formatter = DateFormat.yMMMd(locale); + return formatter.format(dt.toLocal()); }
45-59: Theme awareness for default styles (optional)
HardcodedColors.white*can fight themes. Consider accepting colors or aBuildContextto derive fromTheme.of(context).textTheme.lib/features/roadster/widget/animated_stat_card_widget.dart (2)
5-13: Validate inputs early
Assert non-negative delay to catch misuse in debug.const AnimatedStatCardWidget({ super.key, required this.icon, required this.title, required this.value, required this.unit, required this.color, required this.delay, - }); + }) : assert(delay >= 0);
44-46: Use withOpacity for Flutter SDK < 3.27.0
Your pubspec.yaml declares Flutter>=3.19.2, which predates the introduction ofColor.withValuesin Flutter 3.27.0 (stackoverflow.com). Replace at lines 44–46:- color.withValues(alpha: 0.2), - color.withValues(alpha: 0.1), + color.withOpacity(0.2), + color.withOpacity(0.1),Once you raise your minimum SDK to
>=3.27.0, you can switch back towithValues().lib/features/roadster/roadster_screen.dart (5)
186-196: Add errorBuilder for image failures
Show a graceful fallback when a network image fails.child: Image.network( imageUrl, fit: BoxFit.cover, loadingBuilder: (context, child, loadingProgress) { if (loadingProgress == null) return child; return Center( child: CircularProgressIndicator( value: _calculateProgress(loadingProgress), ), ); }, + errorBuilder: (context, error, stack) => const Center( + child: Icon(Icons.broken_image, color: Colors.white30, size: 48), + ), ),
371-377: SDK compatibility: Color.withValues
Same note as in the stat card; considerwithOpacityfor broader support.- Colors.black.withValues(alpha: 0.7), + Colors.black.withOpacity(0.7),
409-417: Avoid rendering “null” in mission chip labels
Add safe fallbacks when values are null.- Mission( - name: '${widget.roadster.noradId}', + Mission( + name: widget.roadster.noradId?.toString() ?? '-', isPrimary: true, ), - Mission( - name: '${widget.roadster.orbitType}', + Mission( + name: widget.roadster.orbitType ?? '-', isPrimary: false, ),
255-261: Localize the toolbar title (optional)
Hardcoded 'Tesla Roadster'. PreferS.of(context).roadsterTitleor the resource name.
69-69: Const constructor usage (nit)
EmptyWidget()can beconst EmptyWidget()to avoid rebuild work.- return EmptyWidget(); + return const EmptyWidget();lib/l10n/app_localizations_en.dart (2)
405-407: Format the date in a locale-aware way in launched()Use intl to format DateTime; avoid relying on Object.toString().
- String launched(Object date) { - return 'Launched: $date'; - } + String launched(Object date) { + final value = date is DateTime + ? intl.DateFormat.yMMMd(locale).format(date) + : '$date'; + return 'Launched: $value'; + }
361-414: Confirm need for both launchedAt() and launched()These duplicate concepts and may drift across locales. If only one is needed, remove the other via ARB and regenerate; otherwise, ensure consistent copy/formatting between them.
lib/l10n/app_localizations_uk.dart (2)
407-409: Локалізоване форматування дати в launched()Використайте intl для форматування DateTime.
- String launched(Object date) { - return 'Запуск: $date'; - } + String launched(Object date) { + final value = date is DateTime + ? intl.DateFormat.yMMMd(locale).format(date) + : '$date'; + return 'Запуск: $value'; + }
361-416: Перевірте, чи потрібні одночасно launchedAt() і launched()Дублювання може призвести до розбіжностей у копі/форматуванні. За можливості уніфікуйте через ARB і перегенеруйте.
lib/l10n/app_localizations_de.dart (3)
372-396: Use more idiomatic German termsSmall copy tweaks improve clarity.
- String get launchVehicle => 'Startfahrzeug'; + String get launchVehicle => 'Trägerrakete'; - String get orbitalParameters => 'Orbitale Parameter'; + String get orbitalParameters => 'Orbitparameter'; - String get orbitalPeriod => 'Orbitale Periode'; + String get orbitalPeriod => 'Umlaufzeit';
405-407: Format date in launched()Prefer intl formatting for DateTime.
- String launched(Object date) { - return 'Gestartet: $date'; - } + String launched(Object date) { + final value = date is DateTime + ? intl.DateFormat.yMMMd(locale).format(date) + : '$date'; + return 'Gestartet: $value'; + }
361-414: Verify duplication between launchedAt() and launched()Keep one key if possible; otherwise ensure consistent content and formatting across both.
lib/data/network/model/roadster/network_roadster_model.dart (1)
6-42: Optional: reduce JsonKey noise with fieldRenameYou could annotate the class with @JsonSerializable(fieldRename: FieldRename.snake) and drop most @jsonkey(name: ...) entries. Not required, just tidier.
lib/data/network/model/roadster/network_roadster_model.freezed.dart (2)
779-788: Defensive copy for List to avoid external mutation (optional).Getter wraps in EqualUnmodifiableListView, but the backing list can still be mutated by a holder of the original reference. If you want stronger immutability, introduce a named factory in the source (network_roadster_model.dart) that copies:
factory NetworkRoadsterModel.safe({ // ...same params List<String>? flickrImages, // ... }) => NetworkRoadsterModel( // ... flickrImages: flickrImages == null ? null : List.unmodifiable(flickrImages), );Then use the safe factory at construction sites.
18-21: Consider DateTime for launch_date_utc (optional).If callers need temporal ops, model launchDateUtc as DateTime with custom (de)serializers to reduce parsing at call sites.
test/mocks.mocks.dart (1)
8-8: Avoid importing Flutter internals in generated mocks.
package:flutter/src/...is an implementation import. Prefer referencing NavigatorObserver frompackage:flutter/widgets.dartin test/mocks.dart and regenerate:// test/mocks.dart import 'package:flutter/widgets.dart' show NavigatorObserver; // @GenerateMocks([NavigatorObserver, ...])This usually steers Mockito to public imports.
lib/data/network/model/roadster/network_roadster_model.g.dart (1)
43-73: Optionally omit nulls from JSON.If you want leaner payloads, add
@JsonSerializable(includeIfNull: false)on the model (in the source file) and regenerate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
lib/generated/intl/messages_de.dartis excluded by!**/generated/**lib/generated/intl/messages_en.dartis excluded by!**/generated/**lib/generated/intl/messages_pt.dartis excluded by!**/generated/**lib/generated/intl/messages_uk.dartis excluded by!**/generated/**lib/generated/l10n.dartis excluded by!**/generated/**
📒 Files selected for processing (61)
lib/data/network/data_source/roadster_network_data_source.dart(1 hunks)lib/data/network/model/core/network_core_model.freezed.dart(0 hunks)lib/data/network/model/roadster/network_roadster_model.dart(1 hunks)lib/data/network/model/roadster/network_roadster_model.freezed.dart(1 hunks)lib/data/network/model/roadster/network_roadster_model.g.dart(1 hunks)lib/data/network/model/stage/network_first_stage_model.freezed.dart(0 hunks)lib/data/network/service/constants.dart(1 hunks)lib/data/network/service/roadster/roadster_service.dart(1 hunks)lib/data/network/service/roadster/roadster_service.g.dart(1 hunks)lib/di/app_bloc_providers.dart(2 hunks)lib/di/app_repository_providers.dart(2 hunks)lib/di/di_initializer.config.dart(3 hunks)lib/di/di_network_module.dart(3 hunks)lib/di/di_repository_module.dart(2 hunks)lib/features/roadster/bloc/roadster_bloc.dart(1 hunks)lib/features/roadster/bloc/roadster_bloc.freezed.dart(1 hunks)lib/features/roadster/bloc/roadster_event.dart(1 hunks)lib/features/roadster/bloc/roadster_state.dart(1 hunks)lib/features/roadster/model/mission.dart(1 hunks)lib/features/roadster/model/orbital_data.dart(1 hunks)lib/features/roadster/roadster_screen.dart(1 hunks)lib/features/roadster/utils/roadster_utils.dart(1 hunks)lib/features/roadster/widget/animated_counter_widget.dart(1 hunks)lib/features/roadster/widget/animated_star_widget.dart(1 hunks)lib/features/roadster/widget/animated_stat_card_widget.dart(1 hunks)lib/features/roadster/widget/details_card_widget.dart(1 hunks)lib/features/roadster/widget/distance_card_widget.dart(1 hunks)lib/features/roadster/widget/launch_section_widget.dart(1 hunks)lib/features/roadster/widget/links_section_widget.dart(1 hunks)lib/features/roadster/widget/orbital_section_widget.dart(1 hunks)lib/features/settings/settings_screen.dart(1 hunks)lib/l10n/app_localizations.dart(1 hunks)lib/l10n/app_localizations_de.dart(1 hunks)lib/l10n/app_localizations_en.dart(1 hunks)lib/l10n/app_localizations_pt.dart(1 hunks)lib/l10n/app_localizations_uk.dart(1 hunks)lib/l10n/intl_de.arb(1 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_pt.arb(1 hunks)lib/l10n/intl_uk.arb(2 hunks)lib/models/roadster/roadster_ext.dart(1 hunks)lib/models/roadster/roadster_resource.dart(1 hunks)lib/repository/roadster_repository.dart(1 hunks)lib/routes/router.dart(3 hunks)test/data/network/fixtures/roadster/roadster.json(1 hunks)test/data/network/model/roadster/network_roadster_model_test.dart(1 hunks)test/data/network/service/roadster/roadter_service_test.dart(1 hunks)test/features/roadster/bloc/roadster_bloc_test.dart(1 hunks)test/features/roadster/roadster_screen_test.dart(1 hunks)test/features/roadster/utils/roadster_utils_test.dart(1 hunks)test/features/roadster/widget/animated_counter_widget_test.dart(1 hunks)test/features/roadster/widget/animated_star_widget_test.dart(1 hunks)test/features/roadster/widget/animated_stat_card_widget_test.dart(1 hunks)test/features/roadster/widget/details_card_widget_test.dart(1 hunks)test/features/roadster/widget/launch_section_widget_test.dart(1 hunks)test/mocks.dart(2 hunks)test/mocks.mocks.dart(11 hunks)test/models/roadster/roadster_ext_test.dart(1 hunks)test/models/roadster/roadster_resource_test.dart(1 hunks)test/repository/roadster_repository_impl_test.dart(1 hunks)test/repository/roadster_repository_test.dart(1 hunks)
💤 Files with no reviewable changes (2)
- lib/data/network/model/stage/network_first_stage_model.freezed.dart
- lib/data/network/model/core/network_core_model.freezed.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-11T19:29:38.429Z
Learnt from: ashtanko
PR: ashtanko/flutter_bloc_app_template#30
File: lib/di/app_bloc_providers.dart:21-26
Timestamp: 2025-08-11T19:29:38.429Z
Learning: In the flutter_bloc_app_template project, immediate event dispatch using cascade notation (..add()) in BlocProvider creation is the preferred pattern. The blocs should dispatch initial events immediately upon creation rather than deferring them to widget initialization or post-frame callbacks.
Applied to files:
lib/di/app_bloc_providers.dartlib/di/app_repository_providers.dartlib/features/roadster/bloc/roadster_bloc.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Flutter Integration Tests (31)
- GitHub Check: App
🔇 Additional comments (37)
lib/data/network/service/constants.dart (1)
2-2: Good addition: v4 base URL constantClear separation between v3 and v4 endpoints. Looks good.
lib/l10n/app_localizations.dart (1)
771-872: All locale implementations include the new getters andlaunched(date)method. Verified inapp_localizations_en.dart,de.dart,uk.dart, andpt.dart.lib/di/app_bloc_providers.dart (1)
46-52: LGTM: Immediate load event on RoadsterBloc matches project pattern.lib/di/app_repository_providers.dart (2)
26-28: LGTM: Repository provider wiring for Roadster added.
26-28: No action needed: RoadsterRepository is already registered in DI.The
provideRoadsterRepositoryfactory in lib/di/di_repository_module.dart and its correspondinggh.factory<_i128.RoadsterRepository>entry in lib/di/di_initializer.config.dart confirm the DI binding for RoadsterRepository, so runtime resolution will succeed.lib/routes/router.dart (1)
7-7: LGTM: Roadster route, mapping, and animation hook are correctly wired.Also applies to: 17-18, 29-30, 37-38
test/models/roadster/roadster_ext_test.dart (1)
7-22: Solid mapping test coverage for primary fields.Good field-by-field assertions; the list equality is handled correctly by the matcher.
lib/di/di_network_module.dart (3)
39-42: RoadsterService provider wiring looks correct.Matches the existing pattern for Launch/Rocket services.
54-57: RoadsterDataSource provider wiring looks correct.Consistent with other data source registrations.
12-27: Regenerate DI after adding providers
Rundart run build_runner build --delete-conflicting-outputs(orflutter pub run build_runner build --delete-conflicting-outputsin a Flutter project) to rebuilddi_initializer.config.dartso the bindings inNetworkModule(and on lines 39-42, 54-57) are registered.test/mocks.dart (1)
3-16: Regenerate Mockito mocks locally
Please runflutter pub run build_runner build --delete-conflicting-outputsin your local environment to generateMockRoadsterRepository.test/features/roadster/widget/launch_section_widget_test.dart (1)
29-41: Assertions read well and target the key UI elements.Icon/header/labels/values checks are clear and sufficient.
lib/di/di_repository_module.dart (2)
25-27: RoadsterRepository binding looks correct.Constructor injection aligns with other repositories.
1-28: Resolve: RoadsterRepository registration confirmed
The generatedlib/di/di_initializer.config.dartincludes a factory forRoadsterRepository, so the DI config is up to date.test/features/roadster/widget/animated_stat_card_widget_test.dart (1)
37-55: Good coverage of visuals and parsed value.Animation pump and property check look fine.
lib/features/roadster/bloc/roadster_event.dart (2)
3-6: Event union LGTM.Single load() event keeps the API minimal and clear.
1-6: Regenerate Freezed code
Environment tooling isn’t available here to verify codegen. Manually runflutter pub run build_runner build --delete-conflicting-outputsand confirm no missing‐part errors.test/features/roadster/widget/details_card_widget_test.dart (1)
29-37: Overall widget assertions are solid.Covers card, icon, and description texts effectively.
test/features/roadster/bloc/roadster_bloc_test.dart (1)
22-25: LGTM on initial-state assertion.Asserting
RoadsterLoadingStateat creation matches the Bloc's constructor.lib/models/roadster/roadster_ext.dart (1)
5-35: LGTM: straightforward 1:1 mapping from network to resource.Keeps layers thin and explicit; covered by tests.
test/data/network/fixtures/roadster/roadster.json (1)
1-34: LGTM: fixture aligns with expected wire keys.Keys match v4 API and generated model names; suitable for deserialization tests.
test/features/roadster/roadster_screen_test.dart (1)
16-22: LGTM on bloc wiring in tests.Stubbing
streamand closing intearDownavoids leaks.lib/data/network/service/roadster/roadster_service.dart (1)
8-14: LGTM: Retrofit interface is minimal and correct.Factory + v4 base URL + typed return is standard.
lib/features/roadster/bloc/roadster_bloc.dart (2)
10-24: LGTM: clear, idiomatic Bloc with Freezed states.Initial loading + fetch on event is straightforward.
10-24: Verify immediate event dispatch at DI site.Per prior project preference, Bloc providers should dispatch
RoadsterLoadEventon creation via cascade (..add(const RoadsterLoadEvent())). Ensure DI wiring follows this.If you want, I can scan the repo wiring with a script to confirm the cascade pattern is present where
RoadsterBlocis provided.test/repository/roadster_repository_test.dart (1)
16-24: LGTM on success path test.Verifies completion with expected value using Mockito stubbing.
lib/data/network/service/roadster/roadster_service.g.dart (1)
1-75: Generated code — looks good.No manual changes recommended; matches Retrofit/Dio patterns and handles parse errors via
errorLogger.lib/features/roadster/widget/launch_section_widget.dart (1)
39-42: Icon choice compatibility.
Icons.rocketmay be unavailable on older Material icon sets. PreferIcons.rocket_launch.- Icon( - Icons.rocket, - color: Colors.orange[400], - ), + Icon( + Icons.rocket_launch, + color: Colors.orange.shade400, + ),lib/di/di_initializer.config.dart (1)
16-17: Roadster DI wiring looks correctService → DataSource → Repository are registered in order; matches module providers. No action needed.
Also applies to: 22-23, 68-70, 72-74, 80-81
lib/features/roadster/widget/animated_stat_card_widget.dart (1)
24-31: LGTM: animation and layout structure
TweenAnimationBuilder + scale-in, gradient card, and baseline-aligned counter/unit are clean.Also applies to: 31-49, 62-79
lib/features/roadster/roadster_screen.dart (1)
22-35: LGTM: BLoC wiring and state handling
Provider + immediateRoadsterLoadEvent, and clear loading/success/error rendering look good.Also applies to: 38-71
lib/data/network/model/roadster/network_roadster_model.dart (1)
8-36: LGTM: field coverage matches SpaceX Roadster API; Freezed/JSON wiring is correctlib/features/roadster/bloc/roadster_bloc.freezed.dart (2)
451-480: LGTM: success state carries RoadsterResource with copyWith; pattern methods generated correctly
12-14: pubspec.yaml SDK constraint supports Dart 3.x pattern matching
Minimum Dart SDK is set to>=3.2.0, which fully supports the pattern‐matching syntax used in the generated code—no changes required.lib/data/network/model/roadster/network_roadster_model.freezed.dart (1)
1-11: LGTM on generated Freezed scaffold.Looks consistent with the source definition; no functional issues spotted.
test/mocks.mocks.dart (1)
183-207: LGTM: Roadster mock added and typed correctly.Signatures align with RoadsterRepository; good for bloc/repo tests.
lib/data/network/model/roadster/network_roadster_model.g.dart (1)
9-41: LGTM on JSON parsing.Types and conversions (num→int/double, list mapping) look correct.
| import 'package:flutter_bloc_app_template/data/network/api_result.dart'; | ||
| import 'package:flutter_bloc_app_template/data/network/model/roadster/network_roadster_model.dart'; | ||
| import 'package:flutter_bloc_app_template/data/network/service/roadster/roadster_service.dart'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten error handling and remove unnecessary Future.value
Catch DioException explicitly and return error without wrapping in Future.value.
+import 'package:dio/dio.dart';
import 'package:flutter_bloc_app_template/data/network/api_result.dart';
import 'package:flutter_bloc_app_template/data/network/model/roadster/network_roadster_model.dart';
import 'package:flutter_bloc_app_template/data/network/service/roadster/roadster_service.dart';
@override
Future<ApiResult<NetworkRoadsterModel>> getRoadster() async {
try {
final result = await _service.fetchRoadster();
return ApiResult.success(result);
- } catch (e) {
- return Future.value(ApiResult.error(e.toString()));
+ } on DioException catch (e) {
+ return ApiResult.error(e.message ?? e.toString());
+ } catch (e) {
+ return ApiResult.error(e.toString());
}
}Also applies to: 14-21
🤖 Prompt for AI Agents
In lib/data/network/data_source/roadster_network_data_source.dart around lines
1-3 (and similarly lines 14-21), the current error handling wraps errors in
Future.value and doesn't explicitly catch DioException; change the try/catch so
you catch DioException on its own (e.g. on DioException catch (e)) and return
the ApiResult error directly (no Future.value wrapper), and add a generic catch
(e, s) to return ApiResult.error for other exceptions; ensure the method still
returns the correct ApiResult type without unnecessary Future.value calls.
| color: Colors.white.withValues( | ||
| alpha: 0.3 + (pulseController.value * 0.5), | ||
| ), | ||
| shape: BoxShape.circle, |
There was a problem hiding this comment.
Fix compile error: Color.withValues doesn’t exist; use withOpacity
Flutter Color exposes withOpacity(double) and withAlpha(int). Replace withOpacity to match your 0–1 alpha math.
- color: Colors.white.withValues(
- alpha: 0.3 + (pulseController.value * 0.5),
- ),
+ color: Colors.white.withOpacity(
+ 0.3 + (pulseController.value * 0.5),
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| color: Colors.white.withValues( | |
| alpha: 0.3 + (pulseController.value * 0.5), | |
| ), | |
| shape: BoxShape.circle, | |
| color: Colors.white.withOpacity( | |
| 0.3 + (pulseController.value * 0.5), | |
| ), | |
| shape: BoxShape.circle, |
🤖 Prompt for AI Agents
In lib/features/roadster/widget/animated_star_widget.dart around lines 30 to 33,
the code calls Colors.white.withValues(...) which doesn't exist; replace
withColors.white.withOpacity(...) and pass the same 0.3 + (pulseController.value
* 0.5) expression (withOpacity expects a 0.0–1.0 double), so change the method
name to withOpacity and keep the alpha calculation as-is.
| "millionKm": "milhões km", | ||
| "missionDetails": "Detalhes da missão", | ||
| "trackLive": "Acompanhar ao vivo", | ||
| "marsDistance": "Distância a Marte", | ||
| "earthDistance": "Distância à Terra", | ||
| "currentSpeed": "Velocidade Atual", | ||
| "orbitalPeriod": "Período Orbital", | ||
| "unitDays": "dias", | ||
| "unitKph": "km/h", | ||
| "launched": "Lançado: {date}", | ||
| "roadsterTitle": "Roadster", | ||
| "roadsterDescription": "Tesla Roadster de Elon Musk" | ||
| } |
There was a problem hiding this comment.
Fix Portuguese phrasing and add placeholders metadata
- “milhões km” → “milhões de km”.
- Provide @launched metadata for typed placeholder generation (consistent with other keys).
- "millionKm": "milhões km",
+ "millionKm": "milhões de km",
@@
- "launched": "Lançado: {date}",
+ "launched": "Lançado: {date}",
+ "@launched": {
+ "description": "Data de lançamento formatada",
+ "placeholders": { "date": {} }
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "millionKm": "milhões km", | |
| "missionDetails": "Detalhes da missão", | |
| "trackLive": "Acompanhar ao vivo", | |
| "marsDistance": "Distância a Marte", | |
| "earthDistance": "Distância à Terra", | |
| "currentSpeed": "Velocidade Atual", | |
| "orbitalPeriod": "Período Orbital", | |
| "unitDays": "dias", | |
| "unitKph": "km/h", | |
| "launched": "Lançado: {date}", | |
| "roadsterTitle": "Roadster", | |
| "roadsterDescription": "Tesla Roadster de Elon Musk" | |
| } | |
| "millionKm": "milhões de km", | |
| "missionDetails": "Detalhes da missão", | |
| "trackLive": "Acompanhar ao vivo", | |
| "marsDistance": "Distância a Marte", | |
| "earthDistance": "Distância à Terra", | |
| "currentSpeed": "Velocidade Atual", | |
| "orbitalPeriod": "Período Orbital", | |
| "unitDays": "dias", | |
| "unitKph": "km/h", | |
| "launched": "Lançado: {date}", | |
| "@launched": { | |
| "description": "Data de lançamento formatada", | |
| "placeholders": { | |
| "date": {} | |
| } | |
| }, | |
| "roadsterTitle": "Roadster", | |
| "roadsterDescription": "Tesla Roadster de Elon Musk" | |
| } |
🤖 Prompt for AI Agents
In lib/l10n/intl_pt.arb around lines 237 to 249, fix the Portuguese phrasing and
add placeholder metadata: change the value for "millionKm" from "milhões km" to
"milhões de km", and add a @launched metadata entry for the "launched" key that
defines its placeholder (e.g., specifying type "String" or matching the
project's placeholder schema) so typed placeholder generation remains consistent
with other locale files.
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter_bloc_app_template/features/roadster/widget/animated_star_widget.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
|
|
||
| void main() { | ||
| testWidgets('AnimatedStarWidget renders correctly', | ||
| (WidgetTester tester) async { | ||
| final controller = AnimationController( | ||
| vsync: tester, | ||
| duration: const Duration(seconds: 1), | ||
| ); | ||
|
|
||
| await tester.pumpWidget( | ||
| MaterialApp( | ||
| home: Stack( | ||
| children: [ | ||
| AnimatedStarWidget(index: 42, pulseController: controller), | ||
| ], | ||
| ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Use a proper vsync provider in tests
WidgetTester isn’t a TickerProvider. Use TestVSync for AnimationController.
+import 'package:flutter/scheduler.dart';
@@
- final controller = AnimationController(
- vsync: tester,
+ final controller = AnimationController(
+ vsync: const TestVSync(),
duration: const Duration(seconds: 1),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import 'package:flutter/material.dart'; | |
| import 'package:flutter_bloc_app_template/features/roadster/widget/animated_star_widget.dart'; | |
| import 'package:flutter_test/flutter_test.dart'; | |
| void main() { | |
| testWidgets('AnimatedStarWidget renders correctly', | |
| (WidgetTester tester) async { | |
| final controller = AnimationController( | |
| vsync: tester, | |
| duration: const Duration(seconds: 1), | |
| ); | |
| await tester.pumpWidget( | |
| MaterialApp( | |
| home: Stack( | |
| children: [ | |
| AnimatedStarWidget(index: 42, pulseController: controller), | |
| ], | |
| ), | |
| ), | |
| ); | |
| import 'package:flutter/material.dart'; | |
| import 'package:flutter/scheduler.dart'; | |
| import 'package:flutter_bloc_app_template/features/roadster/widget/animated_star_widget.dart'; | |
| import 'package:flutter_test/flutter_test.dart'; | |
| void main() { | |
| testWidgets('AnimatedStarWidget renders correctly', | |
| (WidgetTester tester) async { | |
| final controller = AnimationController( | |
| vsync: const TestVSync(), | |
| duration: const Duration(seconds: 1), | |
| ); | |
| await tester.pumpWidget( | |
| MaterialApp( | |
| home: Stack( | |
| children: [ | |
| AnimatedStarWidget(index: 42, pulseController: controller), | |
| ], | |
| ), | |
| ), | |
| ); | |
| // ... | |
| }); | |
| } |
🤖 Prompt for AI Agents
In test/features/roadster/widget/animated_star_widget_test.dart around lines 1
to 21, the AnimationController is created with vsync: tester which is invalid
because WidgetTester is not a TickerProvider; replace vsync: tester with a
proper TestVSync instance (e.g., TestVSync()) when constructing the
AnimationController and register controller.dispose in a tearDown or addTearDown
so the controller is cleaned up after the test.
| // Start the animation | ||
| controller.value = 0.5; | ||
| await tester.pump(); | ||
|
|
||
| // Verify color alpha changed | ||
| final boxDecoration = container.decoration as BoxDecoration; | ||
| final alpha = boxDecoration.color?.a ?? 0; | ||
| expect(alpha, greaterThan(0)); // should be > 0 | ||
| }); |
There was a problem hiding this comment.
Verify color change correctly and use supported API
Re-read the widget after pumping and use Color.opacity (or alpha) instead of non-existent .a. Also compare before/after.
- // Start the animation
- controller.value = 0.5;
- await tester.pump();
-
- // Verify color alpha changed
- final boxDecoration = container.decoration as BoxDecoration;
- final alpha = boxDecoration.color?.a ?? 0;
- expect(alpha, greaterThan(0)); // should be > 0
+ // Opacity before
+ final before = ((tester.widget<Container>(containerFinder)
+ .decoration as BoxDecoration)
+ .color)!
+ .opacity;
+ // Advance the animation
+ controller.value = 0.5;
+ await tester.pump();
+ // Opacity after
+ final after = ((tester.widget<Container>(containerFinder)
+ .decoration as BoxDecoration)
+ .color)!
+ .opacity;
+ expect(after, greaterThan(before));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Start the animation | |
| controller.value = 0.5; | |
| await tester.pump(); | |
| // Verify color alpha changed | |
| final boxDecoration = container.decoration as BoxDecoration; | |
| final alpha = boxDecoration.color?.a ?? 0; | |
| expect(alpha, greaterThan(0)); // should be > 0 | |
| }); | |
| // Opacity before | |
| final before = ((tester.widget<Container>(containerFinder) | |
| .decoration as BoxDecoration) | |
| .color)! | |
| .opacity; | |
| // Advance the animation | |
| controller.value = 0.5; | |
| await tester.pump(); | |
| // Opacity after | |
| final after = ((tester.widget<Container>(containerFinder) | |
| .decoration as BoxDecoration) | |
| .color)! | |
| .opacity; | |
| expect(after, greaterThan(before)); |
🤖 Prompt for AI Agents
In test/features/roadster/widget/animated_star_widget_test.dart around lines 35
to 43, re-read the Container widget after calling await tester.pump() (e.g. use
tester.widget with the Container finder) to get the updated decoration, and
replace the non-existent .a access with the supported Color API (use
color.opacity for a double 0.0–1.0 or color.alpha for an int 0–255). Capture the
value before starting the animation and the value after the pump, then assert
the after value is greater than the before value using the appropriate
comparison for opacity (greaterThan) or alpha.
| flickrImages: [ | ||
| 'url1', | ||
| 'url2', | ||
| ], |
There was a problem hiding this comment.
Fix const-constructor compile error (non-const list argument).
const RoadsterResource(...) requires compile-time constant arguments. Make the flickrImages list const.
- flickrImages: [
+ flickrImages: const [
'url1',
'url2',
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| flickrImages: [ | |
| 'url1', | |
| 'url2', | |
| ], | |
| flickrImages: const [ | |
| 'url1', | |
| 'url2', | |
| ], |
🤖 Prompt for AI Agents
In test/models/roadster/roadster_resource_test.dart around lines 30 to 33, the
const RoadsterResource(...) instantiation fails because flickrImages is a
non-const List; change the list literal to a const list (e.g., const ['url1',
'url2']) so all constructor arguments are compile-time constants and the const
constructor compiles.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (2)
65-73: Coverage exclude path likely wrong: routers vs routesThe project uses lib/routes/router.dart (not lib/routers/router.dart). The wrong glob means the router file is still counted in coverage and can fail the threshold.
Apply this diff:
lcov --remove coverage/lcov.info \ 'lib/**.gen.dart' \ 'lib/generated/**' \ 'lib/di/**' \ 'lib/constants/**' \ - 'lib/routers/router.dart' \ + 'lib/routes/router.dart' \ -o coverage/lcov.info \ --ignore-errors unused
75-86: Fix the same path in very_good_coverage exclude listMirror the path correction here to keep CI consistent.
exclude: | **/*.g.dart **/*.gen.dart lib/di/** lib/generated/** lib/constants/** - lib/routers/router.dart + lib/routes/router.dart
♻️ Duplicate comments (1)
lib/features/roadster/utils/roadster_utils.dart (1)
27-68: Split-title widget builder looks good and addresses prior feedbackUses first-match split, preserves remainder, and supports configurable suffix. Nice improvement.
🧹 Nitpick comments (59)
.github/workflows/ci.yml (1)
120-139: Streamline integration test invocation and reduce flakiness
- Run all integration tests in one call to cut emulator boot overhead and stabilize timing, or pass a device explicitly.
- flutter pub get - flutter test integration_test/app_test.dart --flavor dev - flutter test integration_test/appearance_test.dart --flavor dev - flutter test integration_test/launch_navigation_test.dart --flavor dev - flutter test integration_test/launch_test.dart --flavor dev - flutter test integration_test/launches_mock_test.dart --flavor dev - flutter test integration_test/launches_test.dart --flavor dev - flutter test integration_test/settings_test.dart --flavor dev - flutter test integration_test/rockets_screen_integration_test.dart --flavor dev - flutter test integration_test/rocket_screen_test.dart --flavor dev - flutter test integration_test/rockets_integration_live_test.dart --flavor dev - flutter test integration_test/roadster_detail_integration_test.dart --flavor dev + flutter pub get + flutter devices + export DEVICE_ID=$(flutter devices | awk '/emulator/ {print $1; exit}') + flutter test integration_test --flavor dev -d "$DEVICE_ID"lib/features/roadster/widget/app_bar/gradient_overlay.dart (2)
19-21: Prefer withOpacity for broader SDK compatibilitywithValues(alpha: 0.7) is newer; withOpacity(0.7) is universally supported and equivalent here.
- Colors.black.withValues(alpha: 0.7), + Colors.black.withOpacity(0.7),
12-14: Consider making height configurableHardcoded 100 might not fit all headers. Expose a height parameter with a sensible default.
-class GradientOverlay extends StatelessWidget { - const GradientOverlay({super.key}); +class GradientOverlay extends StatelessWidget { + const GradientOverlay({super.key, this.height = 100}); + final double height; ... - child: Container( - height: 100, + child: Container( + height: height,lib/features/roadster/widget/app_bar/image_indicators.dart (3)
10-11: Clamp/validate currentIndex to avoid no-active stateIf currentIndex is out of range or negative, no indicator becomes “active”. Clamp or assert bounds.
- final int imageCount; - final int currentIndex; + final int imageCount; + final int currentIndex;Outside the shown lines:
assert(imageCount >= 0); final safeIndex = imageCount == 0 ? 0 : currentIndex.clamp(0, imageCount - 1);and use safeIndex in build.
29-31: Adjust inactive color for light themesColors.white30 may be invisible on light backgrounds. Derive from theme.
- : Colors.white30, + : Theme.of(context).brightness == Brightness.dark + ? Colors.white30 + : Colors.black26,
15-37: Optional: add semantics for accessibilityExpose “Image i of N” so TalkBack/VoiceOver convey context.
Semantics( label: 'Image ${index + 1} of $imageCount', selected: currentIndex == index, child: AnimatedContainer(...), )test/features/roadster/widget/links_section_widget_test.dart (1)
38-51: Stabilize taps with pumpAndSettleAfter taps, prefer pumpAndSettle() to process animations/frames.
- await tester.tap(wikipediaButton); - await tester.pump(); + await tester.tap(wikipediaButton); + await tester.pumpAndSettle(); - await tester.tap(watchVideoButton); - await tester.pump(); + await tester.tap(watchVideoButton); + await tester.pumpAndSettle();test/data/network/service/roadster/roadter_service_test.dart (2)
17-21: Unnecessary fallback registrationNo Uri is used in mocks here; remove redundant registerFallbackValue.
- setUp(() async { - registerFallbackValue(Uri()); + setUp(() async { service = MockRoadsterService(); dataSource = RoadsterNetworkDataSource(service); });
45-57: Minor: tighten verificationsAssert exact call counts for clarity.
- verify(() => service.fetchRoadster()); + verify(() => service.fetchRoadster()).called(1); expect(call, isA<Success<NetworkRoadsterModel>>()); - verifyNoMoreInteractions(service); + verifyNoMoreInteractions(service);Repeat similarly in the error tests.
lib/l10n/intl_en.arb (1)
383-391: Add placeholders metadata for launchedHelps gen-l10n produce a typed method and better translator context.
"launched": "Launched: {date}", + "@launched": { + "description": "Launch date label with formatted date", + "placeholders": { + "date": { + "type": "String", + "example": "Feb 6, 2018" + } + } + },lib/l10n/intl_de.arb (2)
246-254: Add ARB metadata for placeholder in 'launched'.Provide an @launched block with a 'date' placeholder for consistency with other param messages and clearer gen-l10n output.
"launched": "Gestartet: {date}", + "@launched": { + "description": "Label with the launch date", + "placeholders": { "date": {} } + },
237-244: German term nits: consider “Umlaufperiode”.Optional: “Orbitale Periode” → “Umlaufperiode”. “Millionen km” could also be “Mio. km” if you prefer abbreviated units.
lib/l10n/app_localizations_pt.dart (1)
360-431: Polish PT terms (grammar + domain-correct astronomy).Minor translation improvements for clarity and idiomatic PT-BR/pt-PT:
- millionKm: “milhões de km”
- currentSpeed: “Velocidade atual”
- orbitalPeriod: “Período orbital”
- semiMajorAxis: “Semi-eixo maior”
- apoapsis/periapsis: if Roadster is heliocentric, prefer “Afélio”/“Periélio”.
- String get millionKm => 'milhões km'; + String get millionKm => 'milhões de km'; - String get currentSpeed => 'Velocidade Atual'; + String get currentSpeed => 'Velocidade atual'; - String get orbitalPeriod => 'Período Orbital'; + String get orbitalPeriod => 'Período orbital'; - String get semiMajorAxis => 'Eixo semi-maior'; + String get semiMajorAxis => 'Semi-eixo maior'; - String get apoapsis => 'Apoápse'; + String get apoapsis => 'Afélio'; - String get periapsis => 'Periápse'; + String get periapsis => 'Periélio';If you prefer generic terms over heliocentric ones, use “Apocentro/Pericentro” instead.
lib/l10n/intl_uk.arb (1)
368-391: Unify style and tighten units.
- millionKm: prefer “млн км” (concise unit suffix).
- launched: consider “Запущено: {date}” to align with existing “launchedAt”.
- "millionKm": "мільйон км", + "millionKm": "млн км", - "launched": "Запуск: {date}", + "launched": "Запущено: {date}", + "@launched": { + "description": "Підпис із датою запуску", + "placeholders": { "date": {} } + },test/features/roadster/widget/app_bar/image_indicators_test.dart (2)
8-23: Scope the finder to the widget under test.Reduce false positives by counting indicators within ImageIndicators only.
- expect(find.byType(AnimatedContainer), findsNWidgets(3)); + expect( + find.descendant( + of: find.byType(ImageIndicators), + matching: find.byType(AnimatedContainer), + ), + findsNWidgets(3), + );
40-50: Stabilize size assertions with pumpAndSettle.If animation duration changes, this avoids flakiness before measuring sizes.
- final containers = find.byType(AnimatedContainer); + final containers = find.byType(AnimatedContainer); + await tester.pumpAndSettle();test/features/roadster/widget/app_bar/gradient_overlay_test.dart (1)
36-50: Also assert overlay height.Small addition to ensure the 100px overlay height remains stable.
final container = tester.widget<Container>(find.byType(Container)); expect(container.decoration, isA<BoxDecoration>()); final decoration = container.decoration as BoxDecoration; expect(decoration.gradient, isA<LinearGradient>()); + // Height is fixed to 100 in the widget. + expect(tester.getSize(find.byType(Container)).height, equals(100));test/features/roadster/widget/orbital_parameters_section_test.dart (2)
20-28: Stabilize builds with pumpAndSettle after pumpWidgetIf the section introduces any implicit animations/futures, add a settle to avoid flakiness.
Apply this diff in both tests right after pumpWidget:
await tester.pumpWidget( MaterialApp( localizationsDelegates: appLocalizationsDelegates, supportedLocales: appSupportedLocales, home: Scaffold( body: OrbitalParametersSection(roadster: mockRoadster), ), ), ); + await tester.pumpAndSettle();Also applies to: 35-43
20-28: DRY the MaterialApp boilerplateConsider extracting a small helper to wrap children with the localized MaterialApp and reuse it across tests in this suite (or move it into test helpers).
Example helper to add near the top of the file:
Widget buildTestable(Widget body) => MaterialApp( localizationsDelegates: appLocalizationsDelegates, supportedLocales: appSupportedLocales, home: Scaffold(body: body), );Then replace the inline MaterialApp with:
await tester.pumpWidget(buildTestable(OrbitalParametersSection(roadster: mockRoadster)));test/features/roadster/widget/launch_details_section_test.dart (2)
20-28: Add pumpAndSettle after pumpWidget to reduce flakinessPrevents timing issues if LaunchDetailsSection triggers animations or async work.
await tester.pumpWidget( MaterialApp( localizationsDelegates: appLocalizationsDelegates, supportedLocales: appSupportedLocales, home: Scaffold( body: LaunchDetailsSection(roadster: mockRoadster), ), ), ); + await tester.pumpAndSettle();Also applies to: 35-43
49-52: Centralize the 'Falcon Heavy' stringWidget hard-codes 'Falcon Heavy' (lib/features/roadster/widget/launch_details_section.dart:17); read it from the model, a shared constant or localization and have the test reference that same source (test/features/roadster/widget/launch_details_section_test.dart:51).
test/features/roadster/widget/distance_card_widget_test.dart (5)
53-64: Settle after building to ensure localizations/animations are readyAdd a settle after pumpWidget for stability.
await tester.pumpWidget( buildTestableWidget( DistanceCardWidget( title: 'Venus', distance: 108000000, color: Colors.orange, icon: Icons.star, delay: 0, pulseController: controller, ), ), ); + await tester.pumpAndSettle();Apply similarly in the other tests after pumpWidget.
69-75: Scope the Container finder to the card to avoid brittle matchesThe global byWidgetPredicate().last can break if other Containers with BoxDecoration appear. Scope to the DistanceCardWidget subtree.
- final initialContainer = tester.widget<Container>( - find - .byWidgetPredicate( - (w) => w is Container && w.decoration is BoxDecoration) - .last, - ); + final card = find.byType(DistanceCardWidget); + final decoratedContainers = find.descendant( + of: card, + matching: find.byWidgetPredicate( + (w) => w is Container && w.decoration is BoxDecoration, + ), + ); + final initialContainer = + tester.widgetList<Container>(decoratedContainers).last; ... - final updatedContainer = tester.widget<Container>( - find - .byWidgetPredicate( - (w) => w is Container && w.decoration is BoxDecoration) - .last, - ); + final updatedContainer = + tester.widgetList<Container>(decoratedContainers).last;Also applies to: 81-88
111-116: Target the correct Opacity widgetThere may be multiple Opacity widgets in the tree. Restrict the finder to the DistanceCardWidget subtree.
- final initialOpacity = tester.widget<Opacity>(find.byType(Opacity)).opacity; + final opacityFinder = find.descendant( + of: find.byType(DistanceCardWidget), + matching: find.byType(Opacity), + ); + final initialOpacity = + tester.widget<Opacity>(opacityFinder.first).opacity; ... - final finalOpacity = tester.widget<Opacity>(find.byType(Opacity)).opacity; + final finalOpacity = + tester.widget<Opacity>(opacityFinder.first).opacity;
89-90: Use isNot(equals(...)) for clearer intentSmall readability win for the matcher.
- expect(updatedColor != initialColor, true); + expect(updatedColor, isNot(equals(initialColor)));
18-20: Remove empty setUp or move controller lifecycle into itEither drop the empty setUp or initialize/dispose the controller in setUp/tearDown for consistency.
test/features/roadster/widget/background/animated_stars_field_test.dart (2)
11-15: Unify vsync provider for consistency across testsOther Roadster tests use the custom TestVSync from test helpers (and hide the flutter_test one). Consider aligning here to reduce surprises when switching implementations.
31-31: Avoid hard-coding the “50 stars” magic numberThe count is duplicated and tightly couples the test to an internal choice. Prefer exposing a public constant on AnimatedStarsField (or making the count injectable for tests) and referencing it, or at least centralize a local const in the test.
Also applies to: 62-71
test/features/roadster/widget/speed_distance_cards_test.dart (1)
30-41: Also assert the exact number of stat cardsUsing first/last can pass even if more cards slip in. Add a length check to lock the composition.
Apply this diff:
final statCards = tester.widgetList<AnimatedStatCardWidget>( find.byType(AnimatedStatCardWidget), ); + expect(statCards.length, equals(2)); // First card should be speed expect(statCards.first.icon, equals(Icons.speed)); expect(statCards.first.color, equals(Colors.blue));test/features/roadster/widget/mission_details_card_test.dart (1)
69-100: Rename locally-scoped slideController to avoid shadowing the group fieldMinor readability nit: the local variable shadows the setUp-created controller.
Apply this diff:
- final slideController = AnimationController( + final localController = AnimationController( vsync: tester, duration: const Duration(milliseconds: 400), ); @@ body: MissionDetailsCard( roadster: mockRoadster, - slideController: slideController, + slideController: localController, ), @@ - unawaited(slideController.forward()); + unawaited(localController.forward()); @@ - slideController.dispose(); + localController.dispose();test/features/roadster/widget/roadster_content_test.dart (1)
62-90: Reduce brittleness of spacing testCounting SizedBox instances is sensitive to layout refactors. Consider asserting explicit spacing values (e.g., byKey or byType with height predicate) or testing padding on known section wrappers instead.
test/features/roadster/widget/background/animated_gradient_background_test.dart (1)
11-16: Consistent vsync source (optional)Consider aligning with other tests by using the custom TestVSync from helpers (and hiding flutter_test’s), purely for consistency.
lib/features/roadster/widget/launch_details_section.dart (1)
15-19: Handle nullability/formatting and consider localization for vehicle
- If launchMassKg can be null, avoid rendering "null"; prefer a fallback (e.g., '—' or localized N/A).
- Hard-coded 'Falcon Heavy' should ideally be localized or sourced from data if available.
Can you confirm the nullability of RoadsterResource.launchMassKg and whether an l10n key exists for the vehicle label?
lib/features/roadster/widget/buttons/track_live_button.dart (1)
24-29: Add tooltip for accessibility; consider SafeArea for inset-aware placement.
- Add a tooltip so long-press users get a label.
- If this FAB can be occluded by system gestures/notches on some devices, consider wrapping the child with SafeArea(minimum: EdgeInsets.all(20)) while keeping the Positioned offsets unchanged in tests.
child: FloatingActionButton.extended( onPressed: onPressed, icon: const Icon(Icons.rocket_launch), label: Text(S.of(context).trackLive), + tooltip: S.of(context).trackLive, backgroundColor: Theme.of(context).colorScheme.primary, ),lib/features/roadster/widget/distance_cards.dart (1)
20-37: Avoid turning “unknown” distances into 0; prefer an explicit placeholder.Passing 0 for null distances risks misinforming users. Either let DistanceCardWidget accept null and render a placeholder, or pass a pre-formatted string like “N/A”.
Can DistanceCardWidget accept nullable distance or a text override for unknown values?
lib/features/roadster/widget/speed_distance_cards.dart (1)
23-25: Localize the “N/A” placeholder.Hard-coded “N/A” isn’t localized. Consider adding a localization key (e.g., notAvailable) and using it here.
Also applies to: 34-36
lib/features/roadster/widget/background/animated_gradient_background.dart (1)
16-35: Ensure full-screen fill and shave a minor rebuild cost.Use SizedBox.expand + DecoratedBox instead of Container so the background always fills its parent and avoids Container’s extra layout work.
- return Container( - decoration: BoxDecoration( - gradient: LinearGradient( + return SizedBox.expand( + child: DecoratedBox( + decoration: BoxDecoration( + gradient: LinearGradient( begin: Alignment.topLeft, end: Alignment.bottomRight, colors: [ Color.lerp( const Color(0xFF0D0E1C), const Color(0xFF1A1B3A), pulseController.value, )!, Color.lerp( const Color(0xFF1A1B3A), const Color(0xFF2D1B3D), pulseController.value, )!, ], ), - ), + ), );lib/features/roadster/widget/roadster_content.dart (2)
26-26: Remove stray “// <--- added” comment.Leftover note from development; safe to drop.
- child: SingleChildScrollView( // <--- added + child: SingleChildScrollView(
47-48: Make bottom spacer inset-aware to avoid FAB overlap.Use view padding to keep content clear of system insets and the floating button.
- const SizedBox(height: 100), + SizedBox(height: MediaQuery.of(context).viewPadding.bottom + 100),lib/features/roadster/widget/background/animated_stars_field.dart (1)
14-22: Allow stars to animate outside bounds (if desired).If star animations need overdraw, set clipBehavior to Clip.none on the Stack.
- return Stack( + return Stack( + clipBehavior: Clip.none, children: List.generate(lib/features/roadster/widget/mission_details_card.dart (2)
27-38: Avoid showing "null" in UI; format mission values safelyIf
noradId/orbitTypecan be null or empty, string interpolation will render "null". Format defensively.Apply:
@override Widget build(BuildContext context) { + String _safeText(Object? v) => + (v == null || (v is String && v.trim().isEmpty)) ? '-' : v.toString(); return SlideTransition( position: Tween<Offset>( begin: const Offset(0, 0.3), end: Offset.zero, ).animate(CurvedAnimation( parent: slideController, curve: Curves.easeOutCubic, )), child: DetailsCardWidget( description1: roadster.details ?? '', description2: '', missions: [ Mission( - name: '${roadster.noradId}', + name: _safeText(roadster.noradId), isPrimary: true, ), Mission( - name: '${roadster.orbitType}', + name: _safeText(roadster.orbitType), isPrimary: false, ), ], ), );
26-29: Empty secondary description—verify downstream handlingConfirm
DetailsCardWidgettreats an emptydescription2as intended (layout, spacing). If it supports nullables, prefernullto avoid rendering empty text.test/features/roadster/widget/buttons/track_live_button_test.dart (2)
90-93: Stabilize finder for Positioned to avoid false positivesScope the
Positionedlookup to the FAB ancestor to prevent clashes if otherPositionedwidgets exist.Apply:
- final positioned = tester.widget<Positioned>(find.byType(Positioned)); + final positioned = tester.widget<Positioned>( + find.ancestor( + of: find.byType(FloatingActionButton), + matching: find.byType(Positioned), + ).first, + );
182-186: Assert themed color explicitlyStrengthen the assertion by comparing to the theme’s primary color instead of only non-null.
Apply:
- final fab = tester.widget<FloatingActionButton>( - find.byType(FloatingActionButton), - ); - expect(fab.backgroundColor, isNotNull); + final fabFinder = find.byType(FloatingActionButton); + final fab = tester.widget<FloatingActionButton>(fabFinder); + final context = tester.element(fabFinder); + final expected = Theme.of(context).colorScheme.primary; + expect(fab.backgroundColor, equals(expected));test/features/roadster/helpers/simple_image_test_helpers.dart (2)
15-16:fitparameter is unusedEither remove it or use it to scale the placeholder content so tests reflect sizing behavior.
Apply:
final BoxFit fit; @override Widget build(BuildContext context) { return Container( width: width, height: height, decoration: BoxDecoration( color: Colors.grey[300], borderRadius: BorderRadius.circular(8), ), - child: Center( - child: Column( + child: Center( + child: FittedBox( + fit: fit, + child: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ Icon( Icons.image, color: Colors.grey[600], size: 24, ), const SizedBox(height: 4), Text( 'Test Image', style: TextStyle( color: Colors.grey[600], fontSize: 10, ), ), ], - ], - ), + ], + ), + ), ), );
12-16: MakeimageUrlobservable for tests/accessibilityExpose the
imageUrlvia semantics so tests can target specific placeholders and improve a11y.Apply:
- child: Center( - child: FittedBox( + child: Center( + child: Semantics( + label: 'Test image: $imageUrl', + child: FittedBox( fit: fit, child: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ Icon( Icons.image, color: Colors.grey[600], size: 24, ), const SizedBox(height: 4), Text( 'Test Image', style: TextStyle( color: Colors.grey[600], fontSize: 10, ), ), ], ), - ), + ), + ), ),Also applies to: 26-46
lib/features/roadster/widget/app_bar/roadster_app_bar.dart (2)
73-83: Avoid hard-coded title; prefer data-driven or localized stringUse
RoadsterResource(e.g.,widget.roadster.name) or an i18n key to support localization and future reuse.
41-56: Auto-scroll UX: pause on user interaction and handle image list updatesConsider pausing auto-scroll while the user is swiping and resuming afterward; also handle
imageschanges (e.g., after data load) by restarting/canceling the timer as needed (didUpdateWidgetwithlistEquals).I can draft a minimal
didUpdateWidgetand a listener onposition.isScrollingNotifierif you want.lib/features/roadster/roadster_screen.dart (3)
87-87: Remove unused _rotationController (allocates a ticker with a 20s infinite repeat)It never drives any widget. Drop it or wire it to a consumer to avoid unnecessary work.
- late AnimationController _rotationController; @@ - _rotationController = AnimationController( - duration: const Duration(seconds: 20), - vsync: this, - )..repeat(); @@ - _rotationController.dispose();Also applies to: 114-118, 133-133
21-26: Minor: Idiomatic read for DI
context.read<RoadsterRepository>()is a bit cleaner thanRepositoryProvider.of<RoadsterRepository>(context).- create: (context) => RoadsterBloc( - RepositoryProvider.of<RoadsterRepository>(context), - )..add( + create: (context) => RoadsterBloc( + context.read<RoadsterRepository>(), + )..add( const RoadsterLoadEvent(), ),
41-44: Const-construct AppBarsThey’re static here; mark as const to reduce rebuild cost.
- return Scaffold( - appBar: AppBar(), + return Scaffold( + appBar: const AppBar(), body: const LoadingContent(), ); @@ - return Scaffold( - appBar: AppBar(), + return Scaffold( + appBar: const AppBar(), body: ErrorContent(Also applies to: 51-53
lib/features/roadster/widget/app_bar/image_carousel.dart (2)
40-51: Add errorBuilder and gaplessPlayback for resilient image loadingPrevents red error boxes on failures and reduces flicker on fast page changes.
child: Image.network( imageUrl, fit: BoxFit.cover, + gaplessPlayback: true, loadingBuilder: (context, child, loadingProgress) { if (loadingProgress == null) return child; return Center( child: CircularProgressIndicator( value: _calculateProgress(loadingProgress), ), ); }, + errorBuilder: (context, error, stackTrace) => const Center( + child: Icon(Icons.broken_image_outlined), + ), ),
26-33: Optional: Avoid zero-sized cards for offscreen pagesA small floor keeps layout stable and avoids rapid size thrash.
- final cardHeight = transformedValue * 300; - final cardWidth = transformedValue * 400; + const minScale = 0.6; // keep some presence for neighbors + final scale = (transformedValue * (1 - minScale)) + minScale; + final cardHeight = scale * 300; + final cardWidth = scale * 400;test/features/roadster/helpers/test_helpers.dart (1)
62-74: Deduplicate wrappers: wrapWithMaterialApp and wrapWithScaffold are identicalKeep one helper (e.g.,
wrapWithScaffold) and remove the other to reduce noise.-/// Wrapper widget for testing widgets that need MaterialApp context -Widget wrapWithMaterialApp(Widget child) { - return MaterialApp( - home: Scaffold(body: child), - ); -} - -/// Wrapper widget for testing widgets that need Scaffold context -Widget wrapWithScaffold(Widget child) { +/// Wrapper for testing widgets that need MaterialApp/Scaffold +Widget wrapWithScaffold(Widget child) { return MaterialApp( home: Scaffold(body: child), ); }lib/l10n/app_localizations_de.dart (2)
405-407: Align phrasing with existing “launchedAt” stringUse “Gestartet am:” for consistency with Line 75’s “Gestartet am: …”.
@override String launched(Object date) { - return 'Gestartet: $date'; + return 'Gestartet am: $date'; }
375-376: German style tweaks for brevity/idiom (optional)
- Prefer compound nouns: “Orbitalparameter”.
- “Umlaufperiode” is more idiomatic than “Orbitale Periode”.
- “Mio. km” reads cleaner in tight UI than “Millionen km”.
- String get orbitalParameters => 'Orbitale Parameter'; + String get orbitalParameters => 'Orbitalparameter'; - String get orbitalPeriod => 'Orbitale Periode'; + String get orbitalPeriod => 'Umlaufperiode'; - String get millionKm => 'Millionen km'; + String get millionKm => 'Mio. km';Also applies to: 396-397, 378-379
lib/features/roadster/utils/roadster_utils.dart (3)
4-14: Respect active locale and fix extension name typo
- Rename to NumberFormatterExt.
- Use current locale by default and consistent fraction digits helpers.
-extension NumberFormatterEXt on double { - String formatSpeed() { - final formatter = NumberFormat('#,##0.00', 'en_US'); - return formatter.format(this); - } - - String formatPeriod() { - final formatter = NumberFormat('0.0', 'en_US'); - return formatter.format(this); - } -} +extension NumberFormatterExt on double { + String formatSpeed({String? locale, int fractionDigits = 2}) { + final loc = locale ?? Intl.getCurrentLocale(); + final f = NumberFormat.decimalPattern(loc) + ..minimumFractionDigits = fractionDigits + ..maximumFractionDigits = fractionDigits; + return f.format(this); + } + + String formatPeriod({String? locale, int fractionDigits = 1}) { + final loc = locale ?? Intl.getCurrentLocale(); + final f = NumberFormat.decimalPattern(loc) + ..minimumFractionDigits = fractionDigits + ..maximumFractionDigits = fractionDigits; + return f.format(this); + } +}
17-25: Default date formatting to current locale (keep caller override)-extension RoadsterStringExt on String { - String toFormattedDate({String locale = 'en_US'}) { +extension RoadsterStringExt on String { + String toFormattedDate({String? locale}) { try { final dateTime = DateTime.parse(this).toLocal(); - final formatter = DateFormat.yMMMd(locale); + final formatter = DateFormat.yMMMd(locale ?? Intl.getCurrentLocale()); return formatter.format(dateTime); } catch (e) { return this; // fallback if parsing fails } }
71-90: Rename extension and guard fraction digits (optional polish)
- Typo: “DoublRoadsterExtension” → “DoubleRoadsterExtension”.
- Clamp negative fraction digits to 0.
-extension DoublRoadsterExtension on double { +extension DoubleRoadsterExtension on double { /// Converts a double to a formatted string in Astronomical Units (AU) /// Example: 1.664332332453025 -> "1.664 AU" String toAuString({int fractionDigits = 3}) { - return '${toStringAsFixed(fractionDigits)} AU'; + final fd = fractionDigits < 0 ? 0 : fractionDigits; + return '${toStringAsFixed(fd)} AU'; } /// Converts a double to a string with fixed decimal places /// Example: 0.2559348215918733 -> "0.256" String toFixedString({int fractionDigits = 3}) { - return toStringAsFixed(fractionDigits); + final fd = fractionDigits < 0 ? 0 : fractionDigits; + return toStringAsFixed(fd); } /// Converts a double to degrees with a ° suffix /// Example: 1.075052357364693 -> "1.075°" /// Example: 316.9112133411523 -> "316.91°" String toDegreeString({int fractionDigits = 2}) { - return '${toStringAsFixed(fractionDigits)}°'; + final fd = fractionDigits < 0 ? 0 : fractionDigits; + return '${toStringAsFixed(fd)}°'; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
lib/generated/intl/messages_de.dartis excluded by!**/generated/**lib/generated/intl/messages_en.dartis excluded by!**/generated/**lib/generated/intl/messages_pt.dartis excluded by!**/generated/**lib/generated/intl/messages_uk.dartis excluded by!**/generated/**lib/generated/l10n.dartis excluded by!**/generated/**
📒 Files selected for processing (52)
.github/workflows/ci.yml(1 hunks)lib/data/network/data_source/roadster_network_data_source.dart(1 hunks)lib/features/roadster/model/mission.dart(1 hunks)lib/features/roadster/model/orbital_data.dart(1 hunks)lib/features/roadster/roadster_screen.dart(1 hunks)lib/features/roadster/utils/roadster_utils.dart(1 hunks)lib/features/roadster/widget/app_bar/gradient_overlay.dart(1 hunks)lib/features/roadster/widget/app_bar/image_carousel.dart(1 hunks)lib/features/roadster/widget/app_bar/image_indicators.dart(1 hunks)lib/features/roadster/widget/app_bar/roadster_app_bar.dart(1 hunks)lib/features/roadster/widget/app_bar/roadster_title_section.dart(1 hunks)lib/features/roadster/widget/background/animated_gradient_background.dart(1 hunks)lib/features/roadster/widget/background/animated_stars_field.dart(1 hunks)lib/features/roadster/widget/buttons/track_live_button.dart(1 hunks)lib/features/roadster/widget/distance_card_widget.dart(1 hunks)lib/features/roadster/widget/distance_cards.dart(1 hunks)lib/features/roadster/widget/launch_details_section.dart(1 hunks)lib/features/roadster/widget/mission_details_card.dart(1 hunks)lib/features/roadster/widget/orbital_parameters_section.dart(1 hunks)lib/features/roadster/widget/orbital_section_widget.dart(1 hunks)lib/features/roadster/widget/roadster_content.dart(1 hunks)lib/features/roadster/widget/speed_distance_cards.dart(1 hunks)lib/l10n/app_localizations.dart(1 hunks)lib/l10n/app_localizations_de.dart(1 hunks)lib/l10n/app_localizations_en.dart(1 hunks)lib/l10n/app_localizations_pt.dart(1 hunks)lib/l10n/app_localizations_uk.dart(1 hunks)lib/l10n/intl_de.arb(1 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_pt.arb(1 hunks)lib/l10n/intl_uk.arb(2 hunks)test/data/network/service/roadster/roadter_service_test.dart(1 hunks)test/features/roadster/helpers/simple_image_test_helpers.dart(1 hunks)test/features/roadster/helpers/test_helpers.dart(1 hunks)test/features/roadster/model/mission_test.dart(1 hunks)test/features/roadster/model/orbital_data_test.dart(1 hunks)test/features/roadster/performance/widget_performance_test.dart(1 hunks)test/features/roadster/utils/roadster_utils_test.dart(1 hunks)test/features/roadster/widget/animated_star_widget_test.dart(1 hunks)test/features/roadster/widget/app_bar/gradient_overlay_test.dart(1 hunks)test/features/roadster/widget/app_bar/image_indicators_test.dart(1 hunks)test/features/roadster/widget/background/animated_gradient_background_test.dart(1 hunks)test/features/roadster/widget/background/animated_stars_field_test.dart(1 hunks)test/features/roadster/widget/buttons/track_live_button_test.dart(1 hunks)test/features/roadster/widget/distance_card_widget_test.dart(1 hunks)test/features/roadster/widget/distance_cards_test.dart(1 hunks)test/features/roadster/widget/launch_details_section_test.dart(1 hunks)test/features/roadster/widget/links_section_widget_test.dart(1 hunks)test/features/roadster/widget/mission_details_card_test.dart(1 hunks)test/features/roadster/widget/orbital_parameters_section_test.dart(1 hunks)test/features/roadster/widget/roadster_content_test.dart(1 hunks)test/features/roadster/widget/speed_distance_cards_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- test/features/roadster/utils/roadster_utils_test.dart
- test/features/roadster/widget/animated_star_widget_test.dart
- lib/l10n/app_localizations_uk.dart
- lib/features/roadster/model/mission.dart
- lib/l10n/app_localizations_en.dart
- lib/features/roadster/widget/orbital_section_widget.dart
- lib/features/roadster/widget/distance_card_widget.dart
- lib/l10n/intl_pt.arb
- lib/data/network/data_source/roadster_network_data_source.dart
- lib/features/roadster/model/orbital_data.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: App
- GitHub Check: Flutter Integration Tests (31)
🔇 Additional comments (26)
test/features/roadster/widget/links_section_widget_test.dart (1)
16-21: LGTM — localized labels coveredGood smoke tests for labels rendering.
test/data/network/service/roadster/roadter_service_test.dart (1)
59-66: Dio v5: RequestOptions requires pathThis will fail to compile on Dio 5. Provide a path.
- DioException( - requestOptions: RequestOptions(), - ), + DioException( + requestOptions: RequestOptions(path: '/roadster'), + ),Likely an incorrect or invalid review comment.
lib/l10n/intl_en.arb (2)
369-391: LGTM — keys align with Roadster UI/testsNew strings cover all Roadster surfaces referenced by widgets and tests.
369-391: Verify parity across localesSandbox verification failed (/dev/fd/63). Ensure the following keys from lib/l10n/intl_en.arb exist in every lib/l10n/intl_*.arb (except intl_en.arb): learnMore, launchInformation, launchMass, launchVehicle, orbitalParameters, millionKm, missionDetails, trackLive, marsDistance, earthDistance, currentSpeed, orbitalPeriod, unitDays, unitKph, launched, roadsterTitle, roadsterDescription, apoapsis, periapsis, semiMajorAxis, eccentricity, inclination, longitude.
lib/l10n/app_localizations.dart (1)
771-907: API additions verified — all locales implement new keys.Verification script confirms the abstract interface, locale classes (de, en, pt, uk) and ARB files contain every new getter/method (script output: All good).
lib/l10n/intl_uk.arb (1)
339-349: Metadata reformat LGTM.
Structured @activeStatus/@retiredStatus/@SuccessRate blocks read better; no functional impact.test/features/roadster/model/orbital_data_test.dart (1)
5-79: Solid equality/hash tests.Covers equality, inequality by each field, and props content. Good job.
test/features/roadster/model/mission_test.dart (1)
4-34: LGTM on value semantics coverage.Tests check equality, differing fields, and props contents succinctly.
test/features/roadster/widget/app_bar/image_indicators_test.dart (1)
67-74: Positioning checks look good.
Assertions clearly validate bottom-centered layout.test/features/roadster/widget/app_bar/gradient_overlay_test.dart (2)
7-19: Smoke render test is clear.
Presence of Positioned and Container is verified succinctly.
21-34: Positioning assertions LGTM.
Bottom/left/right edges validated correctly.test/features/roadster/widget/orbital_parameters_section_test.dart (1)
18-31: LGTM: Composition smoke test is clear and targetedAsserting the presence of OrbitalSectionWidget is a good sanity check for composition.
test/features/roadster/widget/launch_details_section_test.dart (1)
18-31: LGTM: Launch section presence check is appropriateThe first test cleanly validates composition without over-coupling to implementation details.
test/features/roadster/widget/background/animated_stars_field_test.dart (1)
21-32: LGTM: solid structural assertionsBuilds the widget and verifies expected children cleanly. No issues.
test/features/roadster/widget/speed_distance_cards_test.dart (1)
18-41: LGTM: icon/color mapping checks outGood coverage of the visual identity for both cards.
test/features/roadster/widget/distance_cards_test.dart (2)
27-43: LGTM: verifies exact card countThe findsNWidgets(2) assertion guards composition correctly.
73-95: Controller propagation test looks goodForwarding pulseController to each card is validated cleanly.
test/features/roadster/widget/mission_details_card_test.dart (1)
29-46: LGTM: structure assertions are clearPresence of SlideTransition and DetailsCardWidget is validated appropriately.
test/features/roadster/widget/roadster_content_test.dart (2)
38-60: LGTM: composition checks cover all sectionsVerifies all key child widgets are present. Looks good.
92-117: LGTM: controller wiring verifiedEnsures the right controllers reach the intended children.
test/features/roadster/widget/background/animated_gradient_background_test.dart (1)
22-34: LGTM: structure and decoration checks are appropriateAnimatedBuilder + Container presence validated; good coverage.
lib/features/roadster/widget/launch_details_section.dart (1)
5-19: LGTM: concise section wrapperSimple pass-through to LaunchSectionWidget matches the intended API.
lib/features/roadster/widget/app_bar/roadster_title_section.dart (1)
27-29: Localize the fallback title.Hard-coded English fallback breaks i18n — use a localized fallback string (e.g. context.l10n.roadsterTitleFallback). If the key doesn't exist, add one and use it here.
File: lib/features/roadster/widget/app_bar/roadster_title_section.dart (lines 27–29)
lib/features/roadster/widget/app_bar/roadster_app_bar.dart (1)
58-63: Good lifecycle hygieneDisposing the controller and canceling the timer prevents leaks. LGTM.
test/features/roadster/helpers/test_helpers.dart (1)
96-104: LGTM: Test AnimationController factoryClean, deterministic controller construction for tests.
lib/l10n/app_localizations_de.dart (1)
430-431: “longitude” likely means orbital element (not geographic “Längengrad”)If this is the longitude of the ascending node, translate as “Länge des aufsteigenden Knotens”. If it’s truly geographic, keep “Längengrad”. Please confirm usage in the Roadster UI/model.
- String get longitude => 'Längengrad'; + String get longitude => 'Länge des aufsteigenden Knotens';
| const SizedBox(height: 8), | ||
| Row( | ||
| children: [ | ||
| Icon( | ||
| Icons.rocket_launch, | ||
| color: Colors.orange[400], | ||
| size: 20, | ||
| ), | ||
| const SizedBox(width: 8), | ||
| Text( | ||
| S.of(context).launched( | ||
| roadster.launchDateUtc?.toFormattedDate() ?? '', | ||
| ), | ||
| style: TextStyle( | ||
| color: Colors.orange[400], | ||
| fontSize: 16, | ||
| fontWeight: FontWeight.w500, | ||
| ), | ||
| ), | ||
| ], | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hide the “launched” row when the date is missing.
Rendering with an empty date yields awkward UI. Only show the row when launchDateUtc is present.
- const SizedBox(height: 8),
- Row(
- children: [
- Icon(
- Icons.rocket_launch,
- color: Colors.orange[400],
- size: 20,
- ),
- const SizedBox(width: 8),
- Text(
- S.of(context).launched(
- roadster.launchDateUtc?.toFormattedDate() ?? '',
- ),
- style: TextStyle(
- color: Colors.orange[400],
- fontSize: 16,
- fontWeight: FontWeight.w500,
- ),
- ),
- ],
- ),
+ const SizedBox(height: 8),
+ if (roadster.launchDateUtc != null)
+ Row(
+ children: [
+ Icon(
+ Icons.rocket_launch,
+ color: Colors.orange[400],
+ size: 20,
+ ),
+ const SizedBox(width: 8),
+ Text(
+ S.of(context).launched(
+ roadster.launchDateUtc!.toFormattedDate(),
+ ),
+ style: TextStyle(
+ color: Colors.orange[400],
+ fontSize: 16,
+ fontWeight: FontWeight.w500,
+ ),
+ ),
+ ],
+ ),🤖 Prompt for AI Agents
In lib/features/roadster/widget/app_bar/roadster_title_section.dart around lines
29 to 49, the "launched" Row is rendered even when roadster.launchDateUtc is
null which shows an empty/awkward UI; update the widget tree to only render that
Row when roadster.launchDateUtc is non-null (e.g., wrap the Row in a conditional
check or use a null-aware builder: if roadster.launchDateUtc != null then return
the Row with the icon and formatted date, otherwise return SizedBox.shrink() /
nothing).
| label: S.of(context).apoapsis, | ||
| value: '${roadster.apoapsisAu?.toAuString()}', | ||
| icon: Icons.arrow_upward, | ||
| ), | ||
| OrbitalData( | ||
| label: S.of(context).periapsis, | ||
| value: '${roadster.periapsisAu?.toAuString()}', | ||
| icon: Icons.arrow_downward, | ||
| ), | ||
| OrbitalData( | ||
| label: S.of(context).semiMajorAxis, | ||
| value: '${roadster.semiMajorAxisAu?.toAuString()}', | ||
| icon: Icons.circle_outlined, | ||
| ), | ||
| OrbitalData( | ||
| label: S.of(context).eccentricity, | ||
| value: '${roadster.eccentricity?.toFixedString()}', | ||
| icon: Icons.blur_circular, | ||
| ), | ||
| OrbitalData( | ||
| label: S.of(context).inclination, | ||
| value: '${roadster.inclination?.toDegreeString()}', | ||
| icon: Icons.trending_up, | ||
| ), | ||
| OrbitalData( | ||
| label: S.of(context).longitude, | ||
| value: '${roadster.longitude?.toDegreeString()}', | ||
| icon: Icons.explore, | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent literal “null” values from leaking into the UI.
Interpolating nullable values yields the string “null”. Provide explicit fallbacks.
OrbitalData(
label: S.of(context).apoapsis,
- value: '${roadster.apoapsisAu?.toAuString()}',
+ value: roadster.apoapsisAu?.toAuString() ?? 'N/A',
icon: Icons.arrow_upward,
),
OrbitalData(
label: S.of(context).periapsis,
- value: '${roadster.periapsisAu?.toAuString()}',
+ value: roadster.periapsisAu?.toAuString() ?? 'N/A',
icon: Icons.arrow_downward,
),
OrbitalData(
label: S.of(context).semiMajorAxis,
- value: '${roadster.semiMajorAxisAu?.toAuString()}',
+ value: roadster.semiMajorAxisAu?.toAuString() ?? 'N/A',
icon: Icons.circle_outlined,
),
OrbitalData(
label: S.of(context).eccentricity,
- value: '${roadster.eccentricity?.toFixedString()}',
+ value: roadster.eccentricity?.toFixedString() ?? 'N/A',
icon: Icons.blur_circular,
),
OrbitalData(
label: S.of(context).inclination,
- value: '${roadster.inclination?.toDegreeString()}',
+ value: roadster.inclination?.toDegreeString() ?? 'N/A',
icon: Icons.trending_up,
),
OrbitalData(
label: S.of(context).longitude,
- value: '${roadster.longitude?.toDegreeString()}',
+ value: roadster.longitude?.toDegreeString() ?? 'N/A',
icon: Icons.explore,
),Follow-up: localize the “N/A” placeholder similar to SpeedDistanceCards.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| label: S.of(context).apoapsis, | |
| value: '${roadster.apoapsisAu?.toAuString()}', | |
| icon: Icons.arrow_upward, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).periapsis, | |
| value: '${roadster.periapsisAu?.toAuString()}', | |
| icon: Icons.arrow_downward, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).semiMajorAxis, | |
| value: '${roadster.semiMajorAxisAu?.toAuString()}', | |
| icon: Icons.circle_outlined, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).eccentricity, | |
| value: '${roadster.eccentricity?.toFixedString()}', | |
| icon: Icons.blur_circular, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).inclination, | |
| value: '${roadster.inclination?.toDegreeString()}', | |
| icon: Icons.trending_up, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).longitude, | |
| value: '${roadster.longitude?.toDegreeString()}', | |
| icon: Icons.explore, | |
| ), | |
| label: S.of(context).apoapsis, | |
| value: roadster.apoapsisAu?.toAuString() ?? 'N/A', | |
| icon: Icons.arrow_upward, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).periapsis, | |
| value: roadster.periapsisAu?.toAuString() ?? 'N/A', | |
| icon: Icons.arrow_downward, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).semiMajorAxis, | |
| value: roadster.semiMajorAxisAu?.toAuString() ?? 'N/A', | |
| icon: Icons.circle_outlined, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).eccentricity, | |
| value: roadster.eccentricity?.toFixedString() ?? 'N/A', | |
| icon: Icons.blur_circular, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).inclination, | |
| value: roadster.inclination?.toDegreeString() ?? 'N/A', | |
| icon: Icons.trending_up, | |
| ), | |
| OrbitalData( | |
| label: S.of(context).longitude, | |
| value: roadster.longitude?.toDegreeString() ?? 'N/A', | |
| icon: Icons.explore, | |
| ), |
🤖 Prompt for AI Agents
In lib/features/roadster/widget/orbital_parameters_section.dart around lines
21-49, the current string interpolation of nullable orbital fields can produce
the literal "null" in the UI; change each value expression to provide an
explicit localized fallback (e.g. replace '${roadster.xxx?.toYyyString()}' with
roadster.xxx != null ? roadster.xxx!.toYyyString() : S.of(context).notAvailable
or the same localized "N/A" string used by SpeedDistanceCards), ensuring you
import/use the correct localization key from S and remove unnecessary
interpolation so the placeholder is shown when values are null.
| @@ -0,0 +1,77 @@ | |||
| import 'package:dio/dio.dart'; | |||
There was a problem hiding this comment.
File name typo
roadter_service_test.dart → roadster_service_test.dart (discoverability, consistency).
🤖 Prompt for AI Agents
In test/data/network/service/roadster/roadter_service_test.dart around line 1,
the filename contains a typo ("roadter" → "roadster"); rename the file to
roadster_service_test.dart and update all references to it (imports, test group
names, CI/test patterns, and any path usages in pubspec.yaml or test runner
configs) so they use the corrected name; ensure any internal test descriptions
or constants that mirror the filename are updated for consistency and run the
test suite to verify no path/import breakage.
| // Measure rendering performance | ||
| final stopwatch = Stopwatch()..start(); | ||
|
|
||
| // Run animation for multiple cycles | ||
| for (var i = 0; i < 10; i++) { | ||
| unawaited(controller.forward()); | ||
| await tester.pump(const Duration(milliseconds: 100)); | ||
| unawaited(controller.reverse()); | ||
| await tester.pump(const Duration(milliseconds: 100)); | ||
| } | ||
|
|
||
| stopwatch.stop(); | ||
|
|
||
| // Verify acceptable performance | ||
| expect(stopwatch.elapsedMilliseconds, lessThan(2000)); // 2 seconds max | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid wall-clock timing in widget tests to prevent flakes
Stopwatch-based thresholds are environment-dependent and can cause flaky CI. Drive animations with tester.pump and assert absence of exceptions instead.
Apply this diff:
- // Measure rendering performance
- final stopwatch = Stopwatch()..start();
-
// Run animation for multiple cycles
for (var i = 0; i < 10; i++) {
unawaited(controller.forward());
await tester.pump(const Duration(milliseconds: 100));
unawaited(controller.reverse());
await tester.pump(const Duration(milliseconds: 100));
}
- stopwatch.stop();
-
- // Verify acceptable performance
- expect(stopwatch.elapsedMilliseconds, lessThan(2000)); // 2 seconds max
+ // Verify no errors during animation cycles
+ expect(tester.takeException(), isNull);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Measure rendering performance | |
| final stopwatch = Stopwatch()..start(); | |
| // Run animation for multiple cycles | |
| for (var i = 0; i < 10; i++) { | |
| unawaited(controller.forward()); | |
| await tester.pump(const Duration(milliseconds: 100)); | |
| unawaited(controller.reverse()); | |
| await tester.pump(const Duration(milliseconds: 100)); | |
| } | |
| stopwatch.stop(); | |
| // Verify acceptable performance | |
| expect(stopwatch.elapsedMilliseconds, lessThan(2000)); // 2 seconds max | |
| // Run animation for multiple cycles | |
| for (var i = 0; i < 10; i++) { | |
| unawaited(controller.forward()); | |
| await tester.pump(const Duration(milliseconds: 100)); | |
| unawaited(controller.reverse()); | |
| await tester.pump(const Duration(milliseconds: 100)); | |
| } | |
| // Verify no errors during animation cycles | |
| expect(tester.takeException(), isNull); |
🤖 Prompt for AI Agents
In test/features/roadster/performance/widget_performance_test.dart around lines
21 to 36, remove the wall‑clock Stopwatch timing and the expect on
elapsedMilliseconds; instead drive the animation deterministically with
tester.pump() calls (e.g. pump the exact durations needed for forward and
reverse in the loop or use tester.pumpAndSettle where appropriate) to exercise
multiple cycles and then assert that no exceptions were thrown or that the
animation reached the expected state(s); ensure you do not rely on real time
thresholds and keep the loop deterministic so the test is stable in CI.
test/features/roadster/performance/widget_performance_test.dart
Outdated
Show resolved
Hide resolved
| final fadeController = AnimationController( | ||
| vsync: tester, | ||
| duration: const Duration(milliseconds: 500), | ||
| ); |
There was a problem hiding this comment.
Fix invalid vsync provider
tester is not a TickerProvider. Use TestVSync (as above) to avoid runtime failures.
Apply:
- final fadeController = AnimationController(
- vsync: tester,
- duration: const Duration(milliseconds: 500),
- );
+ final fadeController = AnimationController(
+ vsync: const TestVSync(),
+ duration: const Duration(milliseconds: 500),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final fadeController = AnimationController( | |
| vsync: tester, | |
| duration: const Duration(milliseconds: 500), | |
| ); | |
| final fadeController = AnimationController( | |
| vsync: const TestVSync(), | |
| duration: const Duration(milliseconds: 500), | |
| ); |
🤖 Prompt for AI Agents
In test/features/roadster/widget/buttons/track_live_button_test.dart around
lines 124 to 127, the AnimationController is using tester as the vsync provider
which is invalid; replace the vsync argument with a TestVSync instance (the same
TestVSync used earlier in the file) so the controller is created with a proper
TickerProvider (e.g., use vsync: TestVSync() or a shared TestVSync variable),
and ensure any created TestVSync is disposed/handled consistently with other
test setup.
| controller = AnimationController( | ||
| vsync: tester, | ||
| duration: const Duration(seconds: 1), | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix AnimationController vsync: WidgetTester is not a TickerProvider
Use TestVSync (from flutter_test) or wrap with a TickerProvider; otherwise tests may not compile or will throw ticker errors.
- controller = AnimationController(
- vsync: tester,
- duration: const Duration(seconds: 1),
- );
+ controller = AnimationController(
+ vsync: const TestVSync(),
+ duration: const Duration(seconds: 1),
+ );Repeat the same change in the other two tests. Also add a tearDown to dispose the controller to avoid ticker leaks:
tearDown(() {
controller.dispose();
});Quick scan for similar patterns:
Also applies to: 48-51, 93-96
🏁 Script executed:
#!/bin/bash
rg -nP --type=dart -g '!**/build/**' -g '!**/.dart_tool/**' 'vsync:\s*tester'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Searching for 'vsync: tester' in tests =="
rg -n --hidden -S 'vsync:\s*tester' -g 'test/**' -g '!**/build/**' -g '!**/.dart_tool/**' || true
echo "== Finding AnimationController instantiations (context shown) =="
rg -n --hidden -S 'AnimationController\(' -g 'test/**' -g '!**/build/**' -g '!**/.dart_tool/**' -C3 || true
echo "== Looking for TestVSync usage =="
rg -n --hidden -S 'TestVSync' -g 'test/**' -g '!**/build/**' -g '!**/.dart_tool/**' || true
echo "== Checking for controller.dispose() or tearDown disposing controller =="
rg -n --hidden -S 'controller\.dispose\(|tearDown\(\s*\)\s*\{' -g 'test/**' -g '!**/build/**' -g '!**/.dart_tool/**' -C3 || trueLength of output: 14010
Fix AnimationController vsync and dispose controllers (WidgetTester is not a TickerProvider)
Replace usages of "vsync: tester" with the repository's TestVSync and ensure controllers are disposed to avoid ticker errors/leaks.
Files to change:
- test/features/roadster/widget/distance_card_widget_test.dart — lines ~23-26, 48-51, 93-96.
- test/features/roadster/widget/mission_details_card_test.dart — lines ~71-74.
- test/features/roadster/widget/buttons/track_live_button_test.dart — lines ~124-127.
Suggested changes (apply same replacement to all occurrences):
- controller = AnimationController(
- vsync: tester,
- duration: const Duration(seconds: 1),
- );
+ controller = AnimationController(
+ vsync: const TestVSync(),
+ duration: const Duration(seconds: 1),
+ );Ensure controllers are disposed:
- If controller is a file-level/outer variable: add a tearDown that disposes it:
tearDown(() {
controller.dispose();
});- If controller is local to a test: use addTearDown(controller.dispose) or call controller.dispose() before the test ends.
[TestVSync is defined in test/features/roadster/helpers/test_helpers.dart; reuse it.]
🤖 Prompt for AI Agents
In test/features/roadster/widget/distance_card_widget_test.dart around lines
23-26, 48-51 and 93-96, replace vsync: tester with vsync: TestVSync() (TestVSync
from test/features/roadster/helpers/test_helpers.dart) and ensure each
AnimationController is disposed: if controller is declared at file/outer scope
add a tearDown that calls controller.dispose(), if controller is local to a test
use addTearDown(controller.dispose) or call controller.dispose() before test
end; also apply the same vsync replacement and disposal fix in
test/features/roadster/widget/mission_details_card_test.dart around lines 71-74
and in test/features/roadster/widget/buttons/track_live_button_test.dart around
lines 124-127.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
test/features/roadster/roadster_detail_screen.dart (4)
1-1: Constrain to VM to avoid dart:io issues on web runnersIf your CI ever runs Flutter tests on Chrome, importing dart:io will fail. Add a test-platform annotation.
+@TestOn('vm') import 'dart:io';
15-15: Don't import another test file; extract shared fixturesImporting
../../repository/roadster_repository_test.dartcouples tests and can unexpectedly register other tests. MovemockRoadsterResourceinto a shared fixture (e.g.,test/fixtures/roadster_fixtures.dart) and import that instead.
25-26: Use valid HTTP URLs for mocked network imagesRelative paths may not be treated as network URLs by Image/CachedNetworkImage. Prefer explicit https URLs to ensure
network_image_mockintercepts them.- mockImages = ['image1.jpg', 'image2.jpg']; + mockImages = [ + 'https://example.com/image1.jpg', + 'https://example.com/image2.jpg', + ];
29-40: Stabilize the pump sequence for async image resolutionWrap the callback as async and add an extra
pump()afterpumpWidgetto flush the first frame while keeping animations running.- await mockNetworkImagesFor( - () => tester.pumpWidget( - MaterialApp( - supportedLocales: appSupportedLocales, - localizationsDelegates: appLocalizationsDelegates, - home: RoadsterDetailScreen( - roadster: mockRoadster, - images: mockImages, - ), - ), - ), - ); + await mockNetworkImagesFor(() async { + await tester.pumpWidget( + MaterialApp( + supportedLocales: appSupportedLocales, + localizationsDelegates: appLocalizationsDelegates, + home: RoadsterDetailScreen( + roadster: mockRoadster, + images: mockImages, + ), + ), + ); + await tester.pump(); // let first frame render + });test/features/roadster/widget/app_bar/image_carousel_test.dart (3)
40-42: Avoid brittle UI assertions (Card).Asserting on Card ties the test to a specific implementation. Prefer asserting images rendered.
Apply:
- expect(find.byType(Card), findsAtLeastNWidgets(1)); + expect(find.byType(Image), findsNWidgets(mockImages.length));
62-67: Make the parallax assertion tolerant to float math.Matrix math can introduce minor rounding; use a tolerance.
Apply:
- expect( - transform.transform.getTranslation().y, equals(scrollOffset * 0.5)); + expect( + transform.transform.getTranslation().y, + closeTo(scrollOffset * 0.5, 1e-3), + );
91-93: Drag using the actual page width to reduce flakiness.Hardcoding -400 can fail on non-default test sizes.
Apply:
- // Drag by the width of the page to trigger page change - await tester.drag(find.byType(PageView), const Offset(-400, 0)); + // Drag by the actual page width to trigger page change + final pageWidth = tester.getSize(find.byType(PageView)).width; + await tester.drag(find.byType(PageView), Offset(-pageWidth, 0));test/features/roadster/widget/app_bar/roadster_app_bar_test.dart (4)
78-79: Avoid hard-coded title; assert via fixture data.Use the resource name to decouple from string changes/localization.
Apply:
- expect(find.text('Tesla Roadster'), findsAtLeast(1)); + expect(find.text(mockRoadster.name), findsAtLeast(1));
104-108: Target the title’s AnimatedOpacity specifically.Selecting the first AnimatedOpacity is fragile; scope it to the title.
Apply:
- final animatedOpacity = tester.widget<AnimatedOpacity>( - find.byType(AnimatedOpacity), - ); - expect(animatedOpacity.opacity, equals(0.0)); + final titleOpacity = tester.widget<AnimatedOpacity>( + find.ancestor( + of: find.text(mockRoadster.name), + matching: find.byType(AnimatedOpacity), + ), + ); + expect(titleOpacity.opacity, equals(0.0));
110-135: Test name vs. assertion mismatch.This test doesn’t verify auto-scroll; it only checks widget presence. Either rename or assert auto-advance (e.g., pump a duration and verify indicator/page changes).
Option A (rename):
- testWidgets('should start auto scroll for multiple images', + testWidgets('renders carousel and indicators for multiple images',Option B (verify behavior, sketch):
// After initial pump: final initialIndex = _activeIndicatorIndex(tester); await tester.pump(const Duration(seconds: 3)); // > auto-scroll interval final advancedIndex = _activeIndicatorIndex(tester); expect(advancedIndex, isNot(initialIndex));If you want, I can help wire a reliable _activeIndicatorIndex finder based on keys/semantics.
33-51: DRY the MaterialApp + Scaffold boilerplate.A small helper reduces duplication and noise across tests.
Add near the top of the file:
Widget _app(Widget sliver) => MaterialApp( supportedLocales: appSupportedLocales, localizationsDelegates: appLocalizationsDelegates, home: Scaffold( body: CustomScrollView(slivers: [sliver]), ), );Then replace repeated pumpWidget calls with:
await mockNetworkImagesFor(() => tester.pumpWidget(_app( RoadsterAppBar( roadster: mockRoadster, images: mockImages, scrollOffset: 0.0, fadeController: fadeController, ), )));Also applies to: 57-76, 83-102, 113-131, 140-158
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/features/roadster/widget/app_bar/image_carousel.dart(1 hunks)test/features/roadster/roadster_detail_screen.dart(1 hunks)test/features/roadster/widget/app_bar/image_carousel_test.dart(1 hunks)test/features/roadster/widget/app_bar/roadster_app_bar_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/roadster/widget/app_bar/image_carousel.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Flutter Integration Tests (31)
- GitHub Check: App
🔇 Additional comments (3)
test/features/roadster/roadster_detail_screen.dart (1)
42-51: LGTM: verifies key sections renderGood coverage on background, app bar/content, and FAB presence. This ensures composition doesn't regress.
test/features/roadster/widget/app_bar/image_carousel_test.dart (1)
7-22: Solid test scaffolding (group/setup/teardown).Good use of mockNetworkImagesFor, localization delegates, and controller lifecycle.
test/features/roadster/widget/app_bar/roadster_app_bar_test.dart (1)
31-54: Good coverage for SliverAppBar render path.Layout pumps under network image mocking and localization look correct.
| late RoadsterResource mockRoadster; | ||
| late List<String> mockImages; | ||
|
|
||
| setUpAll(() => HttpOverrides.global = null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid leaking global HttpOverrides across the suite
You nullify HttpOverrides.global but never restore it, which can break other tests. Capture and restore in tearDownAll.
void main() {
- late RoadsterResource mockRoadster;
- late List<String> mockImages;
+ late RoadsterResource mockRoadster;
+ late List<String> mockImages;
+ HttpOverrides? _prevHttpOverrides;
- setUpAll(() => HttpOverrides.global = null);
+ setUpAll(() {
+ _prevHttpOverrides = HttpOverrides.global;
+ HttpOverrides.global = null;
+ });
+ tearDownAll(() {
+ HttpOverrides.global = _prevHttpOverrides;
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setUpAll(() => HttpOverrides.global = null); | |
| void main() { | |
| late RoadsterResource mockRoadster; | |
| late List<String> mockImages; | |
| HttpOverrides? _prevHttpOverrides; | |
| setUpAll(() { | |
| _prevHttpOverrides = HttpOverrides.global; | |
| HttpOverrides.global = null; | |
| }); | |
| tearDownAll(() { | |
| HttpOverrides.global = _prevHttpOverrides; | |
| }); | |
| // ... rest of tests ... | |
| } |
🤖 Prompt for AI Agents
In test/features/roadster/roadster_detail_screen.dart around line 21, the test
suite sets HttpOverrides.global = null in setUpAll without preserving the
original value; capture the current HttpOverrides.global into a local variable
in setUpAll before nullifying it and add a tearDownAll that restores
HttpOverrides.global to that captured value so the global override is not leaked
to other tests.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
63-86: Replace coverage exclude path: lib/routers/router.dart → lib/routes/router.dartReplace occurrences in:
- .github/workflows/ci.yml — lines ~70 and ~85
- codecov.yml — line 1
🧹 Nitpick comments (3)
Makefile (1)
66-68: Switch Makefile screenshot_test target fromflutter drivetoflutter testIntegrationTestWidgetsFlutterBinding is present in screenshot_test/settings_screenshot_test.dart (lines ~8–11); replace the Makefile invocation.
- flutter drive --driver=test_driver/integration_test.dart --target=screenshot_test/settings_screenshot_test.dart --flavor dev + flutter test --flavor dev screenshot_test/settings_screenshot_test.dart
- Add device pinning to avoid CI randomness: append
-d $(DEVICE_ID)and set DEVICE_ID in CI.- Add
screenshot_testto.PHONY.- Remove
test_driver/integration_test.dartand any driver-shim CI steps if no longer needed..github/workflows/ci.yml (2)
120-129: Pin the integration test device to the emulator to avoid selecting web/desktop by defaultWithout
-d <device-id>, Flutter may pick Chrome/Linux on CI, causing flaky or failing runs. Select the started emulator explicitly.Apply this diff to deterministically target the AVD:
script: | flutter pub get - flutter test integration_test --flavor dev + device_id="$(adb devices | awk 'NR==2 && $2==\"device\" {print $1}')" + echo "Using device: ${device_id}" + flutter test integration_test --flavor dev -d "${device_id}"
129-129: Add a trailing newline at EOFFixes YAML lint warning: “no new line character at the end of file”.
- flutter test integration_test --flavor dev + flutter test integration_test --flavor dev +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)Makefile(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 129-129: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: App
- GitHub Check: Flutter Integration Tests (31)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
lib/features/roadster/widget/launch_section_widget.dart (1)
31-33: Fix:Color.withValuesdoesn’t exist — usewithOpacity.This won’t compile. Replace with
withOpacity(...)to achieve the intended translucency.- scheme.primary.withValues(alpha: 0.2), - scheme.secondary.withValues(alpha: 0.1), + scheme.primary.withOpacity(0.2), + scheme.secondary.withOpacity(0.1),lib/features/roadster/widget/details_card_widget.dart (1)
91-105: Theme-based body text colors look good; previous contrast issue resolvedNice switch to onSurface/onSurfaceVariant.
test/features/roadster/performance/widget_performance_test.dart (2)
21-36: Avoid wall‑clock timing; drive animation deterministically and assert no errorsReplace Stopwatch with deterministic pumps to prevent flakes. This mirrors prior guidance.
- // Measure rendering performance - final stopwatch = Stopwatch()..start(); - // Run animation for multiple cycles for (var i = 0; i < 10; i++) { unawaited(controller.forward()); await tester.pump(const Duration(milliseconds: 100)); unawaited(controller.reverse()); await tester.pump(const Duration(milliseconds: 100)); } - - stopwatch.stop(); - - // Verify acceptable performance - expect(stopwatch.elapsedMilliseconds, lessThan(2000)); // 2 seconds max + // Smoke assert: no exceptions during cycles + expect(tester.takeException(), isNull);
43-55: Replace render-time stopwatch with a smoke assertionRemove Stopwatch; assert build succeeds. Matches earlier recommendation.
- final stopwatch = Stopwatch()..start(); - await tester.pumpWidget( MaterialApp( home: AnimatedStarsField(pulseController: controller), ), ); - - stopwatch.stop(); - - // 50 stars should render quickly - expect(stopwatch.elapsedMilliseconds, lessThan(500)); + await tester.pump(); // first frame + expect(tester.takeException(), isNull);lib/features/roadster/widget/animated_counter_widget.dart (1)
41-60: LGTM: animates on value/duration updates (matches prior feedback).Smoothly retargets from the current value; good job closing the gap.
🧹 Nitpick comments (17)
lib/features/roadster/widget/launch_section_widget.dart (3)
55-97: Guard against overflow in narrow layouts.Long vehicle names can overflow with
spaceBetween. UseExpandedand ellipsis.- Row( - mainAxisAlignment: MainAxisAlignment.spaceBetween, - children: [ - Column( + Row( + children: [ + Expanded( + child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ Text( l10n.launchMass, style: Theme.of(context).textTheme.bodySmall?.copyWith( color: scheme.onSurfaceVariant, ), ), const SizedBox(height: 4), Text( massKg, - style: Theme.of(context).textTheme.bodyLarge?.copyWith( + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: Theme.of(context).textTheme.bodyLarge?.copyWith( color: scheme.primary, fontWeight: FontWeight.bold, ), ), ], - ], - ), - Column( + ], + ), + const SizedBox(width: 12), + Expanded( + child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ Text( l10n.launchVehicle, style: Theme.of(context).textTheme.bodySmall?.copyWith( color: scheme.onSurfaceVariant, ), ), const SizedBox(height: 4), Text( vehicle, - style: Theme.of(context).textTheme.bodyLarge?.copyWith( + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: Theme.of(context).textTheme.bodyLarge?.copyWith( color: scheme.primary, fontWeight: FontWeight.bold, ), ), ], - ], - ), + ], + ), ], ),
20-25: Clip Card to its rounded shape.Prevents any child decoration bleed and keeps visuals crisp.
return Card( elevation: 6, shape: RoundedRectangleBorder( borderRadius: BorderRadius.circular(16), ), + clipBehavior: Clip.antiAlias, child: Container(
41-44: A11y: mark the decorative icon as non-semantic.Avoids screen readers announcing an unlabeled “rocket” icon; the header text already conveys meaning.
- Icon( - Icons.rocket, - color: scheme.primary, - ), + ExcludeSemantics( + child: Icon( + Icons.rocket, + color: scheme.primary, + ), + ),lib/features/roadster/widget/details_card_widget.dart (6)
1-1: Use RotationTransition (+ TickerMode) to avoid rebuilding Icon each frame; drop now‑unused math importSimpler, more efficient, and honors reduced‑motion.
-import 'dart:math' as math; +// import removed if RotationTransition is used @@ - AnimatedBuilder( - animation: _rotationController, - builder: (context, child) { - return Transform.rotate( - angle: _rotationController.value * 2 * math.pi, - child: Icon( - Icons.satellite_alt, - color: Theme.of(context).colorScheme.primary, - size: 32, - ), - ); - }, - ), + TickerMode( + enabled: !(MediaQuery.maybeOf(context)?.disableAnimations ?? false), + child: RotationTransition( + turns: _rotationController, + child: Icon( + Icons.satellite_alt, + color: Theme.of(context).colorScheme.primary, + size: 32, + ), + ), + ),Also applies to: 68-80
23-26: Prefer SingleTickerProviderStateMixinYou use one controller; this is the idiomatic mixin.
-class _DetailsCardWidgetState extends State<DetailsCardWidget> - with TickerProviderStateMixin { +class _DetailsCardWidgetState extends State<DetailsCardWidget> + with SingleTickerProviderStateMixin {
45-50: Clip Card to rounded shapePrevents child gradient bleed on corners.
return Card( elevation: 8, + clipBehavior: Clip.antiAlias, shape: RoundedRectangleBorder( borderRadius: BorderRadius.circular(20), ),
58-61: Use withOpacity for wider SDK compatibility (vs withValues)Safer across Flutter versions; visually identical here.
- Theme.of(context).colorScheme.surface.withValues(alpha: 0.8), + Theme.of(context).colorScheme.surface.withOpacity(0.8),
107-117: Chip text/icon contrast may be insufficient when overriding backgroundColorBind label/icon color to the matching “on*Container” token; add runSpacing.
- Wrap( - spacing: 8, - children: [ - ...widget.missions.map((mission) => Chip( - avatar: const Icon(Icons.tag, size: 18), - label: Text(mission.name), - backgroundColor: mission.isPrimary - ? Theme.of(context).colorScheme.primaryContainer - : Theme.of(context).colorScheme.secondaryContainer, - )) - ], - ), + Wrap( + spacing: 8, + runSpacing: 8, + children: [ + ...widget.missions.map((mission) { + final cs = Theme.of(context).colorScheme; + final bg = mission.isPrimary ? cs.primaryContainer : cs.secondaryContainer; + final fg = mission.isPrimary ? cs.onPrimaryContainer : cs.onSecondaryContainer; + return Chip( + avatar: Icon(Icons.tag, size: 18, color: fg), + label: Text( + mission.name, + style: Theme.of(context).textTheme.labelLarge?.copyWith(color: fg), + ), + backgroundColor: bg, + ); + }), + ], + ),
54-61: Optional: make the gradient more perceptibleCurrent start/end are both surface; consider surfaceContainerHigh -> surface or primaryContainer.withOpacity(0.08) -> surface for subtle depth.
lib/features/roadster/widget/background/animated_gradient_background.dart (1)
16-26: Use const palettes to avoid per-build allocationsChange local lists to const so they’re canonicalized and not rebuilt every frame.
- final darkColors = [ + const darkColors = [ Color(0xFF0D0E1C), Color(0xFF1A1B3A), Color(0xFF2D1B3D), ]; - final lightColors = [ + const lightColors = [ Color(0xFFF0F4FF), Color(0xFFE8ECFF), Color(0xFFDDE3FF), ];test/features/roadster/performance/widget_performance_test.dart (1)
41-49: Ensure controller disposal even on test failureUse addTearDown to guarantee cleanup.
- final controller = createTestAnimationController(); + final controller = createTestAnimationController(); + addTearDown(controller.dispose);lib/features/roadster/roadster_screen.dart (3)
41-44: Make constant constructors const to reduce rebuild churnMinor micro-optimization.
- return Scaffold( - appBar: AppBar(), + return Scaffold( + appBar: const AppBar(), body: const LoadingContent(), ); @@ - return Scaffold( - appBar: AppBar(), + return Scaffold( + appBar: const AppBar(), body: ErrorContent( @@ - return EmptyWidget(); + return const EmptyWidget();Also applies to: 50-53, 62-62
153-155: Isolate background layers with RepaintBoundaryHelps avoid unnecessary repaints when foreground content updates.
- AnimatedGradientBackground(pulseController: _pulseController), - AnimatedStarsField(pulseController: _pulseController), + RepaintBoundary( + child: AnimatedGradientBackground(pulseController: _pulseController), + ), + RepaintBoundary( + child: AnimatedStarsField(pulseController: _pulseController), + ),
120-126: Throttle scroll setState to reduce rebuild frequencyConsider NotificationListener or throttling to avoid rebuilding the whole screen every pixel.
If interested, I can propose a small refactor using NotificationListener with a ValueNotifier for scrollOffset.
lib/features/roadster/widget/animated_counter_widget.dart (3)
4-9: Add fast‑fail asserts for invalid inputs (decimals, value, duration).Prevents silent misconfigurations and makes errors actionable in debug builds.
const AnimatedCounterWidget({ super.key, required this.value, required this.duration, this.decimals = 0, - }); + }) : assert(decimals >= 0, 'decimals must be >= 0'), + assert(value.isFinite, 'value must be finite'), + assert(!duration.isNegative, 'duration must be >= 0');
56-58: Nit: prefer forward(from: 0) over reset+forward.Same effect with fewer lifecycle transitions.
- _controller - ..reset() - ..forward(); + _controller.forward(from: 0);
73-78: Use locale-aware NumberFormat; keep integer truncation when decimals == 0.File: lib/features/roadster/widget/animated_counter_widget.dart (around lines 73–78)
pubspec.yaml already contains intl: ^0.20.2. Replace the manual regex with Intl and preserve the current truncation behavior (toInt) for decimals == 0:
import 'package:intl/intl.dart';
final locale = Localizations.localeOf(context).toString();
final fmt = NumberFormat.decimalPattern(locale)
..minimumFractionDigits = widget.decimals
..maximumFractionDigits = widget.decimals;
final num valueForDisplay =
widget.decimals == 0 ? _animation.value.toInt() : _animation.value;
final text = fmt.format(valueForDisplay);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
lib/features/roadster/roadster_screen.dart(1 hunks)lib/features/roadster/widget/animated_counter_widget.dart(1 hunks)lib/features/roadster/widget/animated_stat_card_widget.dart(1 hunks)lib/features/roadster/widget/app_bar/image_carousel.dart(1 hunks)lib/features/roadster/widget/app_bar/roadster_app_bar.dart(1 hunks)lib/features/roadster/widget/background/animated_gradient_background.dart(1 hunks)lib/features/roadster/widget/buttons/track_live_button.dart(1 hunks)lib/features/roadster/widget/details_card_widget.dart(1 hunks)lib/features/roadster/widget/distance_card_widget.dart(1 hunks)lib/features/roadster/widget/launch_section_widget.dart(1 hunks)lib/features/roadster/widget/orbital_parameters_section.dart(1 hunks)lib/features/roadster/widget/orbital_section_widget.dart(1 hunks)test/features/roadster/performance/widget_performance_test.dart(1 hunks)test/features/roadster/widget/animated_star_widget_test.dart(1 hunks)test/features/roadster/widget/app_bar/image_carousel_test.dart(1 hunks)test/features/roadster/widget/distance_card_widget_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- lib/features/roadster/widget/orbital_parameters_section.dart
- lib/features/roadster/widget/app_bar/roadster_app_bar.dart
- lib/features/roadster/widget/buttons/track_live_button.dart
- test/features/roadster/widget/app_bar/image_carousel_test.dart
- test/features/roadster/widget/animated_star_widget_test.dart
- lib/features/roadster/widget/app_bar/image_carousel.dart
- lib/features/roadster/widget/orbital_section_widget.dart
- lib/features/roadster/widget/animated_stat_card_widget.dart
- lib/features/roadster/widget/distance_card_widget.dart
- test/features/roadster/widget/distance_card_widget_test.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Flutter Integration Tests (31)
- GitHub Check: App
🔇 Additional comments (2)
lib/features/roadster/widget/background/animated_gradient_background.dart (1)
30-46: LGTM: clean, theme-aware animated gradientGood use of AnimatedBuilder and Color.lerp with a shared controller.
lib/features/roadster/widget/animated_counter_widget.dart (1)
24-39: LGTM: clean animation initialization.Controller/tween/curve wiring is correct and starts as expected.
| late AnimationController _pulseController; | ||
| late AnimationController _rotationController; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Remove unused _rotationController; it repeats forever with no consumers
It’s never read or passed, yet repeats, burning a ticker every frame. Drop it or wire it into the UI.
late AnimationController _slideController;
late AnimationController _pulseController;
- late AnimationController _rotationController;
@@
- _rotationController = AnimationController(
- duration: const Duration(seconds: 20),
- vsync: this,
- )..repeat();
@@
- _rotationController.dispose();Run to confirm no other references remain:
Also applies to: 114-118, 133-133
🏁 Script executed:
#!/bin/bash
rg -n -C2 --type=dart '_rotationController'Length of output: 3228
Remove unused _rotationController from lib/features/roadster/roadster_screen.dart
It's created and ..repeat()'d but never read — remove the declaration (line 87), its init (lines 114–116) and the dispose call (line 133). Note: lib/features/roadster/widget/details_card_widget.dart has its own _rotationController in use; don't remove that one.
late AnimationController _slideController;
late AnimationController _pulseController;
- late AnimationController _rotationController;
@@
- _rotationController = AnimationController(
- duration: const Duration(seconds: 20),
- vsync: this,
- )..repeat();
@@
- _rotationController.dispose();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| late AnimationController _pulseController; | |
| late AnimationController _rotationController; | |
| late AnimationController _pulseController; |
🤖 Prompt for AI Agents
In lib/features/roadster/roadster_screen.dart around lines 86–88 (and init at
114–116 and dispose at 133), remove the unused _rotationController: delete its
field declaration, the AnimationController creation and .repeat() in initState,
and the corresponding _rotationController.dispose() in dispose(); keep the
remaining _pulseController logic intact and do not modify
lib/features/roadster/widget/details_card_widget.dart which has its own rotation
controller.
1 participant
Summary by CodeRabbit
New Features
UI / Visual Improvements
Localization