On Sat, 16 Apr 2011 17:30:13 +0900 Seunggyun Kim <sgyun....@samsung.com> said:

i'm rather happy that i now get to grumble about much more esoteric things in
patches :)

... why is ediit_mode not Eina_Bool and even better it's together with all the
other Eina_Bool var:1 fields? this leads to much better struct packing. saves a
byte :)
    double            longpress_timeout;
+   int               edit_mode;
 };

why are edit_icon_objs and edit_obj not higher up with pointers like
Evas_Object *spacer etc.? better struct packing and alignment. :)
+   Eina_List                     *edit_icon_objs;
+   Evas_Object                   *edit_obj;


bonus space at end before ")" :)
+        if (it->itc->edit_item_style )


this looks wrong. default case is item/edit_default but other styles are
item/stylename. really shouldnt this be like "item/edit/default" and
"item/edit/stylename".. in fact always item/edit/STYLENAME... ? simpler code
and more consistent and style can provide a simpler stylename not needing edit_
in it... :)
+   if (it->itc->edit_item_style && strcmp(it->itc->edit_item_style, "default"))
+     {
+        strncat(buf, it->itc->edit_item_style, sizeof(buf) - strlen(buf));
+        _elm_theme_object_set(it->base.widget, it->edit_obj, "genlist", buf,
elm_widget_style_get(it->base.widget));
+     }
+   else
+     {
+        _elm_theme_object_set(it->base.widget, it->edit_obj, "genlist",
"item/edit_default", elm_widget_style_get(it->base.widget));
+     }

"original_edc" - you should namespace edc partnames "officially" required by
elm. "elm.swallow.edit.content" for example
+   edje_object_part_swallow(it->edit_obj, "original_edc", it->base.view);

decide on a consistent naming of signals here. "elm,state,edit,enabled" and
"elm/state/edit/disabled" for example... BUT.. why emit a signal at the end..
just before you go DELETE the edit mode obj? pretty pointless signalling it
then deleting it. if your plan is to allow an animation to "go out of edit
mode" then keep the object around until that animation is done (some signal
tells you tis done from the obj).
+   if (effect_on) edje_object_signal_emit(it->edit_obj,
"elm,state,emode_enabled_effect", "elm");
+   else edje_object_signal_emit(it->edit_obj, "elm,state,emode_enabled",
"elm");
AND
+   edje_object_signal_emit(it->edit_obj, "elm,state,edit_end,disable", "elm");
AND
+   evas_object_del(it->edit_obj);

base.view is already a sub object of it->base.widget when it was created.
swallowing it into the edit obj didn't change that. it moved down 1 level in
the evas object tree, but not in the elm one. :) no need for this.
+   elm_widget_sub_object_add(it->base.widget, it->base.view);

that's it! :) not much.

> Hello. All.
> 
> I made genlist edit mode.
> 
> The edit mode can be used when the application want to change existing item
> edc into more expanded item edc.
> The application can use this edit mode for genlist item editing.
> For example, if the application want to show check box or plus/minus button
> at the left or right of item in some case,
> they can use this edit mode.
> 
> This edit mode style includes existing item and will be extended according
> to edit_item_style.
> 
> edit_item_style  "edit_default" EDC layout is like below. each part is
> swallow part.
> the existing item is swllowed to original_edc part.
> --------------------------------------------------------------------------
> | elm.edit.icon.1 |                original_edc                |
> elm.edit.icon,2 |
> --------------------------------------------------------------------------
> 
> After applitciaon set edit mode, genlist loads
> Elm_Genlist_Item_Class.edit_item_style if the style is not NULL.
> Then genlist swallows a existing item to "original_edc" swallow part in
> edit_item_style EDC.)
> and the genlist will start to call application icon_get callback function to
> swallow application objects(checkbox, radio button, plus/minus button. etc.)
> 
> In time, the application returns checkbox or another object in icon_get
> callback. edit mode EDC part name is "elm.edit.icon.1", "elm.edit.icon.2".. 
> 
> This API will change all realized items to edit mode style. In caes the list
> is scrolled, when an item is realized,  the item will be also changed to
> edit mode style.
> 
> For edit mode, I newly added elm_genlist_edit_mode_set /
> elm_genlist_edit_mode_get api.
> 
> I changed below files.
> 
> a) elm_genlist.patch.txt
> -----------------------------------
> elementary/src/lib/Elementary.h.in
> elementary/src/lib/elm_genlist.c
> 
> b) test_genlist.patch.txt
> -----------------------------------
> elementary/src/bin/test.c
> elementary/src/bin/test_genlist.c
> 
> c) theme_default.patch.txt
> -----------------------------------
> elementary/data/themes/default.edc
> 
> Please review about this.
> Thanks.


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    ras...@rasterman.com


------------------------------------------------------------------------------
Benefiting from Server Virtualization: Beyond Initial Workload 
Consolidation -- Increasing the use of server virtualization is a top
priority.Virtualization can reduce costs, simplify management, and improve 
application availability and disaster protection. Learn more about boosting 
the value of server virtualization. http://p.sf.net/sfu/vmware-sfdev2dev
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to