feat: add renaming frames functionality, fix: crash when trying to delete a group #51
Conversation
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)
| if self.editor.frames.get(self.old_name): | ||
| del self.editor.frames[self.old_name] |
There was a problem hiding this comment.
| if self.editor.frames.get(self.old_name): | |
| del self.editor.frames[self.old_name] | |
| self.editor.frames.pop(self.old_name, None): |
or
| 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] |
There was a problem hiding this comment.
Done. Changed it to self.editor.frames.pop(self.old_name, None)
| else: | ||
| try: | ||
| self.editor.command(Command_RenameElement(self.editor, self.editor.frames[request.source_name], request.new_name)) | ||
| except Exception as e: |
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
There was a problem hiding this comment.
Done. I added the AttributeError exception when the dictionary entry cannot be found. The general exception is still there in case I missed something.
| 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)): |
There was a problem hiding this comment.
It would be nice to have an error popup here i think
There was a problem hiding this comment.
Done. Impleneted an error pop-up. Had to insert another check source_name != new_name. Otherwise, an error will pop up every time someone clicks on the box without actually inserting something.
| existing_tf_frames = set(self.editor.all_frame_ids()) | ||
| existing_editor_frames = set(self.editor.all_editor_frame_ids()) |
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
There was a problem hiding this comment.
Still Open.
That is true. I've tried to stick as closely as possible to the source code and was inspired by the function get_valid_frame_name(). I would leave it as is for now, unless you see it differently
| if editor.active_frame is element: | ||
| self.was_active = True | ||
| else: | ||
| self.was_active = False |
There was a problem hiding this comment.
| if editor.active_frame is element: | |
| self.was_active = True | |
| else: | |
| self.was_active = False | |
| self.was_active = editor.active_frame is element |
| if self.editor.frames.get(self.new_name): | ||
| del self.editor.frames[self.new_name] |
3 participants
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.