Implement significant UI bug fixes#5
Conversation
Fix multiple links to new node created via picker
Fix to only disconnect picker_from_port
Fix graph connection issues caused by removal of ChoiceNode options
Fix general UI issues
Fix tab UI issues
Fix zero options UI problem
Fix FileDialog and welcome panel
Implement option ID editing and copy notifications
Reviewer's Guide by SourceryThis pull request addresses multiple visual UI bugs in the Monologue application, enhances file operation handling, and improves node connection logic. Additionally, it introduces filename truncation and refines ID management for nodes and options. Key changes include the addition of filename truncation, dynamic node connection updates, and improved file dialog operations. The PR also introduces visual feedback for ID copy actions and fixes focus issues on the welcome window. File-Level Changes
Tips
|
There was a problem hiding this comment.
Hey @RailKill - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding more inline comments to explain complex logic, especially in the new and heavily modified functions. This will improve long-term maintainability.
- Some error handling has been removed or changed, particularly in file operations. Please review to ensure all error cases are still properly handled.
- Given the extensive changes, it would be beneficial to add or update unit tests to cover the new functionality and prevent regressions.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 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 to tell me if it was helpful.
| graph_node = node | ||
| return graph_node | ||
|
|
||
| func link_option(option_dictionary: Dictionary, establish_link: bool = true): |
There was a problem hiding this comment.
suggestion (bug_risk): Add null checks in link_option function
Consider adding null checks for 'next_node' and 'option_dictionary' to prevent potential null reference exceptions. This will make the function more robust and less prone to runtime errors.
| func link_option(option_dictionary: Dictionary, establish_link: bool = true): | |
| func link_option(option_dictionary: Dictionary, establish_link: bool = true): | |
| if option_dictionary == null: | |
| return | |
| var option_index = options.find(option_dictionary) | |
| var next_node = get_graph_node(option_dictionary.get("NextID")) | |
| if next_node == null: | |
| return |
| panel_node.side_panel.control_node.add_child(ribbon) | ||
|
|
||
|
|
||
| func _on_id_text_changed(new_id): |
There was a problem hiding this comment.
suggestion (bug_risk): Implement ID conflict prevention
Allowing direct editing of IDs could lead to conflicts. Consider implementing a mechanism to ensure uniqueness of IDs, such as validation against existing IDs or auto-generation of unique IDs.
|
Moving back into draft due to critical bugs found:
EDIT: Critical bugs have been fixed. |
There was a problem hiding this comment.
Hey @RailKill - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 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 to tell me if it was helpful.
|
Thanks for continuing to improve this project, all these changes are really nice! |
This PR fixes a bunch of visual UI bugs that were mostly present in Monologue even before v2.2.2:
Summary by Sourcery
This pull request addresses multiple visual UI bugs in the Monologue application and introduces several enhancements to improve user experience, including filename truncation, editable RootNodeID, and better file dialog handling.