Page MenuHomePhabricator

Intruduce Efl.Ui.Panel (Create Efl Ui Widget from elm widget(elm_panel))
Needs ReviewPublic

Authored by CHAN on Nov 4 2018, 8:40 PM.

Details

Summary

Create Efl Ui Widget from elm widget(elm_panel)

Test Plan

elementary_test -> efl_ui_panel

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D7238
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8364
Build 7574: arc lint + arc unit
CHAN created this revision.Nov 4 2018, 8:40 PM

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 4 2018, 8:40 PM
CHAN edited the summary of this revision. (Show Details)Nov 4 2018, 8:41 PM
CHAN edited the test plan for this revision. (Show Details)
CHAN added reviewers: woohyun, Jaehyun_Cho.
Jaehyun_Cho requested changes to this revision.Nov 7 2018, 11:32 PM

Could you check the usage of legacy class and APIs as we talked?

e.g. Elm.Interface_Scrollable

data/elementary/themes/edc/efl/panel.edc
2

The stye default is not required to be written in group name.

So please remove "/default" from the group name.

e.g. "efl/scroller/panel"

src/bin/elementary/test_ui_panel.c
25

The parent of button here will be changed to panel because of efl_content_set().
So it would be better to set panel as button's parent from the beginning.

src/lib/elementary/efl_ui_panel.c
14

Please remove this line.

19

Please remove this line.

1175

I think that the name should be changed here.
e.g. Efl_Panel_Scroll_Info.

src/lib/elementary/efl_ui_panel.eo
13

Please apply indentation here.

19

Please modify documentation like below.

"Set ..." -> "Sets ..." like other documentation

31

For now, hidden property runs animation.

What do you think about animation enable/disable option?

83

Please remove this line.

This revision now requires changes to proceed.Nov 7 2018, 11:32 PM
CHAN updated this revision to Diff 17567.Fri, Nov 23, 3:57 AM

Fix code to following comment and create sample test app.

CHAN updated this revision to Diff 17676.Wed, Nov 28, 10:28 PM

Add scrollable sample.

segfaultxavi requested changes to this revision.Thu, Nov 29, 3:01 AM
segfaultxavi added a subscriber: segfaultxavi.

Failed building with meson:

In file included from ../src/lib/elementary/elm_icon.c:7:0:
../src/lib/elementary/Elementary.h:326:11: fatal error: efl_ui_panel.eo.h: No such file or directory
This revision now requires changes to proceed.Thu, Nov 29, 3:01 AM
CHAN updated this revision to Diff 17707.Thu, Nov 29, 9:15 PM

Support dynamic orient changing case.

CHAN updated this revision to Diff 17730.Sun, Dec 2, 10:11 PM

Support meson build.

Jaehyun_Cho updated this revision to Diff 17732.Mon, Dec 3, 3:18 AM

rebase based on Efl.Ui.Layout class

@CHAN

How about we use one group for scr_edje and panel_edje? (i.e. efl/panel)

How about the group name "efl/scroller/panel/left" is changed to "efl/panel/scrollable/left" or "efl/panel/left/scrollable" and the alias "efl/panel/left/panel" is removed?

data/elementary/themes/edc/efl/panel.edc
2

How about change this to "efl/panel" and both panel_edje and scr_edje use this?

14

how about change this to efl/panel/scrollable/left or efl/panel/left/scrollable?

15

how about remove this?

107

how about change this to efl/panel/scrollable/right or efl/panel/right/scrollable?

108

how about remove this?

200

how about change this to efl/panel/scrollable/top or efl/panel/top/scrollable?

201

how about remove this?

293

how about change this to efl/panel/scrollable/bottom or efl/panel/bottom/scrollable?

294

how about remove this?

CHAN updated this revision to Diff 17801.Thu, Dec 6, 12:30 AM

group name fix to following comments.

CHAN updated this revision to Diff 17802.Thu, Dec 6, 12:33 AM

Patch clean up.

CHAN updated this revision to Diff 17803.Thu, Dec 6, 12:44 AM

Patch Clean up.

CHAN marked 9 inline comments as done.Thu, Dec 6, 12:45 AM

@Jaehyun_Cho Thanks for giving good opinion.

segfaultxavi accepted this revision.Thu, Dec 6, 5:15 AM

I confirm it builds and the Panels in elementary_test work as expected. I cannot comment on the code, though.

CHAN added a comment.Sun, Dec 16, 3:12 PM

@Jaehyun_Cho Could you please push this commit?

please remove the typo

data/elementary/themes/edc/efl/panel.edc
289

please remove the typo