Skip to content

Conversation

@bhowiebkr
Copy link
Owner

Summary

  • Fix critical issue where groups could not be deleted using the Delete key or standard deletion methods
  • Add comprehensive DeleteGroupCommand with full state preservation and undo support
  • Enhance DeleteMultipleCommand to handle Group objects alongside existing node/connection support
  • Fix keyboard event handling in node_graph.py to process all item types including groups

Problem Resolved

Users reported "groups are unable to be deleted" - the Delete key and standard deletion workflows completely ignored Group objects, making them impossible to remove from the graph.

Root Cause Analysis

  1. Missing Command Infrastructure: No DeleteGroupCommand existed for handling group deletion
  2. Incomplete Batch Operations: DeleteMultipleCommand only handled Node/RerouteNode/Connection objects
  3. Broken Keyboard Handling: The keyPressEvent method in node_graph.py ignored Group objects entirely

Technical Implementation

New DeleteGroupCommand (src/commands/delete_group_command.py)

  • Complete state preservation including position, size, colors, member relationships
  • Robust undo functionality that restores original group object (not a recreation)
  • Memory usage estimation for command history management
  • Error handling for edge cases and missing group references

Enhanced DeleteMultipleCommand (src/commands/node/batch_operations.py)

  • Added Group object detection and handling via isinstance(item, Group)
  • Improved description generation for mixed selections (nodes + groups + connections)
  • Maintains existing functionality while extending support

Fixed Keyboard Event Handling (src/core/node_graph.py)

  • Replaced selective command creation with comprehensive DeleteMultipleCommand approach
  • Now processes all selected items regardless of type (Node, RerouteNode, Connection, Group)
  • Eliminates the root cause where groups were completely ignored during deletion

Test Coverage

Comprehensive verification confirms:

  • ✅ Groups can be deleted using Delete key (primary user workflow)
  • ✅ Groups can be deleted via direct command execution
  • ✅ Full undo/redo functionality preserves all group properties
  • ✅ Mixed selections (groups + nodes + connections) work seamlessly
  • ✅ State preservation maintains visual appearance, positioning, and member relationships

Files Changed

  • src/commands/delete_group_command.py - New comprehensive group deletion command
  • src/commands/node/batch_operations.py - Enhanced multi-item deletion support
  • src/core/node_graph.py - Fixed keyboard event handling for all item types
  • src/commands/__init__.py - Updated exports to include new command

Impact

This resolves a critical usability issue where groups became "permanent" once created, significantly improving the user experience and workflow efficiency.

🤖 Generated with Claude Code

Bryan Howard added 8 commits August 22, 2025 23:02
- Initialize groups list in NodeGraph constructor
- Add groups serialization/deserialization to persist groups in files
- Add groups cleanup to clear_graph() method
- Add duplicate prevention checks in CreateGroupCommand
- Groups now properly survive file save/load cycles
- Multiple undo/redo operations no longer create duplicate groups

Fixes issue where undoing and redoing group creation would result in
duplicate groups appearing in the scene.

🤖 Generated with [Claude Code](https://claude.ai/code)
- Add GroupPropertiesDialog with full UI for editing group properties
- Add alpha sliders for precise transparency control (background, border, title, selection colors)
- Add GroupPropertyChangeCommand for undoable property changes with batch support
- Add context menu support in Group class and NodeEditorView
- Add GroupPreviewWidget with transparency visualization
- Fix QPainter cleanup and QPointF arithmetic issues
- Simplify CSS styling to avoid Qt stylesheet parser warnings

Users can now right-click groups to access Properties dialog and adjust all
color properties including transparency values with proper undo/redo support.
- Add Groups section parsing to FlowFormatHandler markdown parser
- Add Groups section export to FlowFormatHandler markdown writer
- Update Group.serialize() to include complete color data with RGBA values
- Update Group.deserialize() to restore all color properties and brushes
- Update FlowSpec documentation with comprehensive groups specification
- Remove creation_timestamp from groups (unused metadata clutter)
- Add password_generator_tool_group.md example demonstrating groups in markdown

Groups now fully serialize/deserialize with transparency support in .md files.
All group properties including colors, position, size, padding, and membership
are properly saved and restored when loading graphs from markdown format.
- Enhanced copy_selected() to include groups in clipboard data
- Added complete property preservation for nodes (colors, size, GUI state)
- Implemented group copy/paste with full property restoration
- Fixed CreateNodeCommand to use correct Node API (calculate_absolute_minimum_size)
- Added external clipboard support for selections with groups
- Resolved 'Node object has no attribute min_width' runtime error

🤖 Generated with [Claude Code](https://claude.ai/code)
Fixed issue where pasted groups with member nodes were being transformed twice:
- Once correctly by PasteNodesCommand applying mouse position offset
- Again incorrectly by Group.itemChange automatically moving member nodes

Changes:
- Added _is_pasting flag to NodeGraph to track paste operations
- Modified Group.itemChange to skip automatic node movement during paste
- Enhanced paste() method to accept mouse position parameter
- Fixed data conversion to handle both 'uuid' and 'id' node identifiers

Groups now paste correctly at mouse cursor position with proper node positioning.

Generated with [Claude Code](https://claude.ai/code)
Added deferred GUI update mechanism for pasted nodes to ensure GUI widgets
refresh correctly, similar to when loading graphs from files.

Changes:
- Added deferred _final_paste_update() method to PasteNodesCommand
- Schedules GUI layout updates using QTimer.singleShot after paste completes
- Calls node._update_layout() to force complete layout rebuild and pin updates
- Only applies to regular nodes (excludes reroute nodes)

This ensures pasted nodes with GUI code display their widgets correctly
and behave identically to loaded nodes.

Generated with [Claude Code](https://claude.ai/code)
Split the monolithic 1,181-line node_commands.py file into a well-organized
package structure for better maintainability and code organization.

Changes:
- Created src/commands/node/ package with focused modules:
  * basic_operations.py - Create, delete, move node commands
  * property_changes.py - Property and code change commands
  * batch_operations.py - Paste, move/delete multiple commands
- Reduced main node_commands.py to 21 lines (97% reduction)
- Maintained 100% backward compatibility through re-exports
- Added comprehensive docstrings and module documentation
- Preserved all existing functionality and command behavior

Benefits:
- Better code organization and maintainability
- Easier navigation and understanding of command types
- Improved testability with focused modules
- Extensible structure for future command additions

Generated with [Claude Code](https://claude.ai/code)
…ete key

- Add DeleteGroupCommand with full state preservation and undo support
- Enhance DeleteMultipleCommand to handle Group objects alongside nodes/connections
- Fix node_graph.py keyPressEvent to use DeleteMultipleCommand for all item types
- Groups were previously ignored in keyboard deletion, now work seamlessly
- Comprehensive undo/redo support maintains group properties, colors, and positioning
- Update command exports to include new DeleteGroupCommand

🤖 Generated with [Claude Code](https://claude.ai/code)
@bhowiebkr bhowiebkr merged commit 452bd0e into main Aug 23, 2025
1 check failed
@bhowiebkr bhowiebkr deleted the bugfix/group-nodes branch August 23, 2025 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants