Macros tab "Move To Group" context menu command#530
Conversation
WalkthroughDraggableGroupVisibilityValueConverter was moved to a new namespace; a MoveToGroupCommand and supporting selection/move logic were added to MacrosTabViewModel; XAML resources and a context-menu item were updated in AutolootTabControl.xaml and MacrosTabControl.xaml to reference the converter and wire the new command. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as MacrosTabControl
participant VM as MacrosTabViewModel
participant Src as SourceCollection
participant Dst as TargetGroup.Children
User->>UI: Right-click MacroEntry
UI->>UI: Evaluate DraggableGroupVisibilityConverter
User->>UI: Select "Move to group" (passes target group)
UI->>VM: Execute MoveToGroupCommand(targetGroup)
VM->>Src: Remove MacroEntry (if in a group use old group's Children else Draggables)
VM->>VM: Set MacroEntry.Group = targetGroup
VM->>Dst: Add MacroEntry to targetGroup.Children
VM->>VM: newIndex = GetNewSelectedIndex(oldItem)
VM->>VM: SetNewSelectedIndex(newIndex)
VM->>UI: Notify UI of collection/selection changes
UI->>User: Updated tree shows item in new group
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ClassicAssist/UI/Misc/ValueConverters/DraggableGroupVisibilityValueConverter.cs(1 hunks)ClassicAssist/UI/ViewModels/MacrosTabViewModel.cs(4 hunks)ClassicAssist/UI/Views/Agents/AutolootTabControl.xaml(1 hunks)ClassicAssist/UI/Views/MacrosTabControl.xaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-22T07:54:53.601Z
Learnt from: Reetus
Repo: Reetus/ClassicAssist PR: 473
File: ClassicAssist/UI/Views/Agents/Autoloot/PropertyGroupControl.xaml:19-28
Timestamp: 2024-11-22T07:54:53.601Z
Learning: In the Autoloot `PropertyGroupControl.xaml`, the `SelectedItem.Children` list is expected to be small, so UI virtualization is unnecessary.
Applied to files:
ClassicAssist/UI/Views/Agents/AutolootTabControl.xaml
🧬 Code graph analysis (1)
ClassicAssist/UI/ViewModels/MacrosTabViewModel.cs (3)
ClassicAssist/UI/ViewModels/Agents/AutolootViewModel.cs (3)
MoveToGroup(873-889)GetNewSelectedIndex(912-935)SetNewSelectedIndex(891-910)ClassicAssist/Data/Macros/MacroGroup.cs (1)
MacroGroup(27-45)ClassicAssist/Data/Macros/MacroEntry.cs (2)
MacroEntry(40-514)MacroEntry(65-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
ClassicAssist/UI/Misc/ValueConverters/DraggableGroupVisibilityValueConverter.cs (1)
26-26: LGTM! Clean namespace refactoring.The move to a centralised
ValueConvertersnamespace is appropriate and improves code organisation.ClassicAssist/UI/Views/Agents/AutolootTabControl.xaml (1)
32-32: LGTM! Namespace reference updated correctly.The resource declaration properly references the converter from its new namespace whilst maintaining the same resource key.
ClassicAssist/UI/Views/MacrosTabControl.xaml (2)
34-34: LGTM! Resource properly registered.The converter resource is correctly added to support the new "Move to group" functionality.
127-163: LGTM! Well-structured context menu implementation.The conditional visibility logic and command bindings follow existing patterns in the codebase and align with the AutolootTabControl implementation.
ClassicAssist/UI/ViewModels/MacrosTabViewModel.cs (3)
77-77: LGTM! Command properly declared and configured.The
MoveToGroupCommandis correctly set up with appropriate predicates to ensureSelectedItemexists and the parameter is aMacroGroup.Also applies to: 207-207
1042-1061: LGTM! Selection management handles edge cases appropriately.The exception handling prevents crashes when the calculated index is out of range, which is acceptable for this scenario.
1063-1088: LGTM! Index calculation logic is sound.The method correctly returns 0 when the item is already in a group (no index adjustment needed) and calculates an appropriate selection index based on position when the item is at root level.
|
1 participant
Summary by CodeRabbit
New Features
Refactor