Dear Mr. Kim

I briefly read the code and have some comments.

1. Use smart callbacks for "moved"
    Elm_Genlist_Item_Class is for item's styles and functions for the
item style.
    "moved" has nothing to do with item style so I recommend change
this to smart callback.

    + GenlistItemMovedFunc     moved;

2. Animator naming.
    Why not item_moving_effect_animator?

    +   Ecore_Animator   *item_moving_effect_timer;

3. Not changing constant numbers to meaningful variables in this patch.
    Those are not related to reorder mode. I suggest you to separate
the patches.
    And ( ) is missing.

    -        if (itb->count < 16)
    +        if (itb->count < itb->wd->max_items_per_block/2)

    -             if ((itbp) && ((itbp->count + itb->count) < 48))
    +             if ((itbp) && ((itbp->count + itb->count) <
itb->wd->max_items_per_block + itb->wd->max_items_per_block/2))

    -             else if ((itbn) && ((itbn->count + itb->count) < 48))
    +             else if ((itbn) && ((itbn->count + itb->count) <
itb->wd->max_items_per_block + itb->wd->max_items_per_block/2))

For other codes, I have to read the code later again.
And writing a good doxygen to test_genlist.c looks very good! I like it!

Daniel Juyung Seo (SeoZ)

On Sat, May 21, 2011 at 12:02 PM, Seunggyun Kim <sgyun....@samsung.com> wrote:
> Thank you for your comment :)
>
>
>
> I added edc effect showing whether the item is longpressed in reorder mode.
>
>
>
> You can watch a sample video on youtube.
>
> http://www.youtube.com/watch?v=ij0vAgqucjM
>
>
>
> and I attached new diff code.
>
>
>
> Newly added API about reorder mode is like below.
>
>
>
>  [API]
>
> ==================================================================
>
> - EAPI void  elm_genlist_reorder_mode_set(Evas_Object *obj, Eina_Bool
> reorder_mode) EINA_ARG_NONNULL(1);
>
> Set genlist reorder mode. This enables the item is moved to another item.
>
>
>
> - EAPI Eina_Bool  elm_genlist_reorder_mode_get(const Evas_Object *obj)
> EINA_ARG_NONNULL(1);
>
> Get the reorder mode state of genlist.
>
> ==================================================================
>
>
>
> -------------------------------------------------------
>
> From: Nicolas Aguirre [mailto:aguirre.nico...@gmail.com]
>
> Sent: Saturday, May 21, 2011 12:08 AM
>
> To: Daniel Juyung Seo
>
> Cc: Seunggyun Kim; enlightenment-devel@lists.sourceforge.net
>
> Subject: Re: [E-devel] [Patch] elm_genlist - added new feature : genlist
> reorder mode
>
>
>
>
>
> 2011/5/20 Daniel Juyung Seo <seojuyu...@gmail.com>
>
> Yeah, this is a very good feature!
>
>
>
>>> when the list is reordered, i think that even/odd background order should
>>> be keeped ?
>
> In genlist, even/odd can be changed dynamically.
>
> Suppose you add/remove items in the middle of genlist.
>
> Genlist redraw items and changes even/odd looks.
>
> even/odd are just for the GUI. They're not genlist items' property.
>
> So I propose that even/odd need to be changed as Kim did.
>
>
>
>
>
> Yes I agree with you, you misunderstood what i want to say :). I'm pretty
> sure that i had detect a bug with odd/even, but i can't reproduce it :( So
> it's ok as is !
>
>
>
> --
>
> Nicolas Aguirre
>
> Mail: aguirre.nico...@gmail.com
>
> Web: http://enna.geexbox.org
>
> Blog: http://dev.enlightenment.fr/~captainigloo/

------------------------------------------------------------------------------
What Every C/C++ and Fortran developer Should Know!
Read this article and learn how Intel has extended the reach of its 
next-generation tools to help Windows* and Linux* C/C++ and Fortran 
developers boost performance applications - including clusters. 
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to