Page MenuHomePhabricator

ui.relative_layout: implement Efl.Pack
ClosedPublic

Authored by YOhoho on Apr 17 2019, 3:23 AM.

Details

Summary

Now, efl_content_iterate, efl_content_count, efl_pack, efl_pack_unpack,
efl_pack_unpack_all and efl_pack_clear are available for relative_layout.

Test Plan

make test

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.
YOhoho created this revision.Apr 17 2019, 3:23 AM
YOhoho requested review of this revision.Apr 17 2019, 3:23 AM
segfaultxavi accepted this revision.Apr 17 2019, 3:52 AM

Nice!

Do we have testsuite for this?

This revision is now accepted and ready to land.Apr 17 2019, 3:52 AM
YOhoho planned changes to this revision.Apr 17 2019, 5:16 AM

I found some bugs with testcase..

What do you mean?

When I tried to write test case for this, I realized that iterator_data is not Eo pointer(child) and efl_content_count doesn't return 0 after efl_ui_relative_layout_unregister_all.
I need to fix them before this path.

YOhoho updated this revision to Diff 21443.Apr 17 2019, 10:36 PM
YOhoho edited the summary of this revision. (Show Details)

Fix some bugs and add unit test

This revision is now accepted and ready to land.Apr 17 2019, 10:36 PM
bu5hm4n added a subscriber: bu5hm4n.EditedApr 21 2019, 2:02 AM

This has nothing really to do with this revision, but do you think implementing Efl.Pack is feasible here ? Pack could do just some default, but unpack, unpack_all, or pack_clear does sound useful for this Widget.

This has nothing really to do with this revision, but do you think implementing Efl.Pack is feasible here ? Pack could do just some default, but unpack, unpack_all, or pack_clear does sound useful for this Widget.

I already tried that when i designed relative_layout. but i couldn't because of some properties that are hard to implement(pack_align, pack_padding). i don't want to make not implemented method.

I already tried that when i designed relative_layout. but i couldn't because of some properties that are hard to implement(pack_align, pack_padding). i don't want to make not implemented method.

There are also other widgets not implementing pack_align and pack_padding (Efl.Ui.Pager, Efl.Ui.List). Maybe we should refactor those two properties into its own interface ?

I already tried that when i designed relative_layout. but i couldn't because of some properties that are hard to implement(pack_align, pack_padding). i don't want to make not implemented method.

There are also other widgets not implementing pack_align and pack_padding (Efl.Ui.Pager, Efl.Ui.List). Maybe we should refactor those two properties into its own interface ?

Then i will implement Efl.Pack here :)

YOhoho updated this revision to Diff 21767.Mon, Apr 29, 4:38 AM
YOhoho retitled this revision from ui.relative_layout: implement Efl.Container to ui.relative_layout: implement Efl.Pack.
YOhoho edited the summary of this revision. (Show Details)
YOhoho edited the test plan for this revision. (Show Details)
YOhoho added a reviewer: bu5hm4n.

implement Efl.Pack

bu5hm4n requested changes to this revision.EditedTue, Apr 30, 11:28 AM

One minor NIT, i get this warning:

In file included from ../src/lib/eina/Eina.h:214,
                 from ../src/lib/elementary/Elementary.h:57,
                 from ../src/lib/elementary/efl_ui_relative_layout_private.h:10,
                 from ../src/lib/elementary/efl_ui_relative_layout.c:1:
../src/lib/elementary/efl_ui_relative_layout.c: In function ‘_efl_ui_relative_layout_efl_pack_pack’:
../src/lib/elementary/efl_ui_relative_layout.c:583:35: warning: passing argument 1 of ‘__builtin_expect’ makes integer from pointer wit
out a cast [-Wint-conversion]
    EINA_SAFETY_ON_TRUE_RETURN_VAL(eina_hash_find(pd->children, &subobj), EINA_FALSE);
../src/lib/eina/eina_types.h:223:51: note: in definition of macro ‘EINA_UNLIKELY’
 #  define EINA_UNLIKELY(exp)    __builtin_expect((exp), 0)
                                                   ^~~
../src/lib/elementary/efl_ui_relative_layout.c:583:4: note: in expansion of macro ‘EINA_SAFETY_ON_TRUE_RETURN_VAL’
    EINA_SAFETY_ON_TRUE_RETURN_VAL(eina_hash_find(pd->children, &subobj), EINA_FALSE);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/lib/elementary/efl_ui_relative_layout.c:583:35: note: expected ‘long int’ but argument is of type ‘void *’
    EINA_SAFETY_ON_TRUE_RETURN_VAL(eina_hash_find(pd->children, &subobj), EINA_FALSE);
../src/lib/eina/eina_types.h:223:51: note: in definition of macro ‘EINA_UNLIKELY’
 #  define EINA_UNLIKELY(exp)    __builtin_expect((exp), 0)
                                                   ^~~
../src/lib/elementary/efl_ui_relative_layout.c:583:4: note: in expansion of macro ‘EINA_SAFETY_ON_TRUE_RETURN_VAL’
    EINA_SAFETY_ON_TRUE_RETURN_VAL(eina_hash_find(pd->children, &subobj), EINA_FALSE);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Otherwise: great work.

This revision now requires changes to proceed.Tue, Apr 30, 11:28 AM
YOhoho updated this revision to Diff 21913.Fri, May 3, 2:20 AM

fix build warning

bu5hm4n accepted this revision.Fri, May 3, 2:54 AM
This revision is now accepted and ready to land.Fri, May 3, 2:54 AM
Closed by commit rEFL2426656fd6b4: ui.relative_layout: implement Efl.Pack (authored by Yeongjong Lee <yj34.lee@samsung.com>, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyFri, May 3, 2:54 AM
This revision was automatically updated to reflect the committed changes.

Thank you :)