Merge lp:~brendan-donegan/checkbox/bug971181_next_button into lp:checkbox

[Brendan Donegan]

Previously, when a test was run by clicking the 'Test' button in the Qt UI, and the appropriate 'Yes'/'No' result button was selected, if the user clicked on Next then they would still be warned about skipping the test. This seemed to be a little bit unintuitive for some people. I made a few changes to fix this so that if Next is pressed then the current state of the Yes/No buttons will be taken into account.

My concern here is that this fix feels like hackery, because we're essentially turning the push buttons into a sort of radio button.

I'd like some feedback on this proposed fix, to see if the approach is agreeable.

[Ara Pulido (ara)]

This wouldn't work in the cases where a test is manual, but you click Test to perform an action.

Example:

On the External Audio test, when you click on Test, Yes is automatically selected (as the script returns 0).

So, this does not seem like a solution for every case.

How many tests do we have that are semi-automated? I am starting to think that we may need to change the descriptions to match the new UI.

review: Needs Fixing

[Jeff Lane  (bladernr)]

Per discussion during the meeting, rejecting this... however, while I have not tested this to verify that Ara's concerns are valid, I like the idea at least so please hold onto this for either an SRU or for 0.14.

I'm thinking maybe we could expand this and just get rid of the Next button alltogether. Clicking Yes or No would initiate the move to the next test instead... <- just a thought at least but I do like the idea of trying to clean up the UI a bit...

So I've merged Daniel's branch based on the discussion from the team meeting, so lets see if that clears up the confusion of if we really do need to revisit the buttons on the new UI later.

review: Disapprove

Unmerged revisions

1356. By Brendan Donegan on 2012-04-11

Fixed UI so that if the Yes/No button is highlighted automatically then the
appropriate result is marked for the test when Next is clicked (LP: #971181)

=== modified file 'checkbox_qt/qt_interface.py'
--- checkbox_qt/qt_interface.py	2012-03-22 16:33:36 +0000
+++ checkbox_qt/qt_interface.py	2012-04-11 08:46:19 +0000
@@ -194,8 +194,14 @@
             self._run_test(test, runner)
 
         def onNextTestClicked():
-            test["status"] = ANSWER_TO_STATUS[SKIP_ANSWER]
+            if self.qtiface.getFocusTestYes():
+                test["status"] = ANSWER_TO_STATUS[YES_ANSWER]
+            elif self.qtiface.getFocusTestNo():
+                test["status"] = ANSWER_TO_STATUS[NO_ANSWER]
+            else:
+                test["status"] = ANSWER_TO_STATUS[SKIP_ANSWER]
             self.direction = NEXT
+            self.qtiface.clearFocusTestYesNo()
             self.loop.quit()
 
         def onPreviousTestClicked():

=== modified file 'debian/changelog'
--- debian/changelog	2012-04-11 07:12:00 +0000
+++ debian/changelog	2012-04-11 08:46:19 +0000
@@ -18,6 +18,10 @@
      made during the cert sprint.
 
 
+  [Brendan Donegan]
+  * Fixed UI so that if the Yes/No button is highlighted automatically then the
+    appropriate result is marked for the test when Next is clicked (LP: #971181)
+
  -- Jeff Lane <[e-mail]>  Mon, 09 Apr 2012 17:12:51 -0400
 
 checkbox (0.13.6) precise; urgency=low

=== modified file 'qt/frontend/qtfront.cpp'
--- qt/frontend/qtfront.cpp	2012-04-09 21:15:16 +0000
+++ qt/frontend/qtfront.cpp	2012-04-11 08:46:19 +0000
@@ -181,7 +181,7 @@
 
 void QtFront::onNextTestClicked()
 {
-    if (!m_skipTestMessage) {
+    if (!m_skipTestMessage && !getFocusTestYes() && !getFocusTestNo()) {
         QMessageBox msgBox(QMessageBox::Question, checkboxTr("Are you sure?", 0), checkboxTr("Do you really want to skip this test?", 0), 0, ui->tabWidget);
         QCheckBox dontPrompt(checkboxTr("Don't ask me again", 0), &msgBox);
         dontPrompt.blockSignals(true);
@@ -375,6 +375,24 @@
     }
 }
 
+void QtFront::clearFocusTestYesNo()
+{
+    ui->noTestButton->setDefault(false);
+    ui->noTestButton->clearFocus();
+    ui->yesTestButton->setDefault(false);
+    ui->yesTestButton->clearFocus();
+}
+
+bool QtFront::getFocusTestYes()
+{
+    return ui->yesTestButton->isDefault();
+}
+
+bool QtFront::getFocusTestNo()
+{
+    return ui->noTestButton->isDefault();
+}
+
 void QtFront::updateTestStatus(QStandardItem *item, QString status)
 {
     int numRows = item->rowCount();

=== modified file 'qt/frontend/qtfront.h'
--- qt/frontend/qtfront.h	2012-04-03 11:38:10 +0000
+++ qt/frontend/qtfront.h	2012-04-11 08:46:19 +0000
@@ -55,6 +55,11 @@
     void showTest(QString purpose, QString steps, QString verification, QString info, QString comment, QString testType, QString testName, bool enableTestButton);
     void showTestControls(bool enableTestControls);
     void setFocusTestYesNo(bool status);
+    void clearFocusTestYesNo();
+
+    bool getFocusTestYes();
+    bool getFocusTestNo();
+
     Q_NOREPLY void showInfo(QString text, QStringList options, QString defaultoption);
     void setUiFlags(QVariantMap flags);
     void updateAutoTestStatus(QString status, QString testName);