ref:Refactor Settings UI and Fix Performance Issues#1026
ref:Refactor Settings UI and Fix Performance Issues#1026GT-610 merged 17 commits intolollipopkit:mainfrom
Conversation
…mize state management - Extract the logic for building list items into a separate method to improve maintainability - Add animation effects to enhance the dragging experience - Use PageStorageKey to maintain the scroll position - Optimize the state management logic of the checkbox - Add new contributors in github_id.dart
…obscured Add SafeArea wrapping content in multiple settings pages to prevent content from being obscured by the navigation bar on certain devices, thereby enhancing user experience
…s loading of iOS settings page Extract the `_getEntities` method from an inline function to a class member method to enhance code readability Preload watch context and push token in the iOS settings page to avoid repeatedly creating Futures
… and merge the Android settings into the main page Migrate the Android platform settings from a standalone page to the main settings page, and remove redundant Android settings page files Adjust the platform setting logic, retaining only the special setting entry for the iOS platform
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughPersisted reorder/enable state for multiple settings lists; moved Android settings into App settings and removed standalone Android page; added store properties for disabled items; stabilized widget keys; added error handling to backup restore dialog; extracted local storage sorting; bumped fl_lib ref. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as "Settings UI"
participant Store as "SettingStore"
participant Pref as "Persistent Storage"
User->>UI: Drag item / Toggle enable
UI->>UI: update local _order / _enabled
UI->>Store: write order prop
Store->>Pref: persist order list
Pref-->>Store: write ack
UI->>Store: write disabled keys prop
Store->>Pref: persist disabled list
Pref-->>Store: write ack
Store-->>UI: (optional) apply persisted state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31126bbe4c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Add persistent storage functionality for server detail cards and SSH virtual key disable status Modify the logic of relevant pages to support the saving and restoration of disabled states
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@coderabbitai Review. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/view/page/server/edit/widget.dart (1)
61-73: Use stable IDs for chip keys instead of index or object identity.Index-based keys (
ValueKey(index)) and object-identity keys (ValueKey(item)) drift when lists reorder, reload, or items are recreated, causing widget state and animation mismatches. BothPrivateKeyInfoandSpihave stableidfields; useValueKey(item.id)for consistent key identity.♻️ Proposed change
- return ChoiceChipX<int>( - key: ValueKey(index), + return ChoiceChipX<int>( + key: ValueKey(item.id), label: item.id, state: state, value: index, onSelected: (idx, on) {- return ChoiceChipX<Spi>( - key: ValueKey(item), + return ChoiceChipX<Spi>( + key: ValueKey(item.id), label: item.name, state: state, value: item, onSelected: (srv, on) {
🤖 Fix all issues with AI agents
In `@lib/view/page/setting/seq/srv_detail_seq.dart`:
- Around line 30-35: The bug is that _saveChanges currently writes only enabled
items to detailCardOrder so prop.fetch() later returns a subset and _order loses
disabled items; update the logic in _saveChanges (and any code that writes
detailCardOrder) to persist the full _order (enabled + disabled) instead of only
enabled, or alternatively modify _loadData to reconstruct the full order by
taking prop.fetch() and appending any items from disabledProp.fetch() that are
missing; ensure the identifiers involved are _saveChanges, _loadData,
detailCardOrder, prop.fetch(), disabledProp.fetch(), _order and _enabled so the
full card order is preserved across page reopen.
In `@lib/view/page/setting/seq/virt_key.dart`:
- Around line 33-38: _prop.fetch() currently returns only enabled keys so
_loadData sets _order to that list and loses disabled items; fix by
reconstructing the full order using both prop.fetch() and disabledProp.fetch():
in _loadData (and the similar block at lines 171-176) set _order to
List<int>.from(keys) then append any ids from disabled that are missing (e.g.
for each id in disabled.keys where !keys.contains(id) add to _order), and keep
_enabled as Set<int>.from(keys.where((k) => !disabled.contains(k))); this
preserves original ordering for enabled items while reattaching disabled items
so they don’t disappear after reopening the page.
In `@lib/view/page/storage/local.dart`:
- Around line 180-185: Collect FileStat once asynchronously and sort tuples in
memory to avoid blocking the isolate and duplicate stats: in _getEntities(),
first await a single Future.wait over Directory(...).list() to produce
List<(FileSystemEntity, FileStat)> via e.stat(), then remove the earlier call to
_sortType.value.sort(files) and instead sort that list in-memory (e.g.,
stats.sort((a,b) => /* use the same comparison logic used by _SortType.sort but
reading a.$2 and b.$2 */)). Alternatively expose a comparator on _SortType
(e.g., _SortType.compare(FileStat a, FileStat b) or _SortType.compareTuple) and
call stats.sort(_sortType.value.compare) so you do not call statSync()/stat()
more than once and avoid blocking the UI in _getEntities.
🧹 Nitpick comments (3)
lib/data/model/app/bak/backup_service.dart (1)
124-132: Good UX improvement: explicit Cancel button added.Adding the Cancel button alongside the OK button improves clarity for users. The pattern is consistent with
_showPasswordDialogat line 160.Consider adding error handling in the async callback.
If
backup.$1.merge(force: true)throws, the dialog remains open and no error is shown. Other restore paths in this file wrap operations in try-catch (e.g., lines 56-59).♻️ Optional: Add error handling for robustness
actions: [ Btn.cancel(), Btn.ok( onTap: () async { - await backup.$1.merge(force: true); - context.pop(); + try { + await backup.$1.merge(force: true); + context.pop(); + } catch (e, s) { + context.pop(); + context.showErrDialog(e, s, libL10n.restore); + } }, ), ],lib/view/page/setting/platform/ios.dart (1)
20-22: Consider a refresh path for cached futures to avoid stale UI.Caching is good for performance, but watch pairing/push token changes after the page opens won’t be reflected. If that’s a possible flow, consider a lightweight refresh trigger (e.g., on resume or a manual refresh) that reassigns the futures and calls
setState.Also applies to: 66-68, 86-88
lib/view/page/setting/entries/app.dart (1)
40-64: Filter widget prefs by prefix before opening the editor.Right now all String prefs are collected. To avoid exposing unrelated prefs (and reduce payload), filter by the
widget_prefix before populatingdata.♻️ Suggested adjustment
- for (final key in keys) { - final val = PrefStore.shared.get<String>(key); - if (val != null) { - data[key] = val; - } - } + for (final key in keys) { + if (!key.startsWith(prefix)) continue; + final val = PrefStore.shared.get<String>(key); + if (val != null) data[key] = val; + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
lib/data/model/app/bak/backup_service.dartlib/data/res/github_id.dartlib/data/store/setting.dartlib/view/page/server/detail/view.dartlib/view/page/server/edit/widget.dartlib/view/page/setting/entries/app.dartlib/view/page/setting/entry.dartlib/view/page/setting/platform/android.dartlib/view/page/setting/platform/ios.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.dartlib/view/page/storage/local.dartpubspec.yaml
💤 Files with no reviewable changes (2)
- lib/view/page/setting/entry.dart
- lib/view/page/setting/platform/android.dart
🧰 Additional context used
📓 Path-based instructions (4)
lib/view/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/view/**/*.dart: Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Prefer using libL10n strings before adding new ones to project l10n
Split UI into build, actions, and utils; use extension on to separate concerns
Files:
lib/view/page/setting/seq/srv_seq.dartlib/view/page/server/edit/widget.dartlib/view/page/setting/seq/srv_func_seq.dartlib/view/page/storage/local.dartlib/view/page/setting/platform/ios.dartlib/view/page/server/detail/view.dartlib/view/page/setting/seq/virt_key.dartlib/view/page/setting/entries/app.dartlib/view/page/setting/seq/srv_detail_seq.dart
lib/data/store/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Persist state using Hive stores placed under lib/data/store/
Files:
lib/data/store/setting.dart
lib/data/model/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
After modifying annotated model files, run: dart run build_runner build --delete-conflicting-outputs
Files:
lib/data/model/app/bak/backup_service.dart
pubspec.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hive_ce instead of hive for Hive integration
Files:
pubspec.yaml
🧠 Learnings (6)
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/store/**/*.dart : Persist state using Hive stores placed under lib/data/store/
Applied to files:
lib/data/store/setting.dart
📚 Learning: 2026-01-22T05:10:54.502Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-22T05:10:54.502Z
Learning: For all future pull requests in the flutter_server_box repository, check if participants from issues resolved by the PR are added to the `participants` field in `lib/data/res/github_id.dart`. If any participants are missing, propose adding them. First verify if they already exist or have been added in the PR commits.
Applied to files:
lib/data/res/github_id.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Applied to files:
lib/view/page/setting/seq/virt_key.dartlib/view/page/setting/entries/app.dartlib/view/page/setting/seq/srv_detail_seq.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Prefer using libL10n strings before adding new ones to project l10n
Applied to files:
lib/view/page/setting/seq/virt_key.dartlib/view/page/setting/seq/srv_detail_seq.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Split UI into build, actions, and utils; use extension on to separate concerns
Applied to files:
lib/view/page/setting/seq/virt_key.dartlib/view/page/setting/entries/app.dartlib/view/page/setting/seq/srv_detail_seq.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/server/**/*.dart : Place server management models under lib/data/model/server/
Applied to files:
lib/view/page/setting/seq/srv_detail_seq.dart
🔇 Additional comments (20)
pubspec.yaml (1)
69-69: LGTM!The fl_lib dependency version bump from v1.0.362 to v1.0.363 aligns with the PR objectives for performance fixes. As per coding guidelines, the project correctly uses
hive_ce_flutterandhive_ce_generatorinstead of the originalhivepackage.lib/view/page/setting/platform/ios.dart (1)
23-26: Good guard before reading the watch context.The
isPairedcheck keeps the async call minimal and the null return integrates cleanly with the UI state.lib/data/store/setting.dart (1)
64-68: LGTM!The new persistent properties for tracking disabled detail cards and SSH virtual keys are correctly added using
listProperty. This follows the existing patterns in the store and aligns with the coding guideline to persist state using Hive stores.lib/view/page/server/detail/view.dart (1)
86-87: LGTM!The filtering logic correctly removes both invalid card names and explicitly disabled cards from the display order. This integrates properly with the new
detailCardDisabledpersistence property.lib/view/page/setting/seq/srv_detail_seq.dart (2)
45-59: LGTM!The
_proxyDecoratorprovides smooth visual feedback during drag operations with animated elevation and scale transformations.
61-71: LGTM!Good use of
PageStorageKeyto preserve scroll position andbuildDefaultDragHandles: falsewith explicit drag listeners for controlled drag behavior.lib/view/page/setting/seq/srv_func_seq.dart (2)
33-37: LGTM!Good additions:
PageStorageKeyfor scroll position preservation and extraction of item building to_buildListItemwith stableValueKey(key). The pattern correctly derives disabled items from the enum on each build, avoiding the persistence issues seen in other sequence pages.
50-67: LGTM!Clean extraction of list item building logic. The
ValueKey(key)ensures stable widget identity during reordering.lib/view/page/setting/seq/srv_seq.dart (1)
41-41: LGTM!Consistent
SafeAreawrapping aligns with the other sequence pages in this PR.lib/view/page/setting/seq/virt_key.dart (3)
65-79: LGTM!Consistent animated drag decoration pattern with the other sequence pages.
93-117: LGTM!Good conditional rendering of drag handle based on
isDesktop, and proper use ofValueKey(key)for stable widget identity.
119-135: LGTM!The opacity-based visual feedback for disabled items provides clear visual distinction. Wrapping in
IgnorePointerprevents accidental interactions with the disabled title area.lib/view/page/setting/entries/app.dart (6)
5-15: Clean conditional insertion of Android settings.The conditional node keeps the settings list tidy and avoids null entries.
22-31: Android settings grouping looks good.The ExpandTile grouping is clear and consistent with other settings sections.
33-37: BG run toggle wiring is straightforward.The StoreSwitch binding is direct and easy to follow.
66-81: Nice localized save/cleanup helper.The prefix-guarded delete/write logic keeps widget prefs scoped and clean.
83-90: iOS-only platform setting is clear.The early return simplifies platform-specific rendering.
196-197: Dialog actions are fine.Using the default cancel button keeps the action row concise.
lib/data/res/github_id.dart (1)
139-141: Verify that all participants from issues closed by PR#1026are included in theGithubIds.participantslist.The three participants (
atikattar1104,coldboy404,puskyer) have been successfully added. However, without access to the GitHub API to identify all issues closed by PR#1026and their participants, I cannot confirm whether this list is complete. Please verify using GitHub's PR details that every participant from the closed issues has been included.lib/view/page/storage/local.dart (1)
93-97: The.listen()guard on_buildBodyprevents unrelated parent rebuilds from triggering redundant I/O.
_buildBodyonly rebuilds when_sortTypechanges (via the popup menu) or on explicit user actions (upload, refresh, navigation). No refactor needed—the reactive listener pattern already solves the hypothetical problem described.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…ormance In the settings page, remove the unnecessary `enabledList` filtering and directly save the `_order` list Optimize the sorting logic on the local file page by first retrieving the file status before proceeding with sorting
… settings page Fix the data filtering logic in the settings page to only process key-value pairs with specific prefixes Add error handling to the backup service, capture and display merge failure exceptions
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/view/page/setting/entries/app.dart`:
- Around line 40-75: _change_ _saveWidgetSP to return Future<void> and await
PrefStore.shared.remove(...) and PrefStore.shared.set(...): update the signature
from void _saveWidgetSP(...) to Future<void> _saveWidgetSP(...) and add await
before both PrefStore.shared.remove(key) and PrefStore.shared.set(entry.key,
entry.value). Also update the caller in _buildAndroidWidgetSharedPreference's
onTap to await _saveWidgetSP(result, data, prefix) since the onTap handler is
async (so the write operations complete before proceeding). Use the existing
symbols _buildAndroidWidgetSharedPreference, _saveWidgetSP, and
PrefStore.shared.remove/set to locate changes.
In `@lib/view/page/storage/local.dart`:
- Line 95: _buildBody currently calls _getEntities() directly creating a fresh
Future on every rebuild; cache that Future as a class property (e.g.,
_entitiesFuture) and initialize it in initState (or lazily) so the FutureWidget
uses the same Future across rebuilds; add a private _refresh() that recreates
_entitiesFuture = _getEntities() and calls setState to trigger reload, and
replace any setState(() {}) calls that intend to refresh data with calls to
_refresh(); ensure references to _getEntities() remain for creating the Future
but never directly inside _buildBody so UI rebuilds reuse the cached Future.
🧹 Nitpick comments (4)
lib/view/page/storage/local.dart (1)
400-413: Remove the unusedsort()method—compareTuple()handles sorting instead.The
_SortType.sort()method contains blockingstatSync()calls but is not called anywhere in the codebase. The active code at line 183 usescompareTuple()instead, which operates on pre-fetched(FileSystemEntity, FileStat)tuples. Removing the deadsort()method eliminates unreachable code that could be mistakenly reused.lib/view/page/setting/seq/virt_key.dart (1)
144-158: Skip persistence on no-op reorder.
Right now_saveChanges()runs even when the target index doesn’t change. Consider an early return to avoid unnecessary writes.♻️ Suggested tweak
void _handleReorder(int oldIndex, int newIndex) { - setState(() { - var targetIndex = newIndex; - if (targetIndex > oldIndex) { - targetIndex -= 1; - } - if (targetIndex == oldIndex) { - return; - } - - final item = _order.removeAt(oldIndex); - _order.insert(targetIndex, item); - }); - _saveChanges(); + var targetIndex = newIndex; + if (targetIndex > oldIndex) { + targetIndex -= 1; + } + if (targetIndex == oldIndex) { + return; + } + setState(() { + final item = _order.removeAt(oldIndex); + _order.insert(targetIndex, item); + }); + _saveChanges(); }lib/view/page/setting/seq/srv_detail_seq.dart (1)
102-116: Skip persistence on no-op reorder.
Same as in the virtual keys list,_saveChanges()runs even when indices don’t change. Consider early-returning beforesetState.♻️ Suggested tweak
void _handleReorder(int oldIndex, int newIndex) { - setState(() { - var targetIndex = newIndex; - if (targetIndex > oldIndex) { - targetIndex -= 1; - } - if (targetIndex == oldIndex) { - return; - } - - final item = _order.removeAt(oldIndex); - _order.insert(targetIndex, item); - }); - _saveChanges(); + var targetIndex = newIndex; + if (targetIndex > oldIndex) { + targetIndex -= 1; + } + if (targetIndex == oldIndex) { + return; + } + setState(() { + final item = _order.removeAt(oldIndex); + _order.insert(targetIndex, item); + }); + _saveChanges(); }lib/view/page/setting/entries/app.dart (1)
22-26: Localize the Android settings title if a key exists.Line 25 mixes a hardcoded “Android” with a localized suffix; consider using a full localized label if available to avoid partial localization. As per coding guidelines, prefer libL10n strings when possible.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/data/model/app/bak/backup_service.dartlib/view/page/setting/entries/app.dartlib/view/page/setting/seq/srv_detail_seq.dartlib/view/page/setting/seq/virt_key.dartlib/view/page/storage/local.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/data/model/app/bak/backup_service.dart
🧰 Additional context used
📓 Path-based instructions (1)
lib/view/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/view/**/*.dart: Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Prefer using libL10n strings before adding new ones to project l10n
Split UI into build, actions, and utils; use extension on to separate concerns
Files:
lib/view/page/setting/seq/virt_key.dartlib/view/page/setting/entries/app.dartlib/view/page/setting/seq/srv_detail_seq.dartlib/view/page/storage/local.dart
🧠 Learnings (4)
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Applied to files:
lib/view/page/setting/seq/virt_key.dartlib/view/page/setting/seq/srv_detail_seq.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Prefer using libL10n strings before adding new ones to project l10n
Applied to files:
lib/view/page/setting/seq/virt_key.dartlib/view/page/setting/seq/srv_detail_seq.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Split UI into build, actions, and utils; use extension on to separate concerns
Applied to files:
lib/view/page/setting/seq/virt_key.dartlib/view/page/setting/seq/srv_detail_seq.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/server/**/*.dart : Place server management models under lib/data/model/server/
Applied to files:
lib/view/page/setting/seq/srv_detail_seq.dart
🔇 Additional comments (19)
lib/view/page/storage/local.dart (2)
180-185: Good: async stat collection avoids UI thread blocking.The refactored
_getEntities()correctly fetchesFileStatasynchronously viaFuture.waitand sorts using the pre-fetched stats viacompareTuple. This addresses the previous concern about blocking the UI thread withstatSync().
415-421: LGTM!The
compareTuplemethod correctly uses pre-fetchedFileStatfrom the tuple, avoiding any filesystem calls during sorting. The switch expression is clean and idiomatic.lib/view/page/setting/seq/virt_key.dart (7)
20-38: Clean separation of order vs enabled state.
State init and_loadDataare straightforward and keep ordering separate from enablement.
40-54: SafeArea + column layout looks good.
Keeps the settings header and list clear of insets.
65-78: Nice drag preview styling.
The animated elevation/scale in_proxyDecoratorprovides clear feedback.
81-90: Reorderable list setup is solid.
Custom drag handles andPageStorageKeyimprove UX/state retention.
93-117: List item composition is clear and stable.
StableValueKeyand trailing controls are well structured.
119-142: Disabled-state styling is clear.
Opacity + checkbox toggle keeps the state visible and consistent.
160-175: Persisting enabled/disabled state is handled cleanly.
_toggleEnabled+_saveChangeskeeps both the order and disabled list in sync.lib/view/page/setting/seq/srv_detail_seq.dart (7)
17-35: State init looks correct and minimal.
Order/enabled split is clear and easy to maintain.
37-42: SafeArea usage is appropriate.
Keeps list content aligned with system insets.
45-58: Drag animation is a nice UX touch.
The elevation/scale proxy reads well during reordering.
61-70: ReorderableListView configuration looks good.
Stable storage key + custom drag handles are a good fit here.
73-93: List item rendering is clear and consistent.
Leading icon + trailing controls read well in the settings list.
95-100: Checkbox handler is clean and direct.
No issues with the toggle wiring.
118-133: Persistence flow is consistent.
_toggleEnabledand_saveChangeskeep order and disabled set in sync.lib/view/page/setting/entries/app.dart (3)
5-15: Conditional Android settings injection looks good.Keeps the list clean for non-Android while still centralizing platform settings.
83-89: iOS-only platform tile wiring looks correct.The null return for non‑iOS keeps the list clean while still preserving the iOS entry.
195-195: VerifyBtn.cancel()still closes the dialog.Previously this explicitly popped; if
Btn.cancel()doesn’t pop by default in fl_lib, the dialog may become non‑dismissable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…luded in the order settings and asynchronously saved preference settings Fix the issue where disabled items in the virtual keyboard and service details order settings are not included in the order list Change the preference setting saving method to an asynchronous operation, and add a mounted check to prevent updating the state after the component is unmounted
…methods Narrow the scope of state updates in the reordering logic to only encompass the parts where data is actually modified Remove the unused sorting methods in `_local.dart` to simplify the code
|
@coderabbitai Review. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Refactor the refresh method that directly calls setState into a unified _refresh method Use the `_entitiesFuture` to cache the list of files to obtain results and avoid redundant calculations
|
@coderabbitai Review. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
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/view/page/storage/local.dart (1)
192-199: Sorting won’t update after changing sort type.With a cached
_entitiesFuture, changing_sortTypeno longer re-sorts the data, so the list stays in the previous order. Trigger a refresh (or resort the in-memory list) when the selection changes.🔧 Proposed fix
onSelected: (value) { - _sortType.value = value; + if (_sortType.value == value) return; + _sortType.value = value; + _refresh(); },
🤖 Fix all issues with AI agents
In `@lib/view/page/storage/local.dart`:
- Around line 47-51: The _refresh method currently calls setState directly and
assigns _entitiesFuture immediately, which can run after dispose and does not
let callers await completion; change _refresh to assign _entitiesFuture to the
result of calling _getEntities() and use setStateSafe to update state, and make
_refresh async return the future so callers (including the RefreshIndicator) can
await it; update any RefreshIndicator.onRefresh usages (the one at lines ~90–92)
to return await _refresh() so the refresh completes only after the future
resolves.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/view/page/storage/local.dart
🧰 Additional context used
📓 Path-based instructions (1)
lib/view/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/view/**/*.dart: Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Prefer using libL10n strings before adding new ones to project l10n
Split UI into build, actions, and utils; use extension on to separate concerns
Files:
lib/view/page/storage/local.dart
🧠 Learnings (1)
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/store/**/*.dart : Persist state using Hive stores placed under lib/data/store/
Applied to files:
lib/view/page/storage/local.dart
🔇 Additional comments (3)
lib/view/page/storage/local.dart (3)
38-38: Good: cached entities future avoids rebuild I/O.This keeps the list fetch stable across rebuilds and aligns with the stated performance fix.
Also applies to: 98-101
65-76: Nice: refresh hooks after file/nav actions keep the list in sync.Also applies to: 79-84, 109-116, 167-173
185-189: Async stat collection + tuple comparator looks solid.Also applies to: 405-410
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Refactors settings pages for better UI consistency and fixes performance issues from fl_lib optimization review.
Changes
UI Refactoring
Performance Fixes
local.dart,ios.dart)srv_func_seq.dart)ValueKey(item))Dependencies
fl_libfromv1.0.361tov1.0.363Breaking Changes
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.