[webkit-reviews] review denied: [Bug 115890] [EFL] Implement colorpicker for HTML5 input type color on Minibrowser : [Attachment 202262] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 20 00:07:22 PDT 2013
Christophe Dumez <dchris at gmail.com> has denied Jungsik Tae
<davidtlay88 at gmail.com>'s request for review:
Bug 115890: [EFL] Implement colorpicker for HTML5 input type color on
Minibrowser
https://bugs.webkit.org/show_bug.cgi?id=115890
Attachment 202262: Patch
https://bugs.webkit.org/attachment.cgi?id=202262&action=review
------- Additional Comments from Christophe Dumez <dchris at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=202262&action=review
> Tools/MiniBrowser/efl/main.c:110
> + Ewk_Color_Picker *color_picker;
How about "ewk_picker" for clarity?
> Tools/MiniBrowser/efl/main.c:111
> + Evas_Object *color_selector;
How about "elm_picker" for clarity?
> Tools/MiniBrowser/efl/main.c:112
> + Evas_Object *rect_data;
I don't believe the rect_data needs to be part of the struct.
> Tools/MiniBrowser/efl/main.c:612
> +on_colorpalette_clicked(void *data, Evas_Object *obj, void *event_info)
I believe Gyuyoung had some naming suggestions that you did not take into
consideration? (color_palette?)
> Tools/MiniBrowser/efl/main.c:627
> + evas_object_color_get(window->color_selector.rect_data, &r, &g, &b, &a);
You can get the color using elm_colorselector_color_get(window->color_selector,
...) instead right?
> Tools/MiniBrowser/efl/main.c:637
> + ewk_color_picker_color_get(window->color_selector.color_picker, &r, &g,
&b, &a);
I think this shows we need a new EWK API to cancel.. Would you mind adding that
API in a separate patch?
More information about the webkit-reviews
mailing list