chore(flutter): modernize deprecations, async guards, and list assertions - #8984
Open
YadneshTeli wants to merge 4 commits into
Open
YadneshTeli wants to merge 4 commits into
YadneshTeli wants to merge 4 commits into
Conversation
Contributor
Reviewer's GuideModernizes deprecated Flutter APIs across the workspace, updates reorder callbacks and destination-index handling, adds mounted and unique-ID safety checks, and broadens dependency constraints with coordinated overrides. Sequence diagram for safe asynchronous file upload feedbacksequenceDiagram
participant Caller
participant insertLocalFile
participant BuildContext
participant showSnackBarMessage
Caller->>insertLocalFile: insertLocalFile(...)
insertLocalFile->>BuildContext: mounted
alt context is unmounted
insertLocalFile-->>Caller: return
else context is mounted
insertLocalFile->>showSnackBarMessage: showSnackBarMessage(context, errorMsg)
showSnackBarMessage-->>Caller: display upload error
end
Flow diagram for reorder destination handlingflowchart LR
UI[ReorderableListView.onReorderItem] -->|fromIndex, toIndex| Bloc[Reorder BLoC]
Bloc --> IDs[Resolve source and destination IDs]
IDs --> Move[Apply reorder without manual downward offset]
Move --> State[Update reordered state]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list.dart" line_range="87-91" />
<code_context>
super.initState();
- // TODO: Add assert for messages having same id
+ assert(
+ chatController.messages.map((m) => m.id).toSet().length ==
+ chatController.messages.length,
+ 'Messages must have unique IDs.',
+ );
oldList = List.from(chatController.messages);
operationsSubscription = chatController.operationsStream.listen((event) {
</code_context>
<issue_to_address>
**issue (broader_impact):** The unique-ID check runs only during `initState` and Dart `assert` statements are removed in release builds, so duplicate message IDs introduced by later chat operations—or present in production—still reach `AnimatedList` and cause key collisions.
**Triggers:** When messages are updated after initialization or the application runs in a release build.
**Suggested fix:** Validate IDs whenever the controller message list changes and use an active runtime guard or collision-safe key strategy if duplicates must be prevented in production.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…se collision-safe keys
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR addresses several high-value TODOs, modernizes deprecated Flutter APIs to current stable patterns, and adds async context safety guards across the workspace:
Flutter Modernization (
onReorder->onReorderItem):ReorderableListView.onReordercallbacks with modernonReorderItemacross 12 UI files (grid_page.dart,mobile_grid_page.dart,checklist_cell_editor.dart,mobile_checklist_cell_editor.dart,desktop_row_detail_checklist_cell.dart,media_cell_editor.dart,select_option_cell_editor.dart,select_option.dart,row_property.dart,setting_property_list.dart,database_field_list.dart,database_sort_bottom_sheet.dart,favorite_folder.dart,mobile_hidden_groups_column.dart,board_hidden_groups.dart,mobile_grid_header.dart,sort_editor.dart).if (fromIndex < toIndex) toIndex--;) in corresponding BLoCs (SortEditorBloc,ChecklistCellBloc,PropertyBloc,RowDetailBloc,FavoriteBloc), eliminating indexing bugs when moving items downwards.Modernized Theme & Widget Deprecations:
desktop_appearance.dart&mobile_appearance.dart: Replaced deprecateddialogBackgroundColorandindicatorColorwithdialogTheme: DialogThemeData(...)andtabBarTheme: TabBarThemeData(...).notifications_setting_group.dart: Replaced deprecatedactiveColorwithactiveTrackColor: theme.colorScheme.primary.interactive_image_viewer.dart: Migrated deprecatedMatrix4.translate/Matrix4.scaletotranslateByDouble/scaleByDouble.Async Context Guards & Invariant Assertions:
file_util.dart: Addedif (!context.mounted) return;andif (context.mounted)safety guards before asynchronousshowSnackBarMessagecalls after file uploads.animated_chat_list.dart&animated_chat_list_reversed.dart: Added runtime assertions ininitStateensuring message IDs are unique, preventingAnimatedListkey collision errors.Type of Change
chore)Verification
flutter analyzeensuring 0 compilation errors.Summary by Sourcery
Modernize deprecated Flutter APIs and strengthen list reordering, asynchronous context safety, and chat-list invariants across the workspace.
Bug Fixes:
Enhancements:
Build:
Chores: