Page MenuHomePhabricator

efl_ui_item : redefine default_item on efl_ui_item from list_item and grid_item.
Needs RevisionPublic

Authored by SanghyeonLee on Fri, Jun 28, 2:14 AM.

Details

Reviewers
cedric
bu5hm4n
Summary

default item is common features for item based widget which holding the default style.
we only need to change style depends on it's parent class.
so combinding list_default_item and grid_default_item to single class and merge items
private data in efl_ui_item.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 12044
Build 8859: arc lint + arc unit
SanghyeonLee created this revision.Fri, Jun 28, 2:14 AM
SanghyeonLee requested review of this revision.Fri, Jun 28, 2:14 AM
bu5hm4n requested changes to this revision.Fri, Jun 28, 2:38 AM
bu5hm4n added inline comments.
src/lib/elementary/efl_ui_default_item.c
48

I am a bit worried about the limitation of this. By the time you add a new positioning code that requires another item style, we would have to patch the default item. How about we say grid / list are a default item *styles* which are then set by the container we set them to ? (Or some other instance).

src/lib/elementary/efl_ui_default_item.eo
7–8

The docuementation here needs a update i think.

This revision now requires changes to proceed.Fri, Jun 28, 2:38 AM
SanghyeonLee added inline comments.Mon, Jul 8, 1:24 AM
src/lib/elementary/efl_ui_default_item.c
48

the purpose of this patch is merge common item style in list / grid and upcoming itembased container widget so that we could reducing duplicated item classes.
default item need to be existed in this widget, and it should have common user-api usage, so me and @cedric decided to merge them in one single class, and only their style can be changed by it's parent.

still there must be list or grid based item which is under the list_item or grid_item, as like list_placeholder_item.
I think I din't get your concern clearly, could you explain your worries with the case actually it makes some problem?
thanks you a lot:)

for the notice,
this patch is the first try of merge this two different style classes,
and not the complete one.
after this patch, I'll keep update the patch with better optimizations.

Sure, i just would like to have the basic version of this patch to enable the wider usage of Efl.Ui.Item, which then can be perfectly extended into something even more general. However, the basic version should lay down the basics i think :)

src/lib/elementary/efl_ui_default_item.c
48

Sure, what i would like to have here is more something which automatically adopts to the theme style of the item container.
For example:

  • Add a way to get the container where a item is in
  • Get the theme klass of this container (list of grid or whatever will use item in the future)
  • Add some code which adds "%s_item", elm_widget_theme_klass_get() into buf, and apply this the theme

If we do not do it like this, any other widget (which is outside EFL) cannot use the item class, as it would need a patch to efl_ui_item.

With the solution above, we work around this limitation, and open up Efl.Ui.Item for widget usage in other widgets.

SanghyeonLee added a comment.EditedTue, Jul 9, 3:24 AM

hmm... I got your point
but as I see... that is too tricky.
as I mentioned, list can use efl_ui_item or efl_ui_list_item,
and it has different namespaces in the theme,
so container should take care of this different namespaces in theme_set.

