[FIX] Migrate deprecated Flutter APIs (PopScope, TextButton, Theme/TextTheme updates)#199
Conversation
- Replace FlatButton with TextButton in bottom_sheet_setup.dart and confirm_bottom_sheet_view.dart (removed in Flutter 3.x) - Remove deprecated toggleableActiveColor from app_theme.dart - Replace deprecated 'primary:' with 'backgroundColor:' across multiple files - Replace deprecated textTheme.subtitle1 with titleMedium - Downgrade stacked_generator from ^0.7.13 to ^0.7.8 to resolve test_api version conflict with Flutter 3.x SDK Fixes CCExtractor#185, closes CCExtractor#177
📝 WalkthroughWalkthroughRemoved deprecated APIs and modernized widgets: dropped Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/views/IRSSI/IRSSI_view.dart (1)
255-283:⚠️ Potential issue | 🟡 MinorReplace deprecated
colorScheme.backgroundwithcolorScheme.surface.Line 282 uses
Theme.of(context).colorScheme.background, which is deprecated in Flutter's Material 3. Replace it withcolorScheme.surfaceas recommended in the Flutter breaking change documentation.Suggested patch
- : Theme.of(context) - .colorScheme - .background, + : Theme.of(context) + .colorScheme + .surface,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/views/IRSSI/IRSSI_view.dart` around lines 255 - 283, In the Container decoration inside the IRSSI view build (the block creating subtitle1 and returning the Container), replace Theme.of(context).colorScheme.background with Theme.of(context).colorScheme.surface to avoid the deprecated background role; update any similar uses in that widget (e.g., where the Container's color is set) to use colorScheme.surface so the UI follows Material 3 color roles (keep the rest of the ternary for dark mode unchanged).
🧹 Nitpick comments (1)
test/helpers/test_helpers.dart (1)
69-75: Consider adding stubs or documenting expected stubbing pattern.
MockHistoryServiceis registered without any stubs. WithreturnNullOnMissingStubremoved, any test calling methods on this mock without explicit stubs will throwMissingStubError.This is fine if individual tests set up their own stubs, but inconsistent with
getAndRegisterAuthenticationService(line 81) andgetAndRegisterHttpIOClientMock(line 63) which pre-configure stubs.Consider either:
- Adding common stubs here for frequently-used methods, or
- Documenting that tests must stub methods before use
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/helpers/test_helpers.dart` around lines 69 - 75, getAndRegisterHistoryService currently registers MockHistoryService without stubbing, so tests will throw MissingStubError when methods are invoked; update getAndRegisterHistoryService to either preconfigure common stubs on the mock (e.g., set up typical method returns/behaviors used across tests) before calling locator.registerSingleton<HistoryService>(service), or add a clear docstring/comment above getAndRegisterHistoryService (similar to getAndRegisterAuthenticationService/getAndRegisterHttpIOClientMock) stating that callers must explicitly stub the mock before use; reference MockHistoryService and HistoryService in the function body and mirror the stubbing pattern used by getAndRegisterAuthenticationService and getAndRegisterHttpIOClientMock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lib/ui/widgets/smart_widgets/bottom_sheets/sort_bottom_sheet/sort_bottom_sheet_view.dart`:
- Around line 63-101: The code uses a non-existent RadioGroup widget and leaves
Radio widgets missing required parameters; replace the RadioGroup wrapper by
building each option with Flutter's standard Radio (or RadioListTile) pattern:
inside the ListView.builder iterate over Sort.values, call model.getSortMap()
once before the builder, and for each index use Radio(value: Sort.values[index],
groupValue: model.sortPreference, onChanged: (Sort? s) { if (s!=null) {
model.setSortPreference(completer, s); Navigator.of(context).pop(); } }), or use
RadioListTile with the same groupValue/onChanged and title from
sortMap[Sort.values[index]]; keep model.setSortPreference and
Navigator.of(context).pop() calls as shown and ensure you supply non-null labels
from getSortMap() when building items.
---
Outside diff comments:
In `@lib/ui/views/IRSSI/IRSSI_view.dart`:
- Around line 255-283: In the Container decoration inside the IRSSI view build
(the block creating subtitle1 and returning the Container), replace
Theme.of(context).colorScheme.background with
Theme.of(context).colorScheme.surface to avoid the deprecated background role;
update any similar uses in that widget (e.g., where the Container's color is
set) to use colorScheme.surface so the UI follows Material 3 color roles (keep
the rest of the ternary for dark mode unchanged).
---
Nitpick comments:
In `@test/helpers/test_helpers.dart`:
- Around line 69-75: getAndRegisterHistoryService currently registers
MockHistoryService without stubbing, so tests will throw MissingStubError when
methods are invoked; update getAndRegisterHistoryService to either preconfigure
common stubs on the mock (e.g., set up typical method returns/behaviors used
across tests) before calling locator.registerSingleton<HistoryService>(service),
or add a clear docstring/comment above getAndRegisterHistoryService (similar to
getAndRegisterAuthenticationService/getAndRegisterHttpIOClientMock) stating that
callers must explicitly stub the mock before use; reference MockHistoryService
and HistoryService in the function body and mirror the stubbing pattern used by
getAndRegisterAuthenticationService and getAndRegisterHttpIOClientMock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a1aa165-a012-41eb-a927-ea733979f0b0
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
lib/theme/app_theme.dartlib/ui/shared/shared_styles.dartlib/ui/views/Download/download_view.dartlib/ui/views/IRSSI/IRSSI_view.dartlib/ui/views/disk_explorer/disk_explorer_view.dartlib/ui/views/login/login_view.dartlib/ui/widgets/dumb_widgets/document_type_card.dartlib/ui/widgets/dumb_widgets/password_change_dialog_widget.dartlib/ui/widgets/dumb_widgets/torrent_label_dialog.dartlib/ui/widgets/smart_widgets/bottom_sheets/bottom_sheet_setup.dartlib/ui/widgets/smart_widgets/bottom_sheets/confirm_bottom_sheet/confirm_bottom_sheet_view.dartlib/ui/widgets/smart_widgets/bottom_sheets/sort_bottom_sheet/sort_bottom_sheet_view.dartlib/ui/widgets/smart_widgets/url_bottom_sheet/url_bottomsheet_view.dartpubspec.yamltest/helpers/test_helpers.dart
💤 Files with no reviewable changes (1)
- lib/theme/app_theme.dart
lib/ui/widgets/smart_widgets/bottom_sheets/sort_bottom_sheet/sort_bottom_sheet_view.dart
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ui/views/media_player/media_stream_viewmodel.dart`:
- Around line 86-87: The init() flow constructs _videoPlayerController with
VideoPlayerController.networkUrl(Uri.parse(_mediaUrl)) which can throw
FormatException; update init() in MediaStreamViewModel to validate or safely
parse the URL before passing it in (use Uri.tryParse and verify scheme/host or
wrap Uri.parse in a try/catch for FormatException), and if invalid, handle it
the same way PlatformException is handled (log via processLogger or state/error
field and abort controller creation) so malformed URLs do not crash the stream
flow; reference the _videoPlayerController assignment and the init() method when
making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24f5c211-d5b2-4406-9bb7-74e1f5f3eb1c
📒 Files selected for processing (3)
lib/ui/views/media_player/media_stream_viewmodel.dartlib/ui/widgets/smart_widgets/bottom_sheets/sort_bottom_sheet/sort_bottom_sheet_view.darttest/helpers/test_helpers.mocks.dart
✅ Files skipped from review due to trivial changes (1)
- test/helpers/test_helpers.mocks.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ui/widgets/smart_widgets/bottom_sheets/sort_bottom_sheet/sort_bottom_sheet_view.dart
Summary
This PR updates deprecated Flutter framework APIs to keep the project compatible with newer Flutter/Dart SDK versions and eliminate analyzer deprecation warnings.
What changed
FlatButton→TextButtonWillPopScope→PopScopeElevatedButton.styleFrom(primary: ...)→backgroundColor: ...textTheme.subtitle1→textTheme.titleMediumtextTheme.button→textTheme.labelLargetoggleableActiveColorfromapp_theme.dartcolorScheme.backgroundwithcolorScheme.surfacewhere applicablewithOpacity(...)→withValues(alpha: ...)SheetResponse(responseData: ...)→SheetResponse(data: ...)stacked_generatorversion and refreshedpubspec.lockMockSpec<T>()usageTesting
flutter analyze(no issues found)Scope
Fixes #185
Closes #177
Summary by CodeRabbit
Refactor
Chores