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

[Ajay Garg]

Hi Simon.

Any latest status update? :P


Thanks and Regards,
Ajay


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

> 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/20120905/a17548b4/attachment.html>