Page MenuHomePhabricator

efl: split Efl.Ui.Factory.create stage into constructing and building
ClosedPublic

Authored by cedric on Sep 15 2019, 11:38 PM.

Details

Summary

constructing is called during construction time, building is called
after finalize. This is usefull for theme related properties that can
only be set after the theme is applied, which happens during finalize.
Being event allow the user of the factory to add more initialization
without needing to implement any new class.

Depends on D9951

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
cedric created this revision.Sep 15 2019, 11:38 PM
cedric requested review of this revision.Sep 15 2019, 11:38 PM

I am the author ... dont want to accept this one.

zmike requested changes to this revision.Sep 16 2019, 6:26 AM

I'm not convinced of this concept. Why can't this just be rolled into the efl.object constructor?

This revision now requires changes to proceed.Sep 16 2019, 6:26 AM
zmike added a comment.Sep 16 2019, 6:27 AM

I'm specifically referring to the constructing method.

Because some time the factory want to initialize the object with a different set of value than the default one and duplicating objects seems unnecessary if you're already doing a new factory, why add also a new set widgets to if just for changing a few default value.

zmike added a comment.Sep 16 2019, 8:07 AM

Because some time the factory want to initialize the object with a different set of value than the default one and duplicating objects seems unnecessary if you're already doing a new factory, why add also a new set widgets to if just for changing a few default value.

I don't understand what this is supposed to mean. I'm talking about using a custom Efl.Object.constructor implementation for those cases.

cedric planned changes to this revision.Sep 17 2019, 4:56 PM
cedric updated this revision to Diff 25195.Sep 19 2019, 1:15 PM
cedric edited the summary of this revision. (Show Details)

rebase and take comment into account

@zmike i am not sure what you are proposing, the factory here cannot just add another constructor to the widget. And if you have a chain of factories (defined by inheritance) you need to create the widget, then ask *all* the factories to apply the constructor properties before the finalizer, then let the finalizer happen. Then again ask to build all the properties as required (needed as a extra step, since we are sometimes doing that on new widgets, and sometimes on cached widgets).
tldr; i do not see a way arround this here.

cedric updated this revision to Diff 25285.Fri, Sep 20, 3:23 PM

phab!!!

bu5hm4n requested changes to this revision.Mon, Sep 23, 7:56 AM

This looks mostly fine except this little nit.

In general, can we say that the user should always use the events, and only inherit the factory if you want to have custom caching and so on.

src/lib/elementary/efl_ui_widget_factory.c
79

Wtf ?

This revision now requires changes to proceed.Mon, Sep 23, 7:56 AM
cedric planned changes to this revision.Mon, Sep 23, 2:20 PM

In general, can we say that the user should always use the events, and only inherit the factory if you want to have custom caching and so on.

I think this position make sense. Will update the documentation accordingly.

src/lib/elementary/efl_ui_widget_factory.c
79

The commit after start to use that event and I didn't want to have to change the setup/destruction of the callback which use a callback array here in another commit to reduce the number of changed line in unrelated piece of code. This handler being empty create no problem.

cedric updated this revision to Diff 25412.Mon, Sep 23, 4:11 PM

rebase and take comments into account

idea looks okay to me as it doesn't need to worry about creating own factory and know about details in factory.
but not sure everyone get agreement on this.. @bu5hm4n @zmike are we good for this now?

bu5hm4n accepted this revision.Mon, Sep 23, 11:57 PM
bu5hm4n added inline comments.
src/lib/elementary/efl_ui_widget_factory.c
79

okay.

This revision was not accepted when it landed; it landed in state Needs Review.Tue, Sep 24, 12:19 AM
Closed by commit rEFL759ac54e7f77: efl: split Efl.Ui.Factory.create stage into constructing and building (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain Why
This revision was automatically updated to reflect the committed changes.