superqt icon indicating copy to clipboard operation
superqt copied to clipboard

QCheckComboBox for Easy Multiple Items Selection

Open MosGeo opened this issue 3 years ago β€’ 15 comments

This implements the QCheckComboBox that is discussed in #89. A few notes:

  • addItems and addItem included checked parameters to quickly initializes the items while adding. We can remove that if you feel that it doesn't follow the "zen" of qt.
  • Quite a few convenience functions are added to the widget to retrieve and control the items, e.g., get the indices of all checked items, set checked states of all items.
  • The label of the combobox has two modes: 1) static set by the user or a list of all selected items. The example included showcases this feature.
  • Still getting to know qtbot for actual interface testing. I was planning on using it for _handleItemPressed test but I failed. For now, it is being tested like any other function.

As always, I am open to suggestions and modifications. In terms of functionality, this is what I needed for my use case but others might have other ideas.

p.s. I see that the list of comboboxes in superqt is growing (enum, search) which is great.

MosGeo avatar Jun 09 '22 22:06 MosGeo

@tlambert03 I think this is ready for your initial review. The example included is a good start.

  • For checkedIndices and uncheckedIndices, I was using model match but it wasn't working in version 6. For now, I did it in a Python for loop instead of using the match function.
  • For macos, I am not sure what is happening. I am not sure if the issue is just in the test_paint_event or the widget actually crashes too. I do not have a macos to test on. Any advise?

MosGeo avatar Jun 13 '22 15:06 MosGeo

@tlambert03 Thanks for checking and no problem on waiting. This is a low priority thing anyway.

Use of Check Combobox

