Merged
Conversation
fulsomenko
commented
Feb 17, 2026
Owner
Author
fulsomenko
left a comment
There was a problem hiding this comment.
PR Review: KAN-209/multi-select-cards
Summary
This PR implements a vim-style visual selection mode for multi-card operations. The implementation is clean and follows existing patterns in the codebase.
👍 Strengths
- Clean architecture: New handlers and components follow SOLID principles and existing patterns
- Good UX: Footer indicator clearly shows selection state (
-- SELECT (N) --vs(N selected)) - Comprehensive feature set: Selection mode toggle, select all, clear, bulk priority, and bulk move
- Proper keybinding integration: Actions properly wired through the keybinding system
🔍 Observations & Suggestions
-
Behavior change in
handle_card_selection_toggle()(card_handlers.rs:37-48)- Previously toggled individual card selection, now toggles selection mode
- This changes existing
vkey behavior - confirm this is intentional
-
Selection mode not cleared after bulk priority (
popup_handlers.rs:118-120)- After setting bulk priority,
selected_cards.clear()is called butselection_mode_activeremainstrue - Should probably also set
self.selection_mode_active = false
- After setting bulk priority,
-
Escape key behavior change (
navigation_handlers.rs:496-502)- Escape now clears selection first before navigating back
- Users might expect escape to go back; hitting escape twice to navigate could feel odd
- Consider: maybe only intercept escape if
selection_mode_activeis true, not when there are just passive selections?
-
Magic number (
popup_handlers.rs:84)self.priority_selection.next(4)uses a magic number- Could extract to a constant like
PRIORITY_COUNTfor clarity
-
Silent failure on bulk move (
card_handlers.rs:571-577)- On error, logs with
tracing::error!and returns - Consider adding user feedback (toast/notification) for failed operations
- On error, logs with
-
Minor: Changeset format
- The changeset lists individual commit messages rather than a user-facing summary
- Consider condensing to a single feature description for the changelog
✅ Overall
The implementation is solid and well-structured. The suggestions above are mostly minor improvements. The core functionality looks correct and the test checklist in the PR description is comprehensive.
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
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.
Implements a visual selection mode for multi-card operations in the TUI.
What
Adds a visual selection mode where pressing
venters selection mode, and navigating withj/kautomatically selects cards. This enables efficient bulk operations on multiple cards without toggling each card individually.Why
The previous implementation required pressing
von each card individually to select it for bulk operations. This was tedious for users who wanted to select multiple consecutive cards. The new vim-style selection mode provides a more intuitive and efficient workflow that matches established vim conventions.How
Core Implementation:
selection_mode_active: boolfield to App struct to track mode statehandle_card_selection_toggle()to toggle mode instead of individual cardshandle_navigation_down()andhandle_navigation_up()that automatically adds cards to selection when navigating in selection modehandle_escape_key()to clear both selection mode and all selectionsNew Features:
v- Toggle selection mode (enters mode and selects current card, or exits mode keeping selections)j/k- Auto-select cards while navigating in selection modeCtrl+a- Select all cards in current viewEscape- Clear all selections and exit selection modeP- Set priority on all selected cards via bulk priority dialogUI Feedback:
-- SELECT (N) --when in selection mode(N selected)when cards are selected but not in selection modeFile Changes:
app.rsselection_mode_activefield,SetMultipleCardsPrioritydialog mode, keybinding action wiring, Ctrl+a and P shortcutscard_handlers.rshandle_clear_card_selection(),handle_select_all_cards_in_view(),handle_set_selected_cards_priority(),move_selected_cards()navigation_handlers.rspopup_handlers.rshandle_set_multiple_cards_priority_popup()ui.rsrender_set_multiple_cards_priority_popup()keybindings/mod.rsClearCardSelection,SelectAllCards,SetSelectedCardsPriorityactionskeybindings/card_list.rskeybindings/registry.rscomponents/selection_dialog.rsBulkPriorityDialogcomponentTesting
cargo build- Compiles successfullycargo test --package kanban-tui- All tests passcargo clippy --all-targets -- -D warnings- No warningsv, verify current card selectedj/k, verify cards auto-selectv, verify selections persistEscape, verify all clearedCtrl+aP