Add strategy pinning with manual ordering#77
Add strategy pinning with manual ordering#77devin-ai-integration[bot] wants to merge 14 commits into
Conversation
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Greptile SummaryThis PR adds strategy/folder pinning with manual ordering to the home screen. A new
Confidence Score: 3/5The core ordering and persistence logic is correct, but the root-view rendering mixes all-box lookups with root-filtered lists, causing pinned subfolder items to appear in two places at once. The duplication in folder_content.dart is a real, user-facing behavioral inconsistency: a root-level pin moves cleanly to the top, but a subfolder pin creates a ghost copy at the home screen while the item also remains in its folder. This will be confusing and could lead to surprising interactions (e.g., a user thinking they moved an item to root when they only pinned it). The fix requires either restricting pins to root-level items or propagating pin-exclusion into subfolder views. lib/widgets/folder_content.dart — the strategyById/folderById lookup and subsequent insertAll logic; lib/providers/pinned_items_provider.dart — the _saveOrder clear→putAll sequence. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as FolderPill or StrategyTile
participant PIP as PinnedItemsProvider
participant Hive as Hive Box int
participant FC as FolderContent root
UI->>PIP: togglePin(id)
PIP->>PIP: pinnedIdsByManualOrder() returns [id, existing...]
PIP->>Hive: box.clear()
PIP->>Hive: box.putAll(id to 0, ...)
PIP->>PIP: "state = nextState"
UI->>PIP: movePinUp(id)
PIP->>PIP: swap index in orderedIds
PIP->>Hive: box.clear() + putAll reindexed
PIP->>PIP: "state = nextState"
FC->>Hive: watch strategiesListenable
FC->>PIP: watch pinnedItemsProvider
PIP-->>FC: Map String to int pinned
FC->>FC: pinnedIdsInManualOrder(pinned)
FC->>FC: build strategyById and folderById from ALL items
FC->>FC: removeWhere pinned from root lists
FC->>FC: insertAll(0, pinnedStrategies and pinnedFolders)
Reviews (1): Last reviewed commit: "Add manual pinned item ordering" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| final strategyById = { | ||
| for (final s in strategyBox.values) s.id: s | ||
| }; | ||
| final folderById = { | ||
| for (final f in folderBox.values) f.id: f | ||
| }; | ||
|
|
||
| final pinnedStrategies = <StrategyData>[]; | ||
| final pinnedFolders = <Folder>[]; | ||
| for (final id in orderedPinnedIds) { | ||
| final s = strategyById[id]; | ||
| if (s != null) { | ||
| pinnedStrategies.add(s); | ||
| continue; | ||
| } | ||
| final f = folderById[id]; | ||
| if (f != null) pinnedFolders.add(f); | ||
| } | ||
|
|
||
| // Remove pinned items from their normal position, then | ||
| // re-insert at the front so they sort to the top. | ||
| strategies | ||
| .removeWhere((s) => pinned.containsKey(s.id)); | ||
| folders.removeWhere((f) => pinned.containsKey(f.id)); | ||
| strategies.insertAll(0, pinnedStrategies); | ||
| folders.insertAll(0, pinnedFolders); |
There was a problem hiding this comment.
strategyById and folderById are built from the entire Hive box (strategyBox.values / folderBox.values), not from the already-filtered root-level lists. As a result, a strategy or folder that lives inside a subfolder and is pinned will be inserted into the root view via pinnedStrategies/pinnedFolders, while also continuing to appear normally inside its own subfolder (the subfolder's FolderContent has no pin-filtering logic). The removeWhere calls have no effect on these items because they were never in the root-filtered lists to begin with. Root-level pinned items are correctly "moved to top" (no duplicate), but subfolder-pinned items end up visible in two places simultaneously.
| Future<void> _saveOrder(List<String> orderedIds) async { | ||
| final nextState = <String, int>{ | ||
| for (final entry in orderedIds.asMap().entries) entry.value: entry.key, | ||
| }; | ||
| await _box.clear(); | ||
| await _box.putAll(nextState); | ||
| state = nextState; | ||
| } |
There was a problem hiding this comment.
_box.clear() and _box.putAll() are two separate async operations. If the app is terminated (crash, force-close, OS kill) between the two calls, the box is left empty and all pin data is permanently lost. A safer pattern is to write the new state first and then remove stale keys, so there is never a window where valid data has been erased but the replacement has not yet landed.
| Future<void> _saveOrder(List<String> orderedIds) async { | |
| final nextState = <String, int>{ | |
| for (final entry in orderedIds.asMap().entries) entry.value: entry.key, | |
| }; | |
| await _box.clear(); | |
| await _box.putAll(nextState); | |
| state = nextState; | |
| } | |
| Future<void> _saveOrder(List<String> orderedIds) async { | |
| final nextState = <String, int>{ | |
| for (final entry in orderedIds.asMap().entries) entry.value: entry.key, | |
| }; | |
| // Write new entries first, then remove stale keys so there is no window | |
| // where the box is empty if the app is terminated mid-write. | |
| await _box.putAll(nextState); | |
| final staleKeys = | |
| _box.keys.whereType<String>().where((k) => !nextState.containsKey(k)); | |
| await _box.deleteAll(staleKeys.toList()); | |
| state = nextState; | |
| } |
|
Runtime tested PR #77 on Windows desktop ( Escalation: one unexpected Windows dialog appeared while quitting once: Manual pinned ordering test results
Evidence
Recordings:
Devin session: https://app.devin.ai/sessions/026effa9989f4a9c9d8ef2da902bc575 |
|
Runtime tested the drag-reorder update on Windows desktop with Escalation: I used the existing pinned test strategies from the prior desktop validation instead of recreating fresh strategies in this pass. This specifically validates the requested replacement behavior: no context-menu reorder buttons and drag/drop manual order. Drag-reorder test results
Evidence
Annotated recording: https://app.devin.ai/attachments/5679b2de-1cae-4bd4-a6e4-7c3306ff8bd6/pinned-drag-reorder-windows-edited.mp4 Session: https://app.devin.ai/sessions/026effa9989f4a9c9d8ef2da902bc575 |
2 participants
Summary
Adds a manual ordering layer for pinned strategies/folders on top of PR #75’s pinning feature: pinned items are stored as zero-based order indexes, surfaced at the top of their current folder scope, and reordered by dragging one pinned tile/pill onto another pinned item.
Key details:
pinnedIdsInManualOrder()/_comparePinnedEntries()and applied throughmovePin(id, targetId, insertAfterTarget)from drag targets.Move Pin to Top,Move Pin Up, andMove Pin Downcontext-menu actions were removed; menus now only expose Pin/Unpin plus the existing item actions.Link to Devin session: https://app.devin.ai/sessions/026effa9989f4a9c9d8ef2da902bc575
Requested by: @SunkenInTime