-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PR: Add clear button to find comboboxes #5537
Conversation
@@ -135,6 +135,8 @@ class PatternComboBox(BaseComboBox): | |||
def __init__(self, parent, items=None, tip=None, | |||
adjust_to_minimum=True): | |||
BaseComboBox.__init__(self, parent) | |||
if hasattr(self.lineEdit(), 'setClearButtonEnabled'): # only Qt >= 5.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this only works for Qt > = 5.2
, you need to enclose the lines you added in
if PYQT5:
...
where PYQT5
can be imported like this
from qtpy import PYQT5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The if hasattr()
serves exactly that purpose: Only use this feature if available. This should work without explicity checking a Qt version.
Also, I would assume that a simple if PYQT5
would be True
for 5.0 <= Qt < 5.2, but they still don't have the clear button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, you're right, hasattr
should be enough.
Besides, we only support Qt 5.2+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides, we only support Qt 5.2+
In that case, just self.lineEdit().setClearButtonEnabled(True)
would be enough.
The if
is unneccesary clutter then. Should I remove it for brevity and clarity of the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nop, because we also support PyQt4.
@dalthviz, please review that this is working locally for you. |
@ccordoba12 I tested locally and worked for me 👍 (tested in PyQt4 and PyQt5) |
Thanks @dalthviz! |
Fixes #5373