Conversation
WalkthroughAdds server order update logic to ServersNotifier and refactors the server sequence settings page to manage local order state and correctly handle drag-and-drop reordering. Also updates Riverpod-generated hash strings for several providers with no behavioral changes. Changes
Sequence Diagram(s)sequenceDiagram
actor U as User
participant P as Server Seq Page
participant R as ReorderableListView
participant N as ServersNotifier
participant S as Storage
participant SY as Sync/Scheduler
U->>R: Drag item to new position
R-->>P: onReorder(oldIndex, newIndex)
rect rgba(200,230,255,0.3)
note right of P: Compute adjusted targetIndex and newOrder
P->>P: setState(_order = newOrder)
P->>N: updateServerOrder(newOrder)
end
N->>N: _isSameOrder? (fast path)
alt Order changed
N->>S: Persist serverOrder
N-->>SY: Trigger synchronization
else No change
N-->>P: No-op
end
note over N,P: Page also listens to serversProvider changes<br/>and syncs local _order when external updates occur.
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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: 0
🧹 Nitpick comments (1)
lib/data/provider/server/all.dart (1)
271-284: Consider using collection equality from fl_lib.The manual list equality implementation is correct with the
identical()fast-path optimization. However, sincefl_libis already imported (line 3), you could use the.equals()extension method instead:bool _isSameOrder(List<String> a, List<String> b) => a.equals(b);The current implementation is fine and avoids any potential overhead, so this is purely a stylistic suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/data/provider/pve.g.dart(1 hunks)lib/data/provider/server/all.dart(1 hunks)lib/data/provider/server/all.g.dart(1 hunks)lib/data/provider/server/single.g.dart(1 hunks)lib/data/provider/systemd.g.dart(1 hunks)lib/view/page/setting/seq/srv_seq.dart(3 hunks)
🔇 Additional comments (10)
lib/data/provider/server/all.g.dart (1)
44-44: Generated file - no review needed.This is a Riverpod-generated hash update corresponding to changes in the source file
lib/data/provider/server/all.dart. Generated files are automatically updated by the build runner.lib/data/provider/pve.g.dart (1)
61-61: Generated file - no review needed.This is a Riverpod-generated hash update. Generated files are automatically updated by the build runner and don't require manual review.
lib/data/provider/systemd.g.dart (1)
61-61: Generated file - no review needed.This is a Riverpod-generated hash update. No manual review required.
lib/data/provider/server/single.g.dart (1)
61-61: Generated file - no review needed.This is a Riverpod-generated hash update. Generated files don't require manual review.
lib/data/provider/server/all.dart (1)
242-269: LGTM - well-implemented server order update logic.The method correctly:
- Deduplicates the input order and filters out invalid server IDs (lines 246-254)
- Ensures all existing servers are included by appending missing IDs (lines 256-260)
- Early exits if the order hasn't changed (lines 262-264)
- Updates state, persists to storage, and triggers sync (lines 266-268)
This addresses the core issue in #927 by providing a proper persistence mechanism for reordered servers.
lib/view/page/setting/seq/srv_seq.dart (5)
20-26: LGTM - proper local state initialization.Initializing local
_orderfrom the provider ininitStateis the correct approach. This provides immediate UI updates during drag operations while maintaining sync with the provider state.
30-37: LGTM - correct Riverpod listener pattern.The listener properly syncs local state with provider changes while preventing infinite loops via the
listEqualscheck. Placingref.listenin the build method is the correct Riverpod pattern - the framework handles subscription lifecycle automatically.
70-86: LGTM - correct reorder logic that fixes the bug.The reorder implementation correctly:
- Calculates
targetIndexaccounting for the index shift when dragging downward (lines 72-73)- Early exits if no actual reorder occurred (lines 75-77)
- Creates the new order by removing and reinserting the item (lines 79-81)
- Updates local state immediately for responsive UI (lines 83-85)
- Persists changes via the provider (line 86)
This fixes issue #927 by maintaining local state that updates immediately, preventing the item from reverting to its original position.
90-94: LGTM - proper data fetching and null handling.The item builder now correctly fetches server data (
Spi) by looking up the ID from the order list in the server state. The null check is good defensive programming that handles edge cases where a server might be briefly missing during deletion or sync operations.
111-114: Good defensive null handling.Returning an empty
SizedBoxfor nullSpigracefully handles edge cases where a server ID exists in the order but not in the server state (e.g., during brief sync windows when a server is deleted). This prevents crashes and UI glitches.
1 participant
Fixes: #927
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style