Dear Daniel, Hello!

Thank you for your review,
about, Eo wrapper.. I don't understand about Eo fully, and..
there is a possibility to change on access API. so..
I will keep your comments and do it when I'm sure the API.

Cordially,
Shinwoo Kim.


On Fri, Jan 11, 2013 at 6:12 AM, Daniel Juyung Seo <[email protected]>wrote:

> Hello kimcinoo,
> thanks for the access commit.
>
> I have some comments.
>
> 1. NEWS/ChangeLog?
>
> On Thu, Jan 10, 2013 at 9:00 PM, Enlightenment SVN <
> [email protected]> wrote:
>
> > Log:
> > [access] export some APIs which would improve application side
> flexibility.
> >     + elm_access_object_item_register();
> >     + elm_access_object_item_unregister();
> >     + elm_access_object_item_access_get();
> >
> >
> >
> > Author:       kimcinoo
> > Date:         2013-01-10 04:00:05 -0800 (Thu, 10 Jan 2013)
> > New Revision: 82561
> > Trac:         http://trac.enlightenment.org/e/changeset/82561
> >
> > Modified:
> >   trunk/elementary/src/lib/elm_access.c
> > trunk/elementary/src/lib/elm_access.h
> > trunk/elementary/src/lib/elm_gengrid.c
> > trunk/elementary/src/lib/elm_genlist.c
> >
> > Modified: trunk/elementary/src/lib/elm_access.c
> > ===================================================================
> > --- trunk/elementary/src/lib/elm_access.c       2013-01-10 11:55:43 UTC
> > (rev 82560)
> > +++ trunk/elementary/src/lib/elm_access.c       2013-01-10 12:00:05 UTC
> > (rev 82561)
> > @@ -419,7 +419,7 @@
> >  EAPI void
> >  _elm_access_object_hilight(Evas_Object *obj)
> >  {
> > -   Evas_Object *o, *parent_obj;
> > +   Evas_Object *o;
> >     Evas_Coord x, y, w, h;
> >
> >     o = evas_object_name_find(evas_object_evas_get(obj),
> > "_elm_access_disp");
> > @@ -447,12 +447,8 @@
> >       }
> >     evas_object_data_set(o, "_elm_access_target", obj);
> >
> > -   parent_obj = obj;
> > -   if (!elm_widget_is(obj))
> > -      parent_obj = evas_object_data_get(obj, "_elm_access_parent");
> > +   elm_widget_theme_object_set(obj, o, "access", "base", "default");
> >
> > -   elm_widget_theme_object_set(parent_obj, o, "access", "base",
> > "default");
> > -
> >     evas_object_event_callback_add(obj, EVAS_CALLBACK_DEL,
> >                                    _access_obj_hilight_del_cb, NULL);
> >     evas_object_event_callback_add(obj, EVAS_CALLBACK_HIDE,
> > @@ -518,110 +514,101 @@
> >     evas_object_move(accessobj, x, y);
> >  }
> >
> > -static char *
> > -_part_access_info_cb(void *data, Evas_Object *obj)
> > +static Evas_Object *
> > +_access_object_register(Evas_Object *obj, Evas_Object *parent)
> >  {
> > -   Evas_Object *eobj = data;
> > -   if (!eobj) return NULL;
> > -
> > -   const char *part = evas_object_data_get(obj, "_elm_access_part");
> > -   const char *txt = edje_object_part_text_get(eobj, part);
> > -   if (txt) return strdup(txt);
> > -   return NULL;
> > -}
> > -
> > -static void
> > -_access_obj_del(void *data __UNUSED__, Evas *e __UNUSED__,
> > -                Evas_Object *obj, void *event_info __UNUSED__)
> > -{
> > -   char *part = evas_object_data_get(obj, "_elm_access_part");
> > -   evas_object_data_del(obj, "_elm_access_part");
> > -   if (part) free(part);
> > -}
> > -
> > -EAPI Evas_Object *
> > -_elm_access_edje_object_part_object_register(Evas_Object* obj,
> > -                                             const Evas_Object *eobj,
> > -                                             const char* part)
> > -{
> >     Evas_Object *ao;
> > -   Evas_Object *po = (Evas_Object *)edje_object_part_object_get(eobj,
> > part);
> > -   Evas_Coord x, y, w, h;
> >     Elm_Access_Info *ac;
> > +   Evas_Coord x, y, w, h;
> >
> > -   if (!obj || !po) return NULL;
> > +   if (!obj) return NULL;
> >
> > -   // check previous access object
> > -   ao = evas_object_data_get(po, "_part_access_obj");
> > -   if (ao)
> > -     _elm_access_edje_object_part_object_unregister(obj, eobj, part);
> > -
> > -   // create access object
> > -   ao = _elm_access_add(obj);
> > +   /* create access object */
> > +   ao = _elm_access_add(parent);
> >     if (!ao) return NULL;
> >
> > -   evas_object_event_callback_add(po, EVAS_CALLBACK_RESIZE,
> > +   evas_object_event_callback_add(obj, EVAS_CALLBACK_RESIZE,
> >                                    _content_resize, ao);
> > -   evas_object_event_callback_add(po, EVAS_CALLBACK_MOVE,
> > +   evas_object_event_callback_add(obj, EVAS_CALLBACK_MOVE,
> >                                    _content_move, ao);
> >
> > -   evas_object_geometry_get(po, &x, &y, &w, &h);
> > +   evas_object_geometry_get(obj, &x, &y, &w, &h);
> >     evas_object_move(ao, x, y);
> >     evas_object_resize(ao, w, h);
> >     evas_object_show(ao);
> >
> > -   // register access object
> > -   _elm_access_object_register(ao, po);
> > -   _elm_access_text_set(_elm_access_object_get(ao),
> > -                        ELM_ACCESS_TYPE, evas_object_type_get(po));
> > -   evas_object_data_set(ao, "_elm_access_part", strdup(part));
> > -   evas_object_event_callback_add(ao, EVAS_CALLBACK_DEL,
> > -                                  _access_obj_del, NULL);
> > -   _elm_access_callback_set(_elm_access_object_get(ao),
> > -                            ELM_ACCESS_INFO,
> > -                            _part_access_info_cb, eobj);
> > +   /* register access object */
> > +   _elm_access_object_register(ao, obj);
> >
> > -   // set access object
> > -   evas_object_data_set(po, "_part_access_obj", ao);
> > +   /* set access object */
> > +   evas_object_data_set(obj, "_part_access_obj", ao);
> >
> >     /* set owner part object */
> >     ac = evas_object_data_get(ao, "_elm_access");
> > -   ac->part_object = po;
> > +   ac->part_object = obj;
> >
> >     return ao;
> >  }
> >
> > -EAPI void
> > -_elm_access_edje_object_part_object_unregister(Evas_Object* obj,
> > -                                               const Evas_Object *eobj,
> > -                                               const char* part)
> > +static void
> > +_access_object_unregister(Evas_Object *obj)
> >  {
> >     Evas_Object *ao;
> > -   Evas_Object *po = (Evas_Object *)edje_object_part_object_get(eobj,
> > part);
> >
> > -   if (!obj || !po) return;
> > +   if (!obj) return;
> >
> > -   ao = evas_object_data_get(po, "_part_access_obj");
> > +   ao = evas_object_data_get(obj, "_part_access_obj");
> >     if (!ao) return;
> >
> > -   evas_object_data_del(po, "_part_access_obj");
> > +   evas_object_data_del(obj, "_part_access_obj");
> >
> > -   // delete callbacks
> > -   evas_object_event_callback_del_full(po, EVAS_CALLBACK_RESIZE,
> > +   /* delete callbacks */
> > +   evas_object_event_callback_del_full(obj, EVAS_CALLBACK_RESIZE,
> >                                         _content_resize, ao);
> > -   evas_object_event_callback_del_full(po, EVAS_CALLBACK_MOVE,
> > +   evas_object_event_callback_del_full(obj, EVAS_CALLBACK_MOVE,
> >                                         _content_move, ao);
> >
> > -   evas_object_event_callback_del_full(po, EVAS_CALLBACK_MOUSE_IN,
> > -                                      _access_obj_mouse_in_cb, ao);
> > -   evas_object_event_callback_del_full(po, EVAS_CALLBACK_MOUSE_OUT,
> > -                                      _access_obj_mouse_out_cb, ao);
> > -   evas_object_event_callback_del_full(po, EVAS_CALLBACK_DEL,
> > -                                       _access_obj_del_cb, ao);
> > +   /* unregister access object */
> > +   _elm_access_object_unregister(ao, obj);
> > +
> >     evas_object_del(ao);
> >  }
> >
> > +EAPI Evas_Object *
> > +_elm_access_edje_object_part_object_register(Evas_Object* obj,
> > +                                             const Evas_Object *eobj,
> > +                                             const char* part)
> > +{
> > +   Evas_Object *ao, *po;
> > +
> > +   po = (Evas_Object *)edje_object_part_object_get(eobj, part);
> > +   if (!obj || !po) return NULL;
> > +
> > +   /* check previous access object */
> > +   ao = evas_object_data_get(po, "_part_access_obj");
> > +   if (ao)
> > +     _elm_access_edje_object_part_object_unregister(obj, eobj, part);
> > +
> > +   ao = _access_object_register(po, obj);
> > +
> > +   return ao;
> > +}
> > +
> > +//FIXME: unused obj should be removed from here and each widget.
> >  EAPI void
> > +_elm_access_edje_object_part_object_unregister(Evas_Object* obj
> > __UNUSED__,
> > +                                               const Evas_Object *eobj,
> > +                                               const char* part)
> > +{
> > +   Evas_Object *po;
> > +
> > +   po = (Evas_Object *)edje_object_part_object_get(eobj, part);
> > +   if (!po) return;
> > +
> > +   _access_object_unregister(po);
> > +}
> > +
> > +EAPI void
> >  _elm_access_object_hilight_disable(Evas *e)
> >  {
> >     Evas_Object *o, *ptarget;
> > @@ -686,7 +673,7 @@
> >     Evas_Coord x, y, w, h;
> >     Elm_Access_Info *ac;
> >
> > -   if (!item) return;
> > +   ELM_WIDGET_ITEM_CHECK_OR_RETURN(item);
> >
> >     /* check previous access object */
> >     if (item->access_obj)
> > @@ -722,8 +709,10 @@
> >  {
> >     Evas_Object *ho;
> >
> > -   if (!item || !item->access_obj) return;
> > +   ELM_WIDGET_ITEM_CHECK_OR_RETURN(item);
> >
> > +   if (!item->access_obj) return;
> > +
> >     ho = item->view;
> >     evas_object_event_callback_del_full(ho, EVAS_CALLBACK_RESIZE,
> >                                    _content_resize, item->access_obj);
> > @@ -805,59 +794,61 @@
> >       NULL
> >  };
> >
> > -EAPI void
> > -elm_access_text_set(Evas_Object *obj, int type, const char *text)
> > +EAPI Evas_Object *
> > +elm_access_object_item_register(Elm_Object_Item *item)
> >  {
> > -   _elm_access_text_set(_elm_access_object_get(obj), type, text);
> > +   Elm_Widget_Item *it;
> > +
> > +   it = (Elm_Widget_Item *)item;
> > +
> > +   _elm_access_widget_item_register(it);
> > +
> > +   if (it) return it->access_obj;
> > +   return NULL;
> >  }
> >
> > -EAPI char *
> > -elm_access_text_get(Evas_Object *obj, int type)
> > +EAPI void
> > +elm_access_object_item_unregister(Elm_Object_Item *item)
> >  {
> > -   return _elm_access_text_get(_elm_access_object_get(obj), type, obj);
> > +   _elm_access_widget_item_unregister((Elm_Widget_Item *)item);
> >  }
> >
> > -EAPI void
> > -elm_access_object_register(Evas_Object *parent, Evas_Object *target)
> > +EAPI Evas_Object *
> > +elm_access_object_item_access_get(Elm_Object_Item *item)
> >  {
> > -   Elm_Access_Info *ai;
> > +   if (!item) return NULL;
> >
> > -   if (!parent || !target) return;
> > +   return ((Elm_Widget_Item *)item)->access_obj;
> > +}
> >
> > -   evas_object_event_callback_add(target, EVAS_CALLBACK_MOUSE_IN,
> > -                                  _access_obj_mouse_in_cb, target);
> > -   evas_object_event_callback_add(target, EVAS_CALLBACK_MOUSE_OUT,
> > -                                  _access_obj_mouse_out_cb, target);
> > -   evas_object_event_callback_add(target, EVAS_CALLBACK_DEL,
> > -                                  _access_obj_del_cb, target);
> > -   ai = calloc(1, sizeof(Elm_Access_Info));
> > -   evas_object_data_set(target, "_elm_access", ai);
> > -
> > -   //TODO: evas_object_data_del(); parent should take care of children.
> > -   evas_object_data_set(target, "_elm_access_parent", parent);
> > +EAPI Evas_Object *
> > +elm_access_object_register(Evas_Object *obj, Evas_Object *parent)
> > +{
> > +   return _access_object_register(obj, parent);
> >  }
> >
> >  EAPI void
> >  elm_access_object_unregister(Evas_Object *obj)
> >  {
> > -   Elm_Access_Info *ac;
> > +   _access_object_unregister(obj);
> > +}
> >
> > -   evas_object_event_callback_del_full(obj, EVAS_CALLBACK_MOUSE_IN,
> > -                                       _access_obj_mouse_in_cb, obj);
> > -   evas_object_event_callback_del_full(obj, EVAS_CALLBACK_MOUSE_OUT,
> > -                                       _access_obj_mouse_out_cb, obj);
> > -   evas_object_event_callback_del_full(obj, EVAS_CALLBACK_DEL,
> > -                                       _access_obj_del_cb, obj);
> > +EAPI Evas_Object *
> > +elm_access_object_access_get(Evas_Object *obj)
> > +{
> > +   return evas_object_data_get(obj, "_part_access_obj");
> > +}
> >
> > -   ac = evas_object_data_get(obj, "_elm_access");
> > -   evas_object_data_del(obj, "_elm_access");
> > -   if (ac)
> > -     {
> > -        _elm_access_clear(ac);
> > -        free(ac);
> > -     }
> > +EAPI void
> > +elm_access_text_set(Evas_Object *obj, int type, const char *text)
> > +{
> > +   _elm_access_text_set(_elm_access_object_get(obj), type, text);
> > +}
> >
> > -   evas_object_data_del(obj, "_elm_access_parent");
> > +EAPI char *
> > +elm_access_text_get(Evas_Object *obj, int type)
> > +{
> > +   return _elm_access_text_get(_elm_access_object_get(obj), type, obj);
> >  }
> >
> >  EAPI void
> >
> > Modified: trunk/elementary/src/lib/elm_access.h
> > ===================================================================
> > --- trunk/elementary/src/lib/elm_access.h       2013-01-10 11:55:43 UTC
> > (rev 82560)
> > +++ trunk/elementary/src/lib/elm_access.h       2013-01-10 12:00:05 UTC
> > (rev 82561)
> > @@ -22,27 +22,70 @@
> >  typedef void (*Elm_Access_Activate_Cb)(void *data, Evas_Object
> *part_obj,
> > Elm_Object_Item *item);
> >
> >  /**
> > + * @brief Register object item as an accessible object.
> > + * @since 1.8
> > + *
> > + * @param item The elementary object item
> > + *
> > + * @ingroup Access
> > + */
> > +EAPI Evas_Object * elm_access_object_item_register(Elm_Object_Item
> *item);
> >
>
> 2. spacing
> We don't put a space after * in this case.
>
> EAPI Evas_Object * elm_access_object_item_register(
> ->
> EAPI Evas_Object *elm_access_object_item_register(
>
> I did it for you this time.
> http://trac.enlightenment.org/e/changeset/82587
>
>
>
> > +
> > +/**
> > + * @brief Unregister accessible object of the object item.
> > + * @since 1.8
> > + *
> > + * @param item The elementary object item
> > + *
> > + * @ingroup Access
> > + */
> > +EAPI void elm_access_object_item_unregister(Elm_Object_Item *item);
> > +
> > +/**
> > + * @brief Get an accessible object of the object item.
> > + * @since 1.8
> > + *
> > + * @param item The elementary object item
> > + * @return Accessible object of the object item or NULL for any error
> > + *
> > + * @ingroup Access
> > + */
> > +EAPI Evas_Object * elm_access_object_item_access_get(Elm_Object_Item
> > *item);
> >
>
> 3. const
>
> 'const' for getters are missing.
>
> EAPI Evas_Object * elm_access_object_item_access_get(Elm_Object_Item
> *item);
> ->
> EAPI Evas_Object *elm_access_object_item_access_get(const Elm_Object_Item
> *item);
>
> I did it for you this time.
> http://trac.enlightenment.org/e/changeset/82588
>
>
> > +
> > +/**
> >   * @brief Register evas object as an accessible object.
> >   * @since 1.8
> >   *
> > - * @param parent Accessibility parent object. this should be one of
> > widget.
> > - * @param target Evas object to register as an accessible object.
> > + * @param obj The evas object to register as an accessible object.
> > + * @param parent The elementary object which is used for creating
> > + * accessible object.
> >   *
> >   * @ingroup Access
> >   */
> > -EAPI void elm_access_object_register(Evas_Object *parent, Evas_Object
> > *target);
> > +EAPI Evas_Object * elm_access_object_register(Evas_Object *obj,
> > Evas_Object *parent);
> >
> >  /**
> >   * @brief Unregister accessible object.
> >   * @since 1.8
> >   *
> > - * @param obj Accessible object.
> > + * @param obj The Evas object to unregister accessible object.
> >   *
> >   * @ingroup Access
> >   */
> > -EAPI void elm_access_object_unregister(Evas_Object *target);
> > +EAPI void elm_access_object_unregister(Evas_Object *obj);
> >
> >  /**
> > + * @brief Get an accessible object of the evas object.
> > + * @since 1.8
> > + *
> > + * @param obj The evas object.
> > + * @return Accessible object of the evas object or NULL for any error
> > + *
> > + * @ingroup Access
> > + */
> > +EAPI Evas_Object * elm_access_object_access_get(Evas_Object *obj);
> >
>
> 3. const for getter.
>
>
> > +
> > +/**
> >   * @brief Set text to give information for specific type.
> >   * @since 1.8
> >   *
> >
> > Modified: trunk/elementary/src/lib/elm_gengrid.c
> > ===================================================================
> > --- trunk/elementary/src/lib/elm_gengrid.c      2013-01-10 11:55:43 UTC
> > (rev 82560)
> > +++ trunk/elementary/src/lib/elm_gengrid.c      2013-01-10 12:00:05 UTC
> > (rev 82561)
> > @@ -735,6 +735,9 @@
> >     evas_object_size_hint_min_set(it->spacer, 2 * elm_config_scale_get(),
> > 1);
> >     edje_object_part_swallow(VIEW(it), "elm.swallow.pad", it->spacer);
> >
> > +   /* access */
> > +   if (_elm_config->access_mode) _access_widget_item_register(it);
> > +
> >     if (it->itc->func.text_get)
> >       {
> >          const Eina_List *l;
> > @@ -852,10 +855,6 @@
> >
> >     it->realized = EINA_TRUE;
> >     it->want_unrealize = EINA_FALSE;
> > -
> > -   // ACCESS
> > -   if (_elm_config->access_mode == ELM_ACCESS_MODE_ON)
> > -     _access_widget_item_register(it);
> >  }
> >
> >  static Eina_Bool
> >
> > Modified: trunk/elementary/src/lib/elm_genlist.c
> > ===================================================================
> > --- trunk/elementary/src/lib/elm_genlist.c      2013-01-10 11:55:43 UTC
> > (rev 82560)
> > +++ trunk/elementary/src/lib/elm_genlist.c      2013-01-10 12:00:05 UTC
> > (rev 82561)
> > @@ -1380,9 +1380,8 @@
> >            (VIEW(it), elm_widget_mirrored_get(WIDGET(it)));
> >       }
> >
> > -   // ACCESS
> > -   if (_elm_config->access_mode == ELM_ACCESS_MODE_ON)
> > -     _access_widget_item_register(it);
> > +   /* access */
> > +   if (_elm_config->access_mode) _access_widget_item_register(it);
> >
> >     _item_order_update(EINA_INLIST_GET(it), in);
> >
> >
> 4. Eo wrapper.
> It looks like you didn't add eo wrapper for access.
>
> Thanks.
>
> Daniel Juyung Seo (SeoZ)
>
>
> >
> >
> >
> >
> ------------------------------------------------------------------------------
> > Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> > MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
> > with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> > MVPs and experts. ON SALE this month only -- learn more at:
> > http://p.sf.net/sfu/learnmore_122712
> > _______________________________________________
> > enlightenment-svn mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/enlightenment-svn
> >
>
> ------------------------------------------------------------------------------
> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> MVPs and experts. ON SALE this month only -- learn more at:
> http://p.sf.net/sfu/learnmore_122712
> _______________________________________________
> enlightenment-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>
------------------------------------------------------------------------------
Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
much more. Get web development skills now with LearnDevNow -
350+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122812
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to