Re: [E-devel] [PATCH] Some modifications about elm_colorselector.c and elm_entry.c

[Bruno D. <bd...@pr...>]

On Tue, Nov 30, 2010 at 8:43 AM, WooHyun Jung <wh0...@sa...> wrote:
> Hello ~
> I modified my patch.
>
> The reason for modifying about elm_colorselector can be "readability".
> Each bar in the colorselector has its own color type (like hue, saturation,
> ...)
> So I thought it will be better, if I added enum for each color type.
>
> And about elm_entry,
> I deleted mouse_up callback function (as you advised),
> because this function didn't do anything by my modification.
>

OK, then. Applied on svn.

Thank you.

> Thanks.
>
> -----Original Message-----
> From: Bruno Dilly [mailto:bd...@pr...]
> Sent: Wednesday, November 24, 2010 11:35 PM
> To: WooHyun Jung
> Cc: enl...@li...
> Subject: Re: [E-devel] [PATCH] Some modifications about elm_colorselector.c
> and elm_entry.c
>
> On Wed, Nov 24, 2010 at 11:44 AM, WooHyun Jung <wh0...@sa...>
> wrote:
>> Hello. I'm WooHyun Jung.
>>
>>
>>
>> I modified some codes of elm_colorselector.c .
>
> Hey WooHyun,
>
> would you matter to explain me why are you proposing the following
> change, please?
>
> @@ -604,9 +635,25 @@ _add_colorbar(Evas_Object *obj)
>        wd->cp[i] = ELM_NEW(Colorselector_Data);
>
>        wd->cp[i]->parent = obj;
> -       wd->cp[i]->colorselector_num = i;
> -
> -       /* load colorbar area */
> +        wd->cp[i]->color_type = i;
> +        switch(i)
> +          {
> +           case 0 :
> +              wd->cp[i]->color_type = HUE;
> +              break;
> +           case 1 :
> +              wd->cp[i]->color_type = SATURATION;
> +              break;
> +           case 2 :
> +              wd->cp[i]->color_type = LIGHTNESS;
> +              break;
> +           case 3 :
> +              wd->cp[i]->color_type = ALPHA;
> +              break;
> +           default :
> +              break;
> +          }
> +        /* load colorbar area */
>
> It's insider a for (i = 0; i < 4; i++).
> Basically you are doing
>  wd->cp[0]->color_type = 0;
> then "i" will match case 0 and:
>  wd->cp[0]->color_type = 0 (HUE) once again...
>
> Or maybe I'm missing something.
>
>>
>> And , for about elm_entry.c , I thought that "SIG_CLICKED" was wrongly
>> emitted (by "MOUSE_UP" event).
>
> If you are proposing to use _signal_mouse_clicked instead of
> _signal_mouse_up your patch should remove this function. Don't leave
> dead code, please.
>
> Thank you
>
>>
>> So I changed it.
>>
>>
>>
>> Please check attached patch file.
>>
>> Thanks.
>>
>>
>>
> ----------------------------------------------------------------------------
> --
>> Increase Visibility of Your 3D Game App & Earn a Chance To Win $500!
>> Tap into the largest installed PC base & get more eyes on your game by
>> optimizing for Intel(R) Graphics Technology. Get started today with the
>> Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
>> http://p.sf.net/sfu/intelisp-dev2dev
>> _______________________________________________
>> enlightenment-devel mailing list
>> enl...@li...
>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>>
>>
>