furthermore,
the way of making the item is totally individual,
so it can generated without the parent..
(though below code doesn't have default case.)

in mvvm cases,
it would be more complicated,
as item is generated by factory and the factory is setted to the view,
and view doesn't know about layouting so layouter will decide how to layouting it...

@cedric,
what do you think about this?
is item should be standalone themable object? or parent widget replace the style set?
we could give some default style set and replace it when the parent exist...

hmm... I got your point
but as I see... that is too tricky.
as I mentioned, list can use efl_ui_item or efl_ui_list_item,
and it has different namespaces in the theme,
so container should take care of this different namespaces in theme_set.

I can also life with the fact that the container is setting the style to the item, and otherwise it just keeps a default one. However, the goal is to not have it here in the item.

furthermore,
the way of making the item is totally individual,
so it can generated without the parent..
(though below code doesn't have default case.)

I never said that the item needs a container parent. However, it might makes sense to get an API that gives you the container you are in.

in mvvm cases,
it would be more complicated,
as item is generated by factory and the factory is setted to the view,
and view doesn't know about layouting so layouter will decide how to layouting it...

Exactly the same case in item_container, and with the current solution, the item here would need to get the Layouter of the item container to decide it's style, which is ... Bad.

@cedric,
what do you think about this?
is item should be standalone themable object? or parent widget replace the style set?
we could give some default style set and replace it when the parent exist...

I am not speaking about the style attribute, I am talking about the theme Klass.

SanghyeonLee added a comment.EditedTue, Jul 9, 5:10 AM

Oh....... sorry I was totally misunderstood about your comment..
so want to merge the list and grid in one item_container,
so you want to using "theme" namespace instead of list or grid class.
sorry I skimmed through the comment.
we wasted a day.. my apologize.

right,
but I wonder... how do you provide the way of choosing it's layout in user side?
list or grid?
are you planning to keep the list and grid widget, and on top of that,
built new item container,
or there will be new API to set layout?

first case, we can still use the class name.. so this might be not user choice (I personally prefer to keep list and grid as the end node which user actually used class)
if you plan to remove the list and grid and only support item container...
there must be some way to set which layout user want...
I think the "style" is not a way to consider...
what do you plan for this?

bu5hm4n accepted this revision.Tue, Jul 9, 6:57 AM
This revision is now accepted and ready to land.Tue, Jul 9, 6:57 AM
bu5hm4n added a comment.EditedTue, Jul 9, 8:13 AM

My proposal to this is:

  • Keep the themes for list and grid like they are right now. This is just a general item style, which is unrelated to the widget it is in.
  • Add an API to the default item, which lets you set the prefered item style, a item style "aXa" would result in a theme group name "efl/aXa_item", The parts the layout accepts is standardtized, so are the signals. When the API is changed, the theme is set again to the layout, resetting the elements.
  • When you add the item to the item_container, the item gets passed to the Position_Manager (that is the actual name of what you call layouter) who can set the item style it wants to have, other widgets can also just do that.

(with this the finalizer of the default item can just be empty, or flush some sort of *default* theme)

The current wip state of everything can be found here: https://git.enlightenment.org/core/efl.git/log/?h=devs/bu5hm4n/work_container

DO NOT MERGE
to stabilize the item and mvvm feature,
let's keep this patch in do not merge stand
and reactivate it later.

we can freely discuss about it in this board.

bu5hm4n requested changes to this revision.Wed, Jul 10, 10:57 AM

This should be on RC and not Approval, sorry.

This revision now requires changes to proceed.Wed, Jul 10, 10:57 AM

My proposal to this is:

  • Keep the themes for list and grid like they are right now. This is just a general item style, which is unrelated to the widget it is in.
  • Add an API to the default item, which lets you set the prefered item style, a item style "aXa" would result in a theme group name "efl/aXa_item", The parts the layout accepts is standardtized, so are the signals. When the API is changed, the theme is set again to the layout, resetting the elements.
  • When you add the item to the item_container, the item gets passed to the Position_Manager (that is the actual name of what you call layouter) who can set the item style it wants to have, other widgets can also just do that.

I will add to this:

  • CollectionView (the V of MVVM) will also be able to use Position_Manager and we won't need to code a grid and a list, just reuse that infrastructure.
  • We will introduce helper class that already do set the Position_Manager to List or Grid for ListView, GridView, ListContainer and GridContainer (or whatever name we choose for them).

It will be possible for anyone to implement their own Position_Manager, like for the list that Samsung use in Watch, or a cover flow one, or whatever.

SanghyeonLee added a comment.EditedThu, Jul 11, 4:23 AM

My proposal to this is:

  • Keep the themes for list and grid like they are right now. This is just a general item style, which is unrelated to the widget it is in.
  • Add an API to the default item, which lets you set the prefered item style, a item style "aXa" would result in a theme group name "efl/aXa_item", The parts the layout accepts is standardtized, so are the signals. When the API is changed, the theme is set again to the layout, resetting the elements.
  • When you add the item to the item_container, the item gets passed to the Position_Manager (that is the actual name of what you call layouter) who can set the item style it wants to have, other widgets can also just do that.

I will add to this:

  • CollectionView (the V of MVVM) will also be able to use Position_Manager and we won't need to code a grid and a list, just reuse that infrastructure.
  • We will introduce helper class that already do set the Position_Manager to List or Grid for ListView, GridView, ListContainer and GridContainer (or whatever name we choose for them).

    It will be possible for anyone to implement their own Position_Manager, like for the list that Samsung use in Watch, or a cover flow one, or whatever.

okay then at least it will have common usability on the container and view.
sound good.