On Fri, Mar 2, 2012 at 7:39 PM, Daniel Juyung Seo <seojuyu...@gmail.com> wrote:
> Thanks for your patch.
> Everything is fine but I still see the floating item bug.
Could you tell me how to reproduce?
i tried several times but i can't see it
>
> And if the item is on dragging, it should not be unrealized.
> As far as I know, when an item is on dragging, unrealizing the item
> will cause unexpected problems.
> I don't remember what it is exactly.
> So I politely recommend you to solve this bug with another solution.
dragging is enabled by mouse_move (except group item)
If then how about to define flipped item would not draggable?
Or as i think we should restrict to only one flipped mode item.
What do you think so?
>
> Thanks.
>
> Daniel Juyung Seo (SeoZ)
>
> On Fri, Mar 2, 2012 at 5:14 PM, Hyoyoung Chang <hyoyo...@gmail.com> wrote:
>> I attached improved version of flip patch
>>
>> Thanks
>>
>> On Thu, Mar 1, 2012 at 10:52 PM, Hyoyoung Chang <hyoyo...@gmail.com> wrote:
>>> First of all, thanks for detailed review
>>>
>>> On Thu, Mar 1, 2012 at 3:01 PM, Daniel Juyung Seo <seojuyu...@gmail.com>
>>> wrote:
>>>> Thanks Hyoyoung, overall it looks good :)
>>>> I have some comments.
>>>>
>>>> 1. diff option
>>>> Create a diff with -x -up optoin.
>>>> It shows c function name for each changes.
>>>> This makes review so much easier and faster.
>>>> $ svn diff -x -up *.c
>>>>
>>>> 2. remove default properties from edc.
>>>> - mouse_events: 1
>>>> - rel1.relative: 0 0
>>>> - rel2.relative: 1 1
>>>>
>>>> 3. use elm_win_util_standard_add
>>>> elm_win_add + elm_bg_add is ok but I encourage you to use
>>>> elm_win_util_standard_add in normal cases.
>>>> It does elm_win_add + elm_bg_add and reduces 6 lines of code.
>>> I didn't know it. I'll apply
>>>>
>>>> 4. formatting
>>>> There are some wrong formatted codes.
>>> In or edc?
>>>>
>>>> 5. build warnings
>>>> test_genlist.c: In function ‘_flip_icon_clicked_cb’:
>>>> test_genlist.c:2495:82: warning: unused parameter ‘event_info’
>>>> test_genlist.c: In function ‘gl16_content_get’:
>>>> test_genlist.c:2516:17: warning: unused variable ‘ic’
>>>> test_genlist.c: In function ‘test_genlist16’:
>>>> test_genlist.c:2553:39: warning: unused variable ‘bt’
>>>> test_genlist.c:2553:33: warning: unused variable ‘bx2’
>>>
>>> Yeah, It should be removed
>>>>
>>>> 6. In _flip_icon_clicked_cb, you can just do:
>>>> tit->checked = !tit->checked
>>>> However, I would recommend to use elm_genlist_item_flip_get() instead
>>>> of remembering tit->checked.
>>>> This covers more API usage and it's useful example.
>>>> And without checked, you don't even need to use tit array, you can
>>>> just pass "(void *)(long) i" as an item data.
>>>> If you don't prefer to use that casting trick, you can just use tit
>>>> but without checked member.
>>> U're right :)
>>>>
>>>> 7. const to getters.
>>>> Yes, add "const" to getters.
>>>> EAPI Eina_Bool elm_genlist_item_flip_get(const Elm_Object_Item *it);
>>> Yeah i'll add it
>>>>
>>>> 8. in elm_genlist_item_flip_set
>>>> Instead of
>>>> _it->flipped = EINA_TRUE / EINA_FALSE;
>>>> You can do
>>>> _it->flipped = flip;
>>>>
>>>> 9. floating item bug
>>>> a. flip many items
>>>> b. scroll
>>>> Then I can see floating items bug. I attached screenshot.
>>>>
>>>> This also happens easily in this case:
>>>> a. flip many items
>>>> b. drag flipped item and scroll it out of viewport.
>>>>
>>>> 10. if statement in elm_genlist_item_flip_set
>>>> You already checked if _it->flipped equals to flip,
>>>> so you do not need to check _it->flipped again.
>>>>
>>>> if (_it->flipped == flip) return; <---- you already checked it
>>>> if (flip)
>>>> else
>>>> {
>>>> if (_it->flipped) <---- do not need to check this
>>>> else
>>>> }
>>> First checking is to verify changed.
>>> Second checking is about previous status.
>>> So It leaves at it is.
>>> I don't think it's good or clean code either.
>>> But checking is right. Maybe it should be re-written to make clean code.
>>>>
>>>> These are just trivial comments.
>>>> Please apply them.
>>>> Otherwise, this patch looks good :)
>>>> Thanks.
>>> you're always passionate.
>>> Thanks very much for detail review.
>>>
>>>>
>>>> Daniel Juyung Seo (SeoZ)
>>>>
>>>> On Wed, Feb 29, 2012 at 9:17 PM, Hyoyoung Chang <hyoyo...@gmail.com> wrote:
>>>>> Dear all.
>>>>>
>>>>> I made a patch to introduce new genlist item mode.
>>>>> Two public apis are added.
>>>>> +EAPI void elm_genlist_item_flip_set(Elm_Object_Item *it, Eina_Bool flip);
>>>>> +EAPI Eina_Bool elm_genlist_item_flip_get(Elm_Object_Item *it);
>>>>>
>>>>> It provides on-the-flying item change. It works like that a new item
>>>>> added on existed item.
>>>>> In elementary test, you can test it.
>>>>> It's useful at adding widgets or show buttons in genlist item.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> Virtualization & Cloud Management Using Capacity Planning
>>>>> Cloud computing makes use of virtualization - but cloud computing
>>>>> also focuses on allowing computing to be delivered as a service.
>>>>> http://www.accelacomm.com/jaw/sfnl/114/51521223/
>>>>> _______________________________________________
>>>>> enlightenment-devel mailing list
>>>>> enlightenment-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Virtualization & Cloud Management Using Capacity Planning
>>>> Cloud computing makes use of virtualization - but cloud computing
>>>> also focuses on allowing computing to be delivered as a service.
>>>> http://www.accelacomm.com/jaw/sfnl/114/51521223/
>>>> _______________________________________________
>>>> enlightenment-devel mailing list
>>>> enlightenment-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>>>>
>>
>> ------------------------------------------------------------------------------
>> Virtualization & Cloud Management Using Capacity Planning
>> Cloud computing makes use of virtualization - but cloud computing
>> also focuses on allowing computing to be delivered as a service.
>> http://www.accelacomm.com/jaw/sfnl/114/51521223/
>> _______________________________________________
>> enlightenment-devel mailing list
>> enlightenment-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>>
>
> ------------------------------------------------------------------------------
> Virtualization & Cloud Management Using Capacity Planning
> Cloud computing makes use of virtualization - but cloud computing
> also focuses on allowing computing to be delivered as a service.
> http://www.accelacomm.com/jaw/sfnl/114/51521223/
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
