Page MenuHomePhabricator

elementary: update Efl.Ui.Caching_Factory to rely on Efl.Ui.Widget_Factory for Efl.Ui.Widget.
ClosedPublic

Authored by cedric on Jan 18 2019, 6:08 PM.

Details

Summary

I am not sure we really need Efl.Ui.Caching_Factory after this, but in case we want a Caching_Factory
for non Efl.Ui.Widget, this is supported by this patch (And is the reason why most of the complexity).
The benefit from inheriting from Efl.Ui.Widget_Factory allow to get the style of an Efl.Ui.Widget
defined by an Efl.Model properly done at creation time.

Depends on D7704

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.Jan 18 2019, 6:08 PM
cedric requested review of this revision.Jan 18 2019, 6:08 PM
segfaultxavi requested changes to this revision.Jan 22 2019, 7:45 AM
segfaultxavi added inline comments.
src/lib/elementary/efl_ui_caching_factory.eo
1

We are extending the Widget factory, so we can only create widgets now, correct?
In that case, wouldn't the name Efl.Ui.Caching_Widget_Factory be more accurate?

If this is not the case and this factory is indeed generic, then there's something strange in our hierarchy...

This revision now requires changes to proceed.Jan 22 2019, 7:45 AM
cedric added inline comments.Jan 23 2019, 12:18 PM
src/lib/elementary/efl_ui_caching_factory.eo
1

It is extending Widget_Factory, but also enable non Efl.Ui.Widget to be cached (In which case, it won't forward there creation to the upper class). I am still not convinced that this is a feature we really need, but well, it was there before I decided to provide support for Efl.Ui.Widget_Factory, so I didn't remove it.

cedric updated this revision to Diff 18767.Jan 23 2019, 12:43 PM
cedric edited the summary of this revision. (Show Details)

Rebase.

segfaultxavi added inline comments.Jan 24 2019, 2:03 AM
src/lib/elementary/efl_ui_caching_factory.eo
1

This is very confusing. According to Efl.Ui.Widget_Factory documentation, it can only produce Efl.Ui.Widget, but here we use it to produce any kind of object?
Also, there seems to be a lot of restrictions in the code regarding the classes that can be produced. If these restrictions are not met the user will get errors. All this should be documented. Hopefully I will understand what is going on once you add those docs :)

cedric updated this revision to Diff 18826.Jan 24 2019, 4:28 PM

Rebase and fix documentation.

cedric updated this revision to Diff 18829.Jan 24 2019, 4:45 PM

Rebase.

segfaultxavi resigned from this revision.Jan 25 2019, 1:26 AM

The docs are OK to me now but I can't review the code, so I'll resign as reviewer. Otherwise if I approve this will get landed without further review.

SanghyeonLee accepted this revision.Jan 29 2019, 2:57 AM

looks good to me.

This revision is now accepted and ready to land.Jan 29 2019, 2:57 AM
larryolj accepted this revision.Jan 29 2019, 5:44 AM

Nice, great job πŸ’ƒ

This revision was automatically updated to reflect the committed changes.