Fix analysis errors#1176
Conversation
📝 WalkthroughWalkthroughThis PR updates Flutter list view widget APIs across six files. The Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/view/page/setting/entries/home_tabs.dart`:
- Line 51: The _onReorder handler is still applying the legacy "if oldIndex <
newIndex then newIndex -= 1" adjustment even though onReorderItem supplies an
already-corrected newIndex; remove that decrement logic from the _onReorder
implementation (referencing _onReorder and the onReorderItem callback
assignment) and simply use the provided newIndex (optionally clamp it to valid
range) when moving the item so downward drags are not mispositioned.
In `@lib/view/page/setting/seq/srv_seq.dart`:
- Line 57: The onReorderItem handler currently applies an extra manual index
adjustment (the if (targetIndex > oldIndex) { targetIndex -= 1; } logic) which
causes off-by-one moves; remove that manual correction so the handler uses the
provided target index directly (i.e., stop mutating targetIndex based on
oldIndex) within the onReorderItem callback to ensure items move to the intended
position.
In `@lib/view/page/setting/seq/virt_key.dart`:
- Line 78: The _handleReorder function is double-normalizing indices: when used
as the callback for onReorderItem it should not manually adjust targetIndex;
remove the conditional block that does "if (targetIndex > oldIndex) targetIndex
-= 1" (or any equivalent decrement) inside _handleReorder so the method uses the
indices provided by onReorderItem directly and reorders the backing list
accordingly (keep references to _handleReorder and onReorderItem to locate the
code).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 26f36268-e608-43f6-a869-79df8a4ed568
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
lib/view/page/server/connection_stats.dartlib/view/page/setting/entries/home_tabs.dartlib/view/page/setting/seq/srv_detail_seq.dartlib/view/page/setting/seq/srv_func_seq.dartlib/view/page/setting/seq/srv_seq.dartlib/view/page/setting/seq/virt_key.dart
| child: ReorderableListView.builder( | ||
| itemCount: _selectedTabs.length, | ||
| onReorder: _onReorder, | ||
| onReorderItem: _onReorder, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Flutter ReorderableListView.builder, does onReorderItem already provide a corrected newIndex (without needing oldIndex < newIndex => newIndex -= 1)?
💡 Result:
No. In Flutter’s ReorderableListView.builder, the older callback onReorder passes a newIndex that still requires the “if oldIndex < newIndex then newIndex -= 1” correction. Flutter deprecated onReorder specifically because it needed that manual adjustment, and introduced onReorderItem to automatically adjust newIndex for you. Evidence: Flutter’s release notes state that the onReorder callback required a manual correction for newIndex when oldIndex is before newIndex, and that onReorderItem “solves this problem by doing the correction automatically” [1]. The ReorderableListView.builder docs also note that onReorderItem adjusts the newIndex parameter and that onReorder is deprecated in favor of it [2]. So: - Using onReorder: you generally need the adjustment (e.g., if oldIndex < newIndex then newIndex -= 1) [1]. - Using onReorderItem: you do not need that adjustment; newIndex is already corrected [1][2].
Citations:
- 1: https://docs.flutter.dev/release/breaking-changes/deprecate-onreorder-callback
- 2: https://api.flutter.dev/flutter/material/ReorderableListView/ReorderableListView.builder.html
Remove legacy newIndex correction when using onReorderItem.
onReorderItem already provides an adjusted newIndex, and Flutter deprecated onReorder specifically because it required the manual “if oldIndex < newIndex then newIndex -= 1” correction. Keeping the decrement in _onReorder will misplace items while dragging downward.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/view/page/setting/entries/home_tabs.dart` at line 51, The _onReorder
handler is still applying the legacy "if oldIndex < newIndex then newIndex -= 1"
adjustment even though onReorderItem supplies an already-corrected newIndex;
remove that decrement logic from the _onReorder implementation (referencing
_onReorder and the onReorderItem callback assignment) and simply use the
provided newIndex (optionally clamp it to valid range) when moving the item so
downward drags are not mispositioned.
| return ReorderableListView.builder( | ||
| footer: const SizedBox(height: 77), | ||
| onReorder: (oldIndex, newIndex) { | ||
| onReorderItem: (oldIndex, newIndex) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
After migrating ReorderableListView from onReorder to onReorderItem, is old logic if (targetIndex > oldIndex) targetIndex -= 1 still valid?
💡 Result:
No, the logic if (targetIndex > oldIndex) targetIndex -= 1 is no longer valid and should be removed when migrating to onReorderItem [1][2]. The onReorder callback was deprecated precisely because it provided raw indices that required manual correction to account for the list's length shrinking after an item was removed [1][2]. The new onReorderItem callback handles this adjustment automatically, providing the expected newIndex directly [2]. If you keep the old adjustment logic after switching to onReorderItem, your reordering will behave incorrectly, likely causing items to be inserted at the wrong positions [2]. Migration Summary: - Old Callback (onReorder): Required manual if (newIndex > oldIndex) newIndex -= 1;. - New Callback (onReorderItem): Requires no index adjustment [2]. Example of the migration: // Before onReorder: (int oldIndex, int newIndex) { if (oldIndex < newIndex) { newIndex -= 1; } // update list... } // After onReorderItem: (int oldIndex, int newIndex) { // update list directly using newIndex... } [2]
Citations:
- 1: https://master-api.flutter.dev/flutter/material/ReorderableListView/onReorder.html
- 2: https://docs.flutter.dev/release/breaking-changes/deprecate-onreorder-callback
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="lib/view/page/setting/seq/srv_seq.dart"
echo "== File =="
ls -la "$FILE"
echo
echo "== Relevant snippet (around onReorderItem) =="
rg -n "onReorderItem\s*:" "$FILE"
# Print a small window around the match
LINE=$(rg -n "onReorderItem\s*:" "$FILE" | head -n1 | cut -d: -f1)
START=$((LINE-10))
END=$((LINE+30))
sed -n "${START},${END}p" "$FILE" | nl -ba | sed 's/^/ /'
echo
echo "== Any targetIndex adjustment logic nearby =="
rg -n "targetIndex|newIndex|oldIndex" "$FILE" | head -n 50Repository: lollipopkit/flutter_server_box
Length of output: 329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="lib/view/page/setting/seq/srv_seq.dart"
echo "== Relevant region (lines 45-90) =="
sed -n '45,90p' "$FILE" | cat -n
echo
echo "== Search for targetIndex / adjustment logic =="
rg -n "targetIndex|newIndex\s*-=|if\s*\(.*newIndex.*>\s*.*oldIndex|oldIndex.*targetIndex" "$FILE" || trueRepository: lollipopkit/flutter_server_box
Length of output: 1959
Fix off-by-one index handling in onReorderItem
Remove the manual if (targetIndex > oldIndex) { targetIndex -= 1; } adjustment inside onReorderItem; onReorderItem already provides the correct target index, and keeping the old onReorder correction will misplace items (notably on downward moves).
Suggested fix
onReorderItem: (oldIndex, newIndex) {
- var targetIndex = newIndex;
- if (targetIndex > oldIndex) {
- targetIndex -= 1;
- }
+ final targetIndex = newIndex;
if (targetIndex == oldIndex) {
return;
}📝 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.
| onReorderItem: (oldIndex, newIndex) { | |
| onReorderItem: (oldIndex, newIndex) { | |
| final targetIndex = newIndex; | |
| if (targetIndex == oldIndex) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/view/page/setting/seq/srv_seq.dart` at line 57, The onReorderItem handler
currently applies an extra manual index adjustment (the if (targetIndex >
oldIndex) { targetIndex -= 1; } logic) which causes off-by-one moves; remove
that manual correction so the handler uses the provided target index directly
(i.e., stop mutating targetIndex based on oldIndex) within the onReorderItem
callback to ensure items move to the intended position.
| proxyDecorator: reorderProxyDecorator, | ||
| itemBuilder: (_, idx) => _buildListItem(_order[idx], idx), | ||
| onReorder: _handleReorder, | ||
| onReorderItem: _handleReorder, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Flutter's onReorderItem callback already normalize newIndex so handlers should not manually decrement it when newIndex > oldIndex?
💡 Result:
Yes—but only if you’re using Flutter’s newer onReorderItem callback (not the deprecated onReorder). When using the deprecated onReorder callback, you must account for the fact that when oldIndex < newIndex, removing the item at oldIndex shortens the list by 1; therefore implementations typically decrement newIndex by 1 before inserting. This is explicitly documented in ReorderCallback’s example and description: “If oldIndex is before newIndex… reducing the list’s length by one… account for this when inserting before newIndex,” with sample code that does if (oldIndex < newIndex) newIndex -= 1. [1] Flutter then introduced onReorderItem to “automatically handle index adjustments” so you should not apply that manual correction when items are moved downward. The Flutter breaking-changes documentation states: “The onReorder callback… required a manual correction for… newIndex… The new callback, onReorderItem, solves this problem by doing the correction automatically,” and shows migrating by removing the manual newIndex -= 1 logic. [2] So the rule is: - onReorderItem: don’t manually decrement newIndex. - onReorder: do manually decrement newIndex when oldIndex < newIndex (i.e., when moving downward). [1]
Citations:
- 1: When use ReorderableListView dragging item from up to down, "newIndex" is incorrect. flutter/flutter#56821
- 2: The index parameter for ReorderableListView's onReorderCallback is confusing. flutter/flutter#127901
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant area around _handleReorder in the specified file
FILE="lib/view/page/setting/seq/virt_key.dart"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
# Print line numbers around the snippet region (approx based on review comment)
nl -ba "$FILE" | sed -n '60,170p'Repository: lollipopkit/flutter_server_box
Length of output: 118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="lib/view/page/setting/seq/virt_key.dart"
awk '{printf "%d\t%s\n", NR, $0}' "$FILE" | sed -n '60,170p'Repository: lollipopkit/flutter_server_box
Length of output: 3090
Remove manual index decrement in _handleReorder when using onReorderItem in virt_key.dart.
_handleReorder still does if (targetIndex > oldIndex) targetIndex -= 1 (lines ~131-134) even though the list uses onReorderItem: _handleReorder, which already applies the needed index normalization—this will misplace reordered items.
Suggested fix
void _handleReorder(int oldIndex, int newIndex) {
- var targetIndex = newIndex;
- if (targetIndex > oldIndex) {
- targetIndex -= 1;
- }
+ final targetIndex = newIndex;
if (targetIndex == oldIndex) {
return;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/view/page/setting/seq/virt_key.dart` at line 78, The _handleReorder
function is double-normalizing indices: when used as the callback for
onReorderItem it should not manually adjust targetIndex; remove the conditional
block that does "if (targetIndex > oldIndex) targetIndex -= 1" (or any
equivalent decrement) inside _handleReorder so the method uses the indices
provided by onReorderItem directly and reorders the backing list accordingly
(keep references to _handleReorder and onReorderItem to locate the code).
1 participant
Summary by CodeRabbit