Page MenuHomePhabricator

Efl.Ui.Popup.Anchor.List: add initial code
Needs RevisionPublic

Authored by Blackmole on Dec 15 2017, 12:43 AM.

Details

Summary

add efl.ui.popup.anchor.list class

Test Plan

elementary_test -to efl.ui.popup.anchor.list

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Blackmole created this revision.Dec 15 2017, 12:43 AM
jpeg requested changes to this revision.Dec 18 2017, 10:16 PM

Unfortunately widget item can't be used. But this is a good first draft.
I think we need to formalize the item & item list interfaces first.
The blocking issue right now is that I assume those should rely on models, which means it depends on @cedric to push the final version of the model / promise API.

src/lib/elementary/efl_ui_popup_anchor_list_item.eo
2

Oh, no. Elm.Widget.Item can't be used.

22

Don't include unimplemented things here.

This revision now requires changes to proceed.Dec 18 2017, 10:16 PM

@Blackmole
Please check the comments.

src/lib/elementary/efl_ui_popup_anchor_list.c
31

Please use efl_layout_signal_emit().

33

Please use efl_layout_signal_emit().

50

Please use new interface API.
efl_add(EFL_CANVAS_LAYOUT_CLASS, pd->box);

52

efl_gfx_size_hint_align_set

54

efl_layout_signal_callback_add

58

efl_text_set

64

efl_content_set

69

efl_gfx_size_hint_min_set

86

efl_layout_signal_callback_del

105

efl_ui_box

163

efl_ui_box

@jpeg
jpeg~ do you mean this patch should be pending until Efl.Ui.Item implementation is finished?
Or is this patch ok if just Elm.Widget.Item is not used?

jpeg added a comment.Dec 19 2017, 12:41 AM

Just don't use Elm.Widget.Item.
I think a model API can be done later.

Blackmole updated this revision to Diff 13563.Dec 19 2017, 2:31 AM
Blackmole marked 12 inline comments as done.

change regacy code

Blackmole updated this revision to Diff 13564.Dec 19 2017, 2:57 AM

remove Elm_Widget_Item

@Blackmole
Please check the comment

src/lib/elementary/efl_ui_popup_anchor_list.c
28

I think that DATA_GET_RETURN is better here.

41

I think that DATA_GET_RETURN is better here.

48

I think that DATA_GET_RETURN is better here.

54

EFL_GFX_SIZE_HINT_FILL

79

I think that DATA_GET_RETURN is better here.

109

visible is true by default.
So this is unnecessary.

166

EFL_GFX_SIZE_HINT_EXPAND

src/lib/elementary/efl_ui_popup_anchor_list.eo
8

Please update the description for Efl.Ui.Popup.Anchor.List.

13

Elm.Widget.Item should not be returned here.
How about returning Eo here?

src/lib/elementary/efl_ui_popup_anchor_list_item.eo
4

I think that it is needed to implement destructor here to deallocate it->label.
For now there is no method here, please remove "methods {" as well.

Blackmole updated this revision to Diff 13575.Dec 19 2017, 5:55 PM
Blackmole marked 9 inline comments as done.

add item destructor and date_get change to data_get_or_return

@Blackmole

I mean DATA_GET_OR_RETURN actually returns the function if the data is not taken.
you can see EFL_UI_POPUP_DATA_GET_OR_RETURN macro.

Blackmole updated this revision to Diff 13576.Dec 19 2017, 6:18 PM

add DATA_GET_OR_RETURN code

Blackmole updated this revision to Diff 13578.Dec 19 2017, 9:15 PM

add pressed effect

jpeg requested changes to this revision.Dec 19 2017, 9:34 PM

I guess this is work in progress?

src/lib/elementary/efl_ui_popup_anchor_list.eo
7

which type?

src/lib/elementary/efl_ui_popup_anchor_list_item.eo
4

no properties at all?
no events?

This revision now requires changes to proceed.Dec 19 2017, 9:34 PM

@jpeg

For now, very primitive things are implemented in Efl.Ui.Popup.Anchor.List. (i.e. Showing item list)
I think it would be better if we provide events and methods for item after Widget Item class is defined.
What do you think about that?

src/lib/elementary/efl_ui_popup_anchor_list.eo
7

According to the T6360, item_append returns Efl.Ui.Item.
However, Efl.Ui.Item is an interface so I think Efl.Ui.Item cannot be returned by item_append.
Do you think it should be Efl.Ui.Popup.Anchor.List.Item?

src/lib/elementary/efl_ui_popup_anchor_list_item.eo
4

The list inside Efl.Ui.Popup.Anchor.List will be modified if Widget Item class is defined.
So no property and no event exists for now.

jpeg added a comment.Dec 20 2017, 12:15 AM

@jpeg

For now, very primitive things are implemented in Efl.Ui.Popup.Anchor.List. (i.e. Showing item list)
I think it would be better if we provide events and methods for item after Widget Item class is defined.
What do you think about that?

I think we can implement everything here first, and then just change the API names by using the interface.

src/lib/elementary/efl_ui_popup_anchor_list.eo
7

Since at this point you're not implementing the interface Efl.Ui.Item_List (which should be done later), you should be returning Efl.Ui.Popup.Anchor.List.Item. But Efl.Ui.Item is also correct. Efl.Ui.Popup.Anchor.List.Item should inherit from Efl.Ui.Item.

src/lib/elementary/efl_ui_popup_anchor_list_item.eo
4

I think they can be implemented here first, and then merged with the interface once we have it.

Blackmole updated this revision to Diff 13637.Jan 2 2018, 4:28 AM

add Efl.Ui.Item interface

Blackmole updated this revision to Diff 13638.Jan 2 2018, 9:37 PM

fix efl.ui.popup.anchor sample error

cedric added a comment.Mar 6 2018, 6:04 PM

What is the status of this ?

Efl.Ui.Popup.Anchor.List class probably requires its own item class or interface.
Since this class instance can be created by adding Efl.Ui.Popup.Anchor with list, the priority of this class is not that high.
I think it would be better that this class implementation is finished after other item based class implementation is finished.

zmike requested changes to this revision.May 2 2018, 4:33 PM
zmike added a project: efl.
zmike added a subscriber: zmike.

This patch needs rebasing.

This revision now requires changes to proceed.May 2 2018, 4:33 PM

@Jaehyun_Cho @herb

Could you give a dicision whether to keep this patch or not ?