Your recent patch does resolve the additional regression here, and your
point regarding color_set being mostly useful only for changing
transparency is also valid, though also potentially it could be used for
providing a tint to widgets, e.g., making them green or something like that.

I think probably additional docs should be added to the efl_ui_widget base
class specifying the result of calling efl_gfx_color_set on a widget so
there is no confusion here.

As for additional API, I wonder if we need an efl_ui_widget property for
bg_color_set to change the color of a widget's internal bg object? There is
at least elm_code which has such a bg object...

On Wed, Aug 1, 2018 at 10:37 PM Hermet Park <[email protected]> wrote:

> Behavior is changed. elm_bg_color_set() will change internal object's
> color. (not only for bg rectangle). It still could occur compatibility
> break. Of course, we can fix this soon for legacy.
>
> But here problem is,
> Normally, we don't use evas_object_color_set() (corresponding to
> gfx_color_set()) for widgets. Change widgets' color is insane because users
> hardly imagine the result of color_change api call from A widget.
> That's why elm_bg_color_set() is useful.
>
> IMHO, the only one scenario of using color_set() (from user point of) is to
> change transparency. not the color actually.
>
> And if this efl_gfx_color_set() is working to change widget's internal
> objects,
> We need to provide an additional api again such as elm_bg_color_set()
>
>
>
>
> On Wed, Aug 1, 2018 at 10:56 PM, Mike Blumenkrantz <
> [email protected]> wrote:
>
> > I think probably the most "correct" change would actually be to remove
> the
> > color_set implementation entirely, which would allow the call to fall
> > through to the base efl widget implementation which would trigger this
> > naturally.
> >
> > There have been 2 paths for setting color on a bg object for some time;
> the
> > evas_object_color_set (efl_gfx_color_set) method was implicit since this
> > works on all objects, and the elm_bg_color_set was the explicit method
> > which was added before there was any form of widget inheritance in elm
> (ca.
> > 2011 and earlier).
> >
> > No compatibility exists problem here. The results of each call are the
> same
> > as they've always been: users of elm_bg_color_set cannot set
> transparency,
> > and users of evas_object_color_set can set transparency. The latter is a
> > "new" feature of the bg widget which has existed for 5+ years but has no
> > relevance to the elm_bg method.
> >
> > On Wed, Aug 1, 2018 at 12:34 AM Hermet Park <[email protected]>
> wrote:
> >
> > > This looks strange to me. It occurs compatibility problem as well .
> > > Originally elm bg has a single color func - elm_bg_color_set() it's
> very
> > > clear and obvious function to change color by users.
> > > Now, users can't expect the result by this internal color chain change.
> > >
> > >
> > > On Wed, Aug 1, 2018 at 8:00 AM, Mike Blumenkrantz <
> > > [email protected]> wrote:
> > >
> > > > devilhorns pushed a commit to branch master.
> > > >
> > > > http://git.enlightenment.org/core/efl.git/commit/?id=
> > > > b9b209f60f040d0afbccc1db0c96c8e36d14e682
> > > >
> > > > commit b9b209f60f040d0afbccc1db0c96c8e36d14e682
> > > > Author: Mike Blumenkrantz <[email protected]>
> > > > Date:   Tue Jul 31 18:49:57 2018 -0400
> > > >
> > > >     elm/bg: call color_set up the class chain internally
> > > >
> > > >     Summary:
> > > >     this ensures that the color_set call is propagated correctly,
> > > resulting
> > > >     eventually in a call to the smart clipped class method for
> > color_set
> > > > which
> > > >     will change the opacity of the edje object in addition to the bg
> > > > widget's
> > > >     internal rect object
> > > >
> > > >     this seems to have been a regression created during the move to
> the
> > > > interfaces
> > > >     version of the bg widget
> > > >
> > > >     fix T7232
> > > >
> > > >     Reviewers: devilhorns
> > > >
> > > >     Reviewed By: devilhorns
> > > >
> > > >     Subscribers: cedric, #committers
> > > >
> > > >     Tags: #efl_widgets
> > > >
> > > >     Maniphest Tasks: T7232
> > > >
> > > >     Differential Revision: https://phab.enlightenment.org/D6716
> > > > ---
> > > >  src/lib/elementary/efl_ui_bg_widget.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/lib/elementary/efl_ui_bg_widget.c
> > > > b/src/lib/elementary/efl_ui_bg_widget.c
> > > > index dcd8fc409e..8662516ad2 100644
> > > > --- a/src/lib/elementary/efl_ui_bg_widget.c
> > > > +++ b/src/lib/elementary/efl_ui_bg_widget.c
> > > > @@ -163,8 +163,9 @@ elm_bg_color_set(Evas_Object *obj,
> > > >  }
> > > >
> > > >  EOLIAN static void
> > > > -_efl_ui_bg_widget_efl_gfx_color_color_set(Eo *obj EINA_UNUSED,
> > > > Efl_Ui_Bg_Widget_Data *sd, int r, int g, int b, int a)
> > > > +_efl_ui_bg_widget_efl_gfx_color_color_set(Eo *obj,
> > Efl_Ui_Bg_Widget_Data
> > > > *sd, int r, int g, int b, int a)
> > > >  {
> > > > +   efl_gfx_color_set(efl_super(obj, MY_CLASS), r, g, b, a);
> > > >     efl_gfx_color_set(sd->rect, r, g, b, a);
> > > >  }
> > > >
> > > >
> > > > --
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > Regards, Hermet
> > >
> > > ------------------------------------------------------------
> > ------------------
> > > Check out the vibrant tech community on one of the world's most
> > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > > _______________________________________________
> > > enlightenment-devel mailing list
> > > [email protected]
> > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> > >
> > ------------------------------------------------------------
> > ------------------
> > Check out the vibrant tech community on one of the world's most
> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > _______________________________________________
> > enlightenment-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> >
>
>
>
> --
> Regards, Hermet
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> enlightenment-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to