Fix flags locking when group editing and inconsistent flags #1594
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes the disabling of flag widgets by deferring it to the dialog’s popUp method and cleans up redundant loops in the label editor.
- Added a
flags_disabledparameter topopUpand moved flag disabling into that method after widget creation. - Refactored
_edit_labelto passflags_disabledinstead of manually iterating over flag widgets, removing duplicate loops.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| labelme/widgets/label_dialog.py | Added flags_disabled param to popUp and moved flag-disabling logic |
| labelme/app.py | Removed manual flag disable/enable loops and passed flags_disabled |
| @@ -211,6 +219,9 @@ def popUp(self, text=None, move=True, flags=None, group_id=None, description=Non | |||
| self.setFlags(flags) | |||
| else: | |||
| self.resetFlags(text) | |||
| if flags_disabled: | |||
| for i in range(self.flagsLayout.count()): | |||
| self.flagsLayout.itemAt(i).widget().setDisabled(True) | |||
There was a problem hiding this comment.
Currently popUp only disables flag widgets when flags_disabled is true but never re-enables them when it's false, which can leave flags permanently disabled. Consider adding an else branch to call setEnabled(True) on each widget.
| self.flagsLayout.itemAt(i).widget().setDisabled(True) | |
| self.flagsLayout.itemAt(i).widget().setDisabled(True) | |
| else: | |
| for i in range(self.flagsLayout.count()): | |
| self.flagsLayout.itemAt(i).widget().setEnabled(True) |
Copilot uses AI. Check for mistakes.
| flags=None, | ||
| group_id=None, | ||
| description=None, | ||
| flags_disabled: bool = False, |
There was a problem hiding this comment.
The new flags_disabled parameter lacks documentation. Please update the popUp docstring to explain its purpose and default behavior.
Copilot uses AI. Check for mistakes.
|
Merged in #1595 |
Why?
Because flags widgets are dynamically created, so locking has to happen after creation.