Feat Board move multiple items to a group with shortcut #8018
Conversation
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
Reviewer's GuideThis PR extends the board’s move operations to handle multiple items at once via keyboard shortcuts by generalizing the moveGroupRow API across Flutter and Rust layers, enhancing the BoardBloc logic for bulk adjacent‐group moves, and adding necessary utility support. Sequence Diagram for Moving Multiple Items to an Adjacent GroupsequenceDiagram
actor User
participant BoardShortcutContainer
participant BoardBloc
participant DatabaseController
participant DatabaseViewBackendService
participant RustBackend
User->>BoardShortcutContainer: Presses shortcut to move multiple items
BoardShortcutContainer->>BoardBloc: add(BoardEvent.moveGroupToAdjacentGroup(List<GroupedRowId>, toPrevious))
BoardBloc->>BoardBloc: Identify target group, filter row IDs for moving
BoardBloc->>DatabaseController: rowCache.getRows(List<RowId>)
DatabaseController-->>BoardBloc: return List<RowInfo> (with RowMetaPB)
BoardBloc->>DatabaseController: moveGroupRow(fromRow: List<RowMetaPB>, fromGroupId, toGroupId)
DatabaseController->>DatabaseViewBackendService: moveGroupRow(fromRowIds: List<RowId>, fromGroupId, toGroupId)
DatabaseViewBackendService->>RustBackend: Send MoveGroupRowPayloadPB (with from_row_ids: List<String>)
RustBackend->>RustBackend: Loop: For each rowId in from_row_ids, call database_editor.move_group_row(...)
RustBackend-->>DatabaseViewBackendService: Result
DatabaseViewBackendService-->>DatabaseController: Result
DatabaseController-->>BoardBloc: Result
alt On Success
BoardBloc->>BoardBloc: emit(BoardState.setFocus(List<GroupedRowId>))
end
Class Diagram for Board Event and Bloc Logic for Multi-Item MoveclassDiagram
class BoardEvent {
<<factory>> +moveGroupToAdjacentGroup(List~GroupedRowId~ groupedRowIds, bool toPrevious)
}
class _MoveGroupToAdjacentGroup {
+List~GroupedRowId~ groupedRowIds -- Parameter type changed from GroupedRowId --
+bool toPrevious
}
BoardEvent <|-- _MoveGroupToAdjacentGroup
class BoardBloc {
+on<_MoveGroupToAdjacentGroup>(_handleMoveGroupToAdjacentGroup)
-_handleMoveGroupToAdjacentGroup(event: _MoveGroupToAdjacentGroup, emit)
%% Methods called within handler reflect new list-based logic %%
#databaseController.moveGroupRow(fromRow: List~RowMetaPB~, ...)
#databaseController.rowCache.getRows(List~RowId~)
}
class GroupedRowId {
+String groupId
+String rowId
}
BoardBloc *-- DatabaseController : uses
Class Diagram for DatabaseController and Service Layer ChangesclassDiagram
class DatabaseController {
- _databaseViewBackendSvc: DatabaseViewBackendService
+ moveGroupRow(fromRow: List~RowMetaPB~, fromGroupId: String, toGroupId: String, toRow: RowMetaPB?): Future~FlowyResult~
-- Parameter fromRow changed from RowMetaPB to List<RowMetaPB> --
}
class DatabaseViewBackendService {
+ moveGroupRow(fromRowIds: List~RowId~, fromGroupId: String, toGroupId: String, toRowId: RowId?): Future~FlowyResult~
-- Parameter fromRowIds changed from RowId to List<RowId> --
}
class RowMetaPB {
+String id
%% other fields %%
}
class RowId~String~
DatabaseController o-- DatabaseViewBackendService : uses
DatabaseController ..> RowMetaPB : uses
DatabaseViewBackendService ..> RowId : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @PirateBrook - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
_moveGroupToAdjacentGroupno longer returns a bool consistently (link)
General comments:
- In Dart,
checkedGroupIndexs.sorted((a, b) => a - b)on a Set isn’t a built-in method—convert the Set to a List and callsort()(or import thecollectionextension) to avoid runtime errors. - The Rust handler loops and calls
move_group_rowper ID, which may hurt performance or transactional consistency; consider adding a batch move operation in your database editor instead of issuing individual calls.
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 1 other issue
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| toPrevious ? currentGroupIndex - 1 : currentGroupIndex + 1; | ||
| if (fromRow != null && | ||
| moveGroupToAdjacentGroup: (groupedRowIds, toPrevious) async { | ||
| final checkedGroupIndexs = groupedRowIds |
There was a problem hiding this comment.
nitpick (typo): Typo in variable name 'checkedGroupIndexs'
Rename to checkedGroupIndices or checkedGroupIndexes for correct pluralization and clarity.
| } | ||
|
|
||
| bool _moveGroupToAdjacentGroup(BuildContext context, bool toPrevious) { | ||
| if (focusScope.value.length != 1) { |
There was a problem hiding this comment.
issue (bug_risk): _moveGroupToAdjacentGroup no longer returns a bool consistently
Add an explicit return true after dispatching, and ensure return false is used for invalid cases, to match the method's bool return type.
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.
related issue #5524 [FR] Board move multiple items to a group with shortcut
Feature Preview
Kapture.2025-06-03.at.15.43.14.mp4
PR Checklist
Summary by Sourcery
Enable batch moving of board items by extending the moveGroupRow flow end-to-end—from RPC payloads and Rust handlers to Dart events, controller, and UI bindings—so multiple cards can be moved together via shortcut.
New Features:
Enhancements: