Merge lp:~stefan-schwarzburg/qreator/fix_bug_1006635_selection into lp:~dpm/qreator/trunk

[Schwarzburg]

Asuming that you prefer the ListStore and IconView version, this implements a method to provide single click activation of qr_types_view by using the selection_changed signal of the IconView.

[David Planella (dpm)]

Thanks a lot for this work. I especially like the fact that it is a very non-intrusive change in terms of code modifications. I'm still not sure what to do with the IconView, though. Your other branch using buttons was very useful to gather data to assess if regular buttons might be an option, and after having tested it, as mentioned there, I believe the IconView is a better option.

However, while I thought the single click approach might be an improvement to solve the confusion several users were experiencing with te double click, after having seen it live, in the current implementation the interaction seems a bit unnatural, probably because what you were commenting already on the AskUbuntu question.

I wonder we could get to a state where the IconView behaves the same way as the elements in the System Parameters dialog [1], where I feel the UX experience is pretty good in terms of selection, feedback and activation. That is:

1. Click on an item
2. The item is selected and the user get a visual cue (i.e. they see for a split second that it is selected)
3. When going back to the list of QR codes by pressing the 'New' button there is no QR type selected

Do you think this behaviour could be implemented in Python?

[1] http://ubuntuone.com/78djtRLkkZ1ahBjHq61LVt

P.S. An issue that I've found is that on occasions (it cannot always be reproduced), the UI blocks on the QR types screen after clicking on the Software Center app type, rather than going to the QR code screen first. While this is not a blocker for this branch (I should properly fix bug 1006639 instead), I thought I'd mention it nevertheless.

review: Needs Information

[lp:~stefan-schwarzburg/qreator/fix_bug_1006635_selection]

86. By Schwarzburg on 2012-06-07

Basic idea behind this modification:

1) keep the IconView and the on_selection_changed signal
2) instead of directly switching to the QR code screen, shedule an "activate" function (with idle_add) of the QR code, when the activate function is ready, shedule the actual switching.
3) add a line that removes the selection when 'new' is clicked.

This is help to mimik the gnome-control-center behaviour.

[Schwarzburg (stefan-schwarzburg)]

If you prefer more time between the click and the switching, this would be very simple (timeout_add instead of the add_idle, I think).

Is this more what you had in mind? I think it behaves very much like the gnome-control-center, although most actions are much faster...

[David Planella (dpm)]

Al 07/06/12 09:07, En/na Schwarzburg ha escrit:
> If you prefer more time between the click and the switching, this would be very simple (timeout_add instead of the add_idle, I think).
>
> Is this more what you had in mind? I think it behaves very much like the gnome-control-center, although most actions are much faster...

I think the timeout could help in making the transition less rushed, and
I assume it would have the benefit of the user actually seeing that the
icon is selected (i.e. seeing the big orange square surrounding it)
before he/she is taken to the next screen.

But what I also think it's important is that the icon is unselected when
going back to the 'New' screen with the QR code type choices (i.e. point
3 in the previous reply). Do you think that could be possible as well?

Thanks!

[Schwarzburg (stefan-schwarzburg)]

The point 3 (unselection) is already implemented in rev86, so you can test that already.

The timeout is implemented here on my local branch, but since a few minutes, I can no longer push to launchpad... So that will come later.

[lp:~stefan-schwarzburg/qreator/fix_bug_1006635_selection]

87. By Schwarzburg on 2012-06-07

added a timeout instead of the idle_add.

[David Planella (dpm)]

Hi Stefan,

I pulled rev. 87 and I'm quite happy with how it's working now, it's exactly what I had in mind. That's excellent workk, thanks!

Let me know if you're done with the branch and if so I'll merge it in.

As a side note, and as I said earlier, that makes bug 1006639 much more evident, but it's got nothing to do with this feature, which looks good to go now.

[Schwarzburg (stefan-schwarzburg)]

Glad you like the result :-)
I would say that this branch is good to be merged.

As for bug 1006639: for me, the window freezes mainly on the QR Code (not the QR type) window. Not sure why, but I doubt that it is based on the modifications in this branch.

[David Planella (dpm)]

Excellent, thanks! I've just merged it with a minor change: I made the timeout a property of the main window and decreased to 200 milliseconds to increase UI responsiveness. We might have to play with it and revert it to 400 if that is too fast for some users.

review: Approve

=== modified file 'data/ui/QreatorWindow.ui'
--- data/ui/QreatorWindow.ui	2012-06-02 09:17:40 +0000
+++ data/ui/QreatorWindow.ui	2012-06-07 07:42:21 +0000
@@ -193,6 +193,7 @@
                     <property name="visible">True</property>
                     <property name="can_focus">True</property>
                     <property name="hexpand">True</property>
+                    <signal name="selection-changed" handler="on_qr_types_view_selection_changed" swapped="no"/>
                   </object>
                 </child>
               </object>

=== modified file 'qreator/QreatorWindow.py'
--- qreator/QreatorWindow.py	2012-06-02 09:17:40 +0000
+++ qreator/QreatorWindow.py	2012-06-07 07:42:21 +0000
@@ -16,7 +16,7 @@
 
 import cairo
 import math
-from gi.repository import Gtk, Gdk, GdkPixbuf  # pylint: disable=E0611
+from gi.repository import Gtk, Gdk, GdkPixbuf, GObject  # pylint: disable=E0611
 import logging
 logger = logging.getLogger('qreator')
 
@@ -166,6 +166,7 @@
 
     def on_toolbuttonNew_clicked(self, widget, data=None):
         '''Shows the home page'''
+        self.ui.qr_types_view.unselect_all()
         self.ui.notebook1.set_current_page(PAGE_NEW)
 
     def on_toolbuttonHistory_clicked(self, widget, data=None):
@@ -177,6 +178,10 @@
         self.ui.notebook1.set_current_page(PAGE_ABOUT)
 
 ##########################################
+    def on_qr_types_view_selection_changed(self, widget):
+        if len(widget.get_selected_items()) > 0:
+            GObject.idle_add(self.on_qr_types_view_item_activated,
+                             widget, widget.get_selected_items()[0])
 
     def on_qr_types_view_item_activated(self, widget, item):
         '''Loads the UI for the appropriate QR type'''
@@ -185,12 +190,16 @@
         qr_id = model[item][COL_ID]
         self.qr_types[qr_id].widget.on_activated()
 
-        self.ui.notebook1.set_current_page(PAGE_QR)
+        def switch_callback(page):
+            self.ui.notebook1.set_current_page(page)
+            return False  # stop this callback from being called again
+        GObject.timeout_add(400, switch_callback, PAGE_QR) # timeout in milliseconds
 
         for child in self.ui.qr_input_box.get_children():
             child.hide()
         self.ui.qr_input_box.get_children()[qr_id].show()
 
+
 ##########################################
 
     def get_pixbuf_from_drawing_area(self):