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

[Ajay Garg]

Simon,

Mailed the (version-2) patch.

Patchwork:
http://patchwork.sugarlabs.org/patch/1707/

Sugar-Devel:
http://lists.sugarlabs.org/archive/sugar-devel/2012-August/039338.html


Note that I had sent the patch with "--in-reply-to=
1345225428-12572-1-git-send-email-ajay at activitycentral.com" ( that is, in
continuation with the patch at http://patchwork.sugarlabs.org/patch/1662/ )
but apparently, the effect did not take place :-\


Regards,
Ajay

On Tue, Aug 28, 2012 at 4:38 PM, Ajay Garg <ajay at activitycentral.com> wrote:

> Hi Simon.
>
> Thanks for the reply !!
>
> Please find my comments inline.
>
>
> On Tue, Aug 28, 2012 at 3:13 PM, Simon Schampijer <simon at schampijer.de>wrote:
>
>> On 08/19/2012 10:05 PM, Ajay Garg wrote:
>>
>>> Hi all.
>>>
>>> Just saw Simon's patch at http://patchwork.sugarlabs.**org/patch/1670/<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 3aa88a85143c48b64cf57ab7c79b45**31dcfa6e8b
>> (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.
>>
>
> Okies..
>
>
>
>
>
>>
>> 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.
>>
>
> I had already sent the patches to sugar-devel earlier :) at ::
>
> http://lists.sugarlabs.org/archive/sugar-devel/2012-August/039054.html
> http://lists.sugarlabs.org/archive/sugar-devel/2012-August/039055.html
> http://lists.sugarlabs.org/archive/sugar-devel/2012-August/039056.html
>
>
> As per your suggestion, I will be changing, and soon posting the changed,
> version-2 sugar-patch (replying on that same thread), that would do away
> with the need of the sugar-toolkit patch :)
>
>
>
>
>
>
>>
>> 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.
>>
>
> Ok, I think the bulk of the work happens in the different, segregated out
> stages during a particular metadata-processing. I took care of being
> verbose there as far as possible :)
>
> Anyhow, in my version-2 patch (that would do away with the need of the
> sugar-tollkit patch, as pointed by you), I will also try my hand to have a
> better, unified description :)
>
>
>
>
>
>
>>
>> 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.
>>
>
> +1.
>
>
>
>
>
>>
>> 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.
>>
>
> Hmm.. Yes, there is a tendency for things to break.
> I will be posting with a better, unified description in version-2 patch.
>
>
>
>>
>> On the design end, the work flow is good. I guess the 'uifreeze' is meant
>> to prevent races.
>
>
>
> +1.
> Gary's aggressive testing produced plenty of those :D
>
> So, freezing the UI was the only (and actually, simple and correct)
> approach.
>
>
>
>
>> 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<http://wiki.sugarlabs.org/go/Development_Team/Code_Review>
>> [2] http://git.sugarlabs.org/**sugar/mainline/commit/**
>> d9fbf9db79b9abab2491ebbdc16998**5e38a59055<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)
>> }}}
>>
>
> This would be called in the "journal._volume_error_cb(menu_item, message,
> severity)" in "CopyMenuHelper" class in "src/jarabe/journal/palettes.py".
> Since "CopyMenuHelper" is a singleton class, and is NOT attached anywhere
> to the singleton "JournalActivity" widget, thus, there is no way to call
> the private "__volume_error_cb" method of "JournalActivity" widget directly.
>
>
>
>
> Thanks and Regards,
> Ajay
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20120828/70b2f541/attachment-0001.html>