Page MenuHomePhabricator

Efl.Ui.Box: Add box custom layout set method.
AbandonedPublic

Authored by CHAN on Nov 19 2018, 3:24 AM.

Details

Reviewers
woohyun
Jaehyun_Cho
Unknown Object (User)
zmike
segfaultxavi
Summary

elm_box_layout_set() API has supported in legacy ELM.

There is many user want to box layout custom.

But now there is no other way to support it.

So i added method for it.

Test Plan

elementary_test -> add efl_ui_box_layout_set() in test_ui_box.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8080
Build 7462: arc lint + arc unit
CHAN created this revision.Nov 19 2018, 3:24 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

CHAN requested review of this revision.Nov 19 2018, 3:24 AM
CHAN edited the summary of this revision. (Show Details)Nov 19 2018, 3:27 AM
CHAN edited the test plan for this revision. (Show Details)
CHAN added reviewers: woohyun, Jaehyun_Cho, Unknown Object (User), zmike.
Jaehyun_Cho requested changes to this revision.Nov 19 2018, 4:17 AM

I left a trivial suggestion which changes the variable names of callback and data.

src/lib/elementary/efl_ui_box_private.h
29

This is trivial suggestion. How about changing the name as follows?

cb_func_data -> layout_cb_data

30

This is trivial suggestion. How about changing the name as follows?

cb_func -> layout_cb

This revision now requires changes to proceed.Nov 19 2018, 4:17 AM

How is it possible that elementary_test already had a test for this functionality if you are adding it in this patch?

@segfaultxavi

How is it possible that elementary_test already had a test for this functionality if you are adding it in this patch?

Because elm_box_layout_set() is supported in legacy elm_box but it is not supported in Efl.Ui.Box.
So @CHAN is going to support it in Efl.Ui.Box.

After examining elementary_test, I see it is changing the layout by overriding (efl_object_override) the efl_pack_layout_update method (in file src/bin/elementary/test_ui_box.c:196).

Therefore, it looks like this new API is not being tested, because nobody is calling the new method efl_ui_box_layout_set() that you are adding. How do you verify that it is working?

CHAN abandoned this revision.EditedNov 22 2018, 3:35 AM

@xavi @Jaehyun_Cho

Thanks you for giving opinion for this.

I wrote this commit to supoort layout custom.

And i knew we can support this easily to override Updatelyaout() method.

But, in that case the user can't get event callback (LAYOUT_UPDATED).

That's why i wrote this commit, but i think the user doesn't need to that callback cuz user can know updatelayout calling timing. (override function called)

So i abandon this commit.

Thanks.

OK, we'll need to document very precisely what is the correct method in the new API to define a custom layout.