[Sugar-devel] [DESIGN] Proposal: Multi-Selection and Batch Operations on Journal entries

[Simon Schampijer]

On 08/19/2012 10:05 PM, Ajay Garg wrote:
> Hi all.
>
> Just saw Simon's patch at http://patchwork.sugarlabs.org/patch/1670/
>
> Thereby, it's a gentle request ::
>
>                    * to get this multi-select feature approved (with all the
> latest feedback-changes/bug-fixes).
>                    * to get the multi-select patch(es) reviewed.
>                    * to get the multi-select patch(es) pushed (if all ok).
>
>
> so that any such enhancements (confirmation alert before erasing, currently
> in single-mode) can be easily merged (for single- AND batch-mode); and
> there is minimal slogging involved that comes in with merging patches :)
>
>
> Thanks and Regards,
> Ajay



Hi Ajay,

first of all thanks for working on this. Yes, Sugar is moving fast those 
days, you can apply your patch on top of 
3aa88a85143c48b64cf57ab7c79b4531dcfa6e8b (Release 0.97.1). You can keep 
working on top of that and rebase later so you don't have to keep up 
with master. In any case, you need to port them to gtk3 anyhow later.

First of all, I think it would be good to send the patches directly to 
the devel list (as for all I know they are currently only in patchwork) 
[1] . If I missed anything, please point me to it.

Please add detailed descriptions to the patch (see how I tried and 
hopefully managed to describe a big change we lately landed in [2]). Get 
to the point what the patch does. Add items that don't work yet or are 
critical or if you have questions or are unsure about the approach.

The good news is, the artwork and toolkit patches are easy to handle. 
The icons should be cleaned up and can be landed. For the toolkit patch 
I think you can use add_button fine:

diff --git a/src/jarabe/journal/journalactivity.py 
b/src/jarabe/journal/journalactivity.py
index 7ed8b18..060416b 100644
--- a/src/jarabe/journal/journalactivity.py
+++ b/src/jarabe/journal/journalactivity.py
@@ -132,16 +132,17 @@ class JournalActivity(JournalWindow):
          self._editing_mode = False
          self._alert = Alert()

-        self._error_alert = NButtonAlert()
-        self._error_alert._populate_buttons([
-            ('dialog-ok', gtk.RESPONSE_OK, _('Ok'))
-                                            ])
-
-        self._confirmation_alert = NButtonAlert()
-        self._confirmation_alert._populate_buttons([
-            ('dialog-cancel', gtk.RESPONSE_CANCEL, _('Stop')),
-            ('dialog-ok', gtk.RESPONSE_OK, _('Continue'))
-                                                   ])
+        self._error_alert = Alert()
+        icon = Icon(icon_name='dialog-ok')
+        self._error_alert.add_button(gtk.RESPONSE_OK, _('Ok'), icon)
+
+        self._confirmation_alert = Alert()
+        icon = Icon(icon_name='dialog-cancel')
+        self._confirmation_alert.add_button(gtk.RESPONSE_CANCEL,
+                                            _('Stop'), icon)
+        icon = Icon(icon_name='dialog-ok')
+        self._confirmation_alert.add_button(gtk.RESPONSE_OK,
+                                            _('Continue'), icon)

We could argue that the add_button API should allow to pass in an icon 
name instead of the icon directly, I don't think there is a case where 
this would break. On the other hand not sure changing that now. I would 
just go with the above and drop your toolkit patch.

So we are left with the shell patch basically. It is loong and invasive 
:) So it will take some time. More review eyes welcome. I wonder as well 
if splitting it up in sub-patches would help, but maybe is too hard at 
this point.

On the design end, the work flow is good. I guess the 'uifreeze' is 
meant to prevent races. Is the first time we do it in the UI. Might make 
sense here. I will think about it a bit more.

Cheers,
    Simon

[1] http://wiki.sugarlabs.org/go/Development_Team/Code_Review
[2] 
http://git.sugarlabs.org/sugar/mainline/commit/d9fbf9db79b9abab2491ebbdc169985e38a59055

PS: something that caught my eye in journalactivity.py:
{{{
  def __volume_error_cb(self, gobject, message, severity):
         alert = ErrorAlert(title=severity, msg=message)
         alert.connect('response', self.__alert_response_cb)
         self.add_alert(alert)
         alert.show()

     def _volume_error_cb(self, gobject, message, severity):
         self.update_error_alert(severity, message, None, None)
}}}