feat: add renaming frames functionality, fix: crash when trying to delete a group #51
+146
−3
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
ipa-danb
requested changes
Jan 8, 2026
Collaborator
ipa-danb
left a comment
ipa-danb
left a comment
There was a problem hiding this comment.
Need to check it out before merging (ill try to get to it next week)
Comment on lines
+216
to
+217
| if self.editor.frames.get(self.old_name): | ||
| del self.editor.frames[self.old_name] |
Collaborator
There was a problem hiding this comment.
Suggested change
| if self.editor.frames.get(self.old_name): | |
| del self.editor.frames[self.old_name] | |
| self.editor.frames.pop(self.old_name, None): |
or
Suggested change
| if self.editor.frames.get(self.old_name): | |
| del self.editor.frames[self.old_name] | |
| if self.old_name in self.editor.frames: | |
| del self.editor.frames[self.old_name] |
| else: | ||
| try: | ||
| self.editor.command(Command_RenameElement(self.editor, self.editor.frames[request.source_name], request.new_name)) | ||
| except Exception as e: |
Collaborator
There was a problem hiding this comment.
could you specify the exception a bit more? I think here it should be key and attribute errors
| existing_editor_frames = set(self.editor.all_editor_frame_ids()) | ||
|
|
||
| # allow recreating if frame was published by frameditor node originally | ||
| if new_name in existing_editor_frames or (new_name in existing_tf_frames and not Frame.was_published_by_frameeditor(new_name)): |
Collaborator
There was a problem hiding this comment.
It would be nice to have an error popup here i think
Comment on lines
+751
to
+752
| existing_tf_frames = set(self.editor.all_frame_ids()) | ||
| existing_editor_frames = set(self.editor.all_editor_frame_ids()) |
Collaborator
There was a problem hiding this comment.
i dont think it is required to make them into a set if you just check if an element is in the list
Comment on lines
+200
to
+203
| if editor.active_frame is element: | ||
| self.was_active = True | ||
| else: | ||
| self.was_active = False |
Collaborator
There was a problem hiding this comment.
Suggested change
| if editor.active_frame is element: | |
| self.was_active = True | |
| else: | |
| self.was_active = False | |
| self.was_active = editor.active_frame is element |
Comment on lines
+235
to
+236
| if self.editor.frames.get(self.new_name): | ||
| del self.editor.frames[self.new_name] |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Add the ability to rename a frame either by clicking the button or by directly clicking the frame name field.
Additionally, fix the bug that causes the system to crash when trying to delete a group.