Check Combobox is pretty standard in control libraries nowadays. It goes by many names though (multiple select comobobox, check combobox, ...). Here are some examples for different programming languages:

  • In Javascript (web application), here is an example for the framework Vue. The Vuetify combobox allows for multiple selection through the use of check comobox (https://vuetifyjs.com/en/components/combobox/#multiple-combobox)
  • In C# world, again multiple combobox (https://www.syncfusion.com/winforms-ui-controls/combobox/multiselect-combobox). This example is using winforms framework which has been around since early 2000s. Other newer frameworks (WPF, UWP, and WinUI) have equivalents.

Some notes:

  • Some implementations allows the user to type to filter the list (similar to autocomplete behavior).
  • Some implementations allows the user to remove items directly from the main list/label of the combobox (each item has an x button) (https://vuetifyjs.com/en/components/combobox/#advanced-custom-options)

Double Click Issue

I agree that it is a weird behavior. I didn't catch it while testing as i always close it by clicking outside the box. It is actually a remanent of the original comobox. It shouldn't behave that way. I suggest I remove the behavior and make it do what is expected: double clicking on the item should check/uncheck twice. That is the behavior that is usually implemented in other ones (https://vuetifyjs.com/en/components/combobox/#multiple-combobox)

MosGeo avatar Jul 04 '22 04:07 MosGeo

@tlambert03 I think the behavior is now much better. Double click does not close the pop-up/run the item.

I still have the issue of passing the test with macos. I don't know what to do with that as I do not have a macos!

MosGeo avatar Jul 06 '22 08:07 MosGeo

Hi @MosGeo thanks for working on this. To answer @tlambert03 question, yes I have seen this used in other places and I think it is useful now that the double click behavior has been fixed :) πŸš€

I agree with @Czaki suggestions also. It would make the implementation complete and much better

Thanks a lot!

goanpeca avatar Jul 06 '22 15:07 goanpeca

yeah, once you provided the links to the other examples, I get it better :joy:

It's kinda the same thing as the labels selector in github: Screen Shot 2022-07-06 at 11 03 19 AM

I think the primary things that confused me were the double click issue, and what seemed like an inability to "escape" the dropdown easily. (some of the examples you provided have OK/Cancel buttons which I think helped). Another nice thing that most of these "token selector" dropdowns have is some way to show what's currently selected (i.e. tokens with little "x" to remove it). That could come in a later PR, but that's the bit that made me realize what this was really going for.

thanks again @MosGeo

tlambert03 avatar Jul 06 '22 15:07 tlambert03

@tlambert03 base on your comment maybe we should also add a searchable and checkable combo box also?

And to this PR. There should be also the following method (inspired by currentText):

    def checkedTexts(self) -> List[str]:
        """Returns the checked indices"""
        texts = []
        for i in range(self.count()):
            item = self.model().item(i)
            if item.checkState() == Qt.Checked:
                texts.append(item.text())
        return texts

Czaki avatar Jul 06 '22 23:07 Czaki

some typing and namespace suggestions

diff --git a/src/superqt/combobox/_check_combobox.py b/src/superqt/combobox/_check_combobox.py
index a95e3e0..0149846 100644
--- a/src/superqt/combobox/_check_combobox.py
+++ b/src/superqt/combobox/_check_combobox.py
@@ -1,8 +1,8 @@
 from enum import Enum, auto
-from typing import Any, List, Union
+from typing import Any, List, Union, cast
 
-from qtpy.QtCore import QEvent, Qt
-from qtpy.QtGui import QStandardItem
+from qtpy.QtCore import QEvent, QModelIndex, Qt
+from qtpy.QtGui import QStandardItem, QStandardItemModel
 from qtpy.QtWidgets import QComboBox, QStyle, QStyleOptionComboBox, QStylePainter
 
 
@@ -24,10 +24,14 @@ class QCheckComboBox(QComboBox):
     def __init__(self) -> None:
         """Initializes the widget"""
         super().__init__()
-        self.view().pressed.connect(self._handleItemPressed)
-        self.view().doubleClicked.connect(self._handleItemPressed)
+        self.view().pressed.connect(self._handleItemPressed)  # type: ignore
+        self.view().doubleClicked.connect(self._handleItemPressed)  # type: ignore
         self._changed = False
 
+    def model(self) -> QStandardItemModel:
+        # this is true, but annotated incorrectly in pyside2
+        return cast(QStandardItemModel, super().model())
+
     def _update_label_text_with_selected_items(self) -> None:
         checked_indices = self.checkedIndices()
         selected_text_list = []
@@ -53,13 +57,13 @@ class QCheckComboBox(QComboBox):
         """Returns label type"""
         return self._label_type
 
-    def _handleItemPressed(self, index: int) -> None:
+    def _handleItemPressed(self, index: QModelIndex) -> None:
         """Updates item checked status"""
         item = self.model().itemFromIndex(index)
-        if item.checkState() == Qt.Checked:
-            item.setCheckState(Qt.Unchecked)
+        if item.checkState() == Qt.CheckState.Checked:
+            item.setCheckState(Qt.CheckState.Unchecked)
         else:
-            item.setCheckState(Qt.Checked)
+            item.setCheckState(Qt.CheckState.Checked)
 
         if self._label_type == QCheckComboBox.QCheckComboBoxLabelType.SELECTED_ITEMS:
             self._update_label_text_with_selected_items()
@@ -73,7 +77,7 @@ class QCheckComboBox(QComboBox):
         self.setItemChecked(self.count() - 1, checked=checked)
         if (
             self._label_type == QCheckComboBox.QCheckComboBoxLabelType.SELECTED_ITEMS
-            and checked is True
+            and checked
         ):
             self._update_label_text_with_selected_items()
 
@@ -89,7 +93,7 @@ class QCheckComboBox(QComboBox):
 
         if (
             self._label_type == QCheckComboBox.QCheckComboBoxLabelType.SELECTED_ITEMS
-            and any(checked) is True
+            and any(checked)
         ):
             self._update_label_text_with_selected_items()
 
@@ -102,14 +106,15 @@ class QCheckComboBox(QComboBox):
     def itemChecked(self, index: int) -> bool:
         """Returns current checked state as boolean"""
         item: QStandardItem = self.model().item(index, self.modelColumn())
-        is_checked: bool = item.checkState() == Qt.Checked
-        return is_checked
+        return bool(item.checkState() == Qt.CheckState.Checked)
 
     def setItemChecked(self, index: int, checked: bool = True) -> None:
         """Sets the status"""
         item: QStandardItem = self.model().item(index)
         checked_state_old = item.checkState()
-        checked_state_new = Qt.Checked if checked else Qt.Unchecked
+        checked_state_new = (
+            Qt.CheckState.Checked if checked else Qt.CheckState.Unchecked
+        )
 
         # Stopping condition
         if checked_state_old == checked_state_new:
@@ -130,7 +135,7 @@ class QCheckComboBox(QComboBox):
         indecies = []
         for i in range(self.count()):
             item = self.model().item(i)
-            if item.checkState() == Qt.Checked:
+            if item.checkState() == Qt.CheckState.Checked:
                 indecies.append(i)
         return indecies
 
@@ -139,7 +144,7 @@ class QCheckComboBox(QComboBox):
         indecies = []
         for i in range(self.count()):
             item = self.model().item(i)
-            if item.checkState() == Qt.Unchecked:
+            if item.checkState() == Qt.CheckState.Unchecked:
                 indecies.append(i)
         return indecies
 
@@ -149,5 +154,5 @@ class QCheckComboBox(QComboBox):
         opt = QStyleOptionComboBox()
         self.initStyleOption(opt)
         opt.currentText = self._label_text
-        painter.drawComplexControl(QStyle.CC_ComboBox, opt)
-        painter.drawControl(QStyle.CE_ComboBoxLabel, opt)
+        painter.drawComplexControl(QStyle.ComplexControl.CC_ComboBox, opt)
+        painter.drawControl(QStyle.ControlElement.CE_ComboBoxLabel, opt)
diff --git a/tests/test_check_combobox.py b/tests/test_check_combobox.py
index c121cb5..0856ccb 100644
--- a/tests/test_check_combobox.py
+++ b/tests/test_check_combobox.py
@@ -99,7 +99,7 @@ def test_paint_event(qtbot: QtBot) -> None:
     """Simple test for paint event; execute without error"""
     check_combobox = QCheckComboBox()
     check_combobox.setLabelText("A new label")
-    check_combobox.paintEvent(QEvent(QEvent.Paint))
+    check_combobox.paintEvent(QEvent(QEvent.Type.Paint))
 
 
 def test_hidepopup(qtbot: QtBot) -> None:

tlambert03 avatar Jul 07 '22 20:07 tlambert03

ugh, sorry, that was ugly! πŸ˜‚ I was trying a feature in vscode that I've never tried before to suggest edits without having to do the whole suggestion thing. but it was supposed to give you a button to accept it. doesn't look like it though, so I'll just push it

tlambert03 avatar Jul 07 '22 20:07 tlambert03

Codecov Report

Patch coverage: 87.28% and project coverage change: -1.48 :warning:

Comparison is base (6ce87d4) 85.30% compared to head (0fae340) 83.82%.

:exclamation: Current head 0fae340 differs from pull request most recent head 6137bcb. Consider uploading reports for the commit 6137bcb to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
- Coverage   85.30%   83.82%   -1.48%     
==========================================
  Files          31       32       +1     
  Lines        2607     2721     +114     
==========================================
+ Hits         2224     2281      +57     
- Misses        383      440      +57     
Impacted Files Coverage Ξ”
src/superqt/combobox/_check_combobox.py 86.95% <86.95%> (ΓΈ)
src/superqt/__init__.py 93.75% <100.00%> (ΓΈ)
src/superqt/combobox/__init__.py 100.00% <100.00%> (ΓΈ)

... and 11 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Jul 07 '22 20:07 codecov[bot]

ugh... and now I've made a real mess :joy: I pushed my suggestions to main. I'm a little confused, and I think my vscode might also be, because it looks like you made these changes to your main branch. So i hope i didn't mess anything up for you. I'm going to stop trying to suggest/make edits now :)

one observation I had: when you uncheck an item, whatever the last item was gets immediately rechecked the next time you click on the dropdown (note that I'm not trying to re-check item 2 in the movie below)

https://user-images.githubusercontent.com/1609449/177870578-3bc12479-69db-4417-8a04-71ba32f341db.mov

tlambert03 avatar Jul 07 '22 21:07 tlambert03

as for the mac error, I just don't think it likes the way you're directly calling paint event. Do you need to do it that way? Can you perhaps call show() on the widget and then use .update() or something?

tlambert03 avatar Jul 07 '22 21:07 tlambert03

@MosGeo, did I scare you off with my botched attempt to make suggestions above? or are you just busy :)

I think this could go in, if we can just figure out what's going on with the reselection of stuff in https://github.com/napari/superqt/pull/91#issuecomment-1178225161?

tlambert03 avatar Jul 24 '22 15:07 tlambert03

Sorry, yah, no problem. I was just busy with other stuff. I have to find a day to work on this. All comments here are good.

Yes, I agree that is a big bug that I need to fix. My guess is that this was added when I introduced the fix to the double click. I felt it was a hacky way of doing it and I was not sure why it worked 🀣

I will work on this week and get back to you. I want to check on icon, signal and so on in other comments too.

For documentation, honestly, I didn't know that the md files existed. Is it something of interest now or should we wait for the proper documentation?

MosGeo avatar Jul 25 '22 06:07 MosGeo

For documentation, honestly, I didn't know that the md files existed. Is it something of interest now or should we wait for the proper documentation?

It's fine to add in a later PR. We desperately need real docs πŸ™ƒ, the md files are completely undiscoverable

tlambert03 avatar Jul 25 '22 09:07 tlambert03

ugh... and now I've made a real mess πŸ˜‚ I pushed my suggestions to main. I'm a little confused, and I think my vscode might also be, because it looks like you made these changes to your main branch. So i hope i didn't mess anything up for you. I'm going to stop trying to suggest/make edits now :)

one observation I had: when you uncheck an item, whatever the last item was gets immediately rechecked the next time you click on the dropdown (note that I'm not trying to re-check item 2 in the movie below)

Untitled.mov

@tlambert03 I am having issues reproducing. What Qt backend are you using? What OS?

MosGeo avatar Jul 27 '22 04:07 MosGeo