BUG: Make sure ctkButtonGroup acts before other widgets slots are called

by listening to the buttonClicked(int) signal rather than
buttonClicked(QAbstractButton*), ctkButtonGroup is first to catch the
signal and can uncheck the button if needed.

Libs/Widgets/Testing/Cpp/ctkButtonGroupTest1.cpp

@@ -20,7 +20,10 @@
 
 // Qt includes
 #include <QApplication>
+#include <QHBoxLayout>
 #include <QPushButton>
+#include <QSignalSpy>
+#include <QTimer>
 
 // CTK includes
 #include "ctkButtonGroup.h"
@@ -35,10 +38,18 @@ int ctkButtonGroupTest1(int argc, char * argv [] )
 {
   QApplication app(argc, argv);
 
-  QPushButton* button1 = new QPushButton(0);
-  QPushButton* button2 = new QPushButton(0);
-  QPushButton* button3 = new QPushButton(0);
-  ctkCollapsibleButton* button4 = new ctkCollapsibleButton(0);
+  QWidget widget(0);
+  widget.show();
+  QHBoxLayout* layout = new QHBoxLayout(&widget);
+  QPushButton* button1 = new QPushButton(&widget);
+  layout->addWidget(button1);
+  QPushButton* button2 = new QPushButton(&widget);
+  layout->addWidget(button2);
+  QPushButton* button3 = new QPushButton(&widget);
+  layout->addWidget(button3);
+  ctkCollapsibleButton* button4 = new ctkCollapsibleButton(&widget);
+  layout->addWidget(button4);
+  widget.setLayout(layout);
 
   button1->setCheckable(true);
   button2->setCheckable(true);
@@ -49,7 +60,6 @@ int ctkButtonGroupTest1(int argc, char * argv [] )
   button4->setChecked(true);
   
   ctkButtonGroup buttonGroup(0);
-  //QButtonGroup buttonGroup(0);
 
   buttonGroup.addButton(button1);
   buttonGroup.addButton(button2);
@@ -128,7 +138,40 @@ int ctkButtonGroupTest1(int argc, char * argv [] )
               << button2->isChecked() << std::endl;
     return EXIT_FAILURE;
     }
-
-  return EXIT_SUCCESS;
+  qRegisterMetaType<QAbstractButton*>("QAbstractButton*");
+  QSignalSpy spy(&buttonGroup, SIGNAL(buttonClicked(QAbstractButton*)));
+  QSignalSpy spyInt(&buttonGroup, SIGNAL(buttonClicked(int)));
+  button1->click();
+  if (spy.count() != 1 || spyInt.count() != 1)
+    {
+    std::cerr << "ctkButtonGroup::click7 failed"
+              << button1->isChecked() << ", "
+              << spy.count() << "clicks" << std::endl;
+    return EXIT_FAILURE;
+    }
+  button4->click();
+  if (spy.count() != 2 || spyInt.count() != 2)
+    {
+    std::cerr << "ctkButtonGroup::click8 failed"
+              << button4->isChecked() << ", "
+              << spy.count() << "clicks" << std::endl;
+    return EXIT_FAILURE;
+    }
+  button4->click();
+  if (spy.count() != 3 || spyInt.count() != 3)
+    {
+    std::cerr << "ctkButtonGroup::click9 failed"
+              << button4->isChecked() << ", "
+              << spy.count() << "clicks" << std::endl;
+    return EXIT_FAILURE;
+    }
+  QTimer autoExit;
+  if (argc < 2 || QString(argv[1]) != "-I")
+    {
+    std::cout << argc << argv[1] << std::endl;
+    QObject::connect(&autoExit, SIGNAL(timeout()), &app, SLOT(quit()));
+    autoExit.start(500);
+    }
+  return app.exec();
 }
 

Libs/Widgets/ctkButtonGroup.cpp

@@ -39,27 +39,39 @@ ctkButtonGroup::ctkButtonGroup(QObject* _parent)
   :QButtonGroup(_parent)
 {
   CTK_INIT_PRIVATE(ctkButtonGroup);
-  connect(this, SIGNAL(buttonClicked(QAbstractButton*)), this, SLOT(onButtonClicked(QAbstractButton*)));
-  connect(this, SIGNAL(buttonPressed(QAbstractButton*)), this, SLOT(onButtonPressed(QAbstractButton*)));
+  CTK_D(ctkButtonGroup);
+  d->IsLastButtonPressedChecked = false;
+  // we need to connect to button{Clicked,Pressed}(int) instead of
+  // button{Clicked,Pressed}(QAbstractButton*) in order to be first to catch the
+  // signals
+  connect(this, SIGNAL(buttonClicked(int)), this, SLOT(onButtonClicked(int)));
+  connect(this, SIGNAL(buttonPressed(int)), this, SLOT(onButtonPressed(int)));
 }
 
 //------------------------------------------------------------------------------
-void ctkButtonGroup::onButtonClicked(QAbstractButton *clickedButton)
+void ctkButtonGroup::onButtonClicked(int buttonId)
 {
   CTK_D(ctkButtonGroup);
+  QAbstractButton* clickedButton = this->button(buttonId);
+  Q_ASSERT(clickedButton);
   if (!this->exclusive() || !d->IsLastButtonPressedChecked)
     {
     return;
     }
+  // here the button is clicked and we click it again... so we want to
+  // uncheck, a behavior not supported by QButtonGroup.
+  // The only way to uncheck the button is to remove it from the group, and put it back
   this->removeButton(clickedButton);
   clickedButton->setChecked(false);
   this->addButton(clickedButton);
+  d->IsLastButtonPressedChecked = false;
 }
 
 //------------------------------------------------------------------------------
-void ctkButtonGroup::onButtonPressed(QAbstractButton *pressedButton)
+void ctkButtonGroup::onButtonPressed(int buttonId)
 {
   CTK_D(ctkButtonGroup);
+  QAbstractButton* pressedButton = this->button(buttonId);
   Q_ASSERT(pressedButton);
   d->IsLastButtonPressedChecked = pressedButton->isChecked();
 }

Libs/Widgets/ctkButtonGroup.h

@@ -52,8 +52,8 @@ public:
   explicit ctkButtonGroup(QObject *_parent = 0);
 
 protected slots:
-  void onButtonClicked(QAbstractButton * button);
-  void onButtonPressed(QAbstractButton * button);
+  void onButtonClicked(int button);
+  void onButtonPressed(int button);
 
 private:
   CTK_DECLARE_PRIVATE(ctkButtonGroup);