Page MenuHomePhabricator

efl_ui : update item classes and apply efl_part.
Needs RevisionPublic

Authored by SanghyeonLee on Dec 18 2018, 2:25 AM.

Details

Summary
  1. Rename the item classes based on their functionality.

Empty is explain the item visualization,
which is not proper to be understanding by user of this item.
using name of 'content', it explain what this item is purposed,
so let user using this item as 'container' of whole decoration.

  1. Apply efl_part class for label and content. we need to support specific efl_part for item to provide

easy binding data from the model in factory.

Test Plan

Build tested.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9014
Build 7812: arc lint + arc unit
SanghyeonLee created this revision.Dec 18 2018, 2:25 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/

SanghyeonLee requested review of this revision.Dec 18 2018, 2:25 AM
  • efl_ui : implement efl parts in efl.ui.list items

rebase the patch with applying new eo class rules,
and fix the title and summary.

SanghyeonLee retitled this revision from efl_ui : change efl_ui_empty_item name to efl_ui_content_item. to efl_ui : update item classes and apply efl_part..Jan 23 2019, 3:54 AM
SanghyeonLee edited the summary of this revision. (Show Details)
SanghyeonLee edited the summary of this revision. (Show Details)

fix build break: Efl.Ui.Translatable is renamed Efl.Ui.L10n

cedric requested changes to this revision.Jan 23 2019, 10:29 AM
cedric added subscribers: segfaultxavi, bu5hm4n.

Overall the patch is going into where we want. Might be good to figure a way to have macro generate those part like it is done for other elementary efl_part with elm_part_helper.h.

src/lib/elementary/efl_ui_list_content_item_part_full.eo
1

Reading both documentation her and in List_Content_Item, it is not obvious what this part do provide and what is there purpose. Could you clarify the goal of this class/part?

src/lib/elementary/efl_ui_list_default_item.eo
15

I am not a big fan of enforcing List in the namespace here. Wouldn't this same item actually work in Tree_View or even in Grid_View ? Wouldn't something like Efl.Ui.View.Default_Item make more sense? My understanding is that we should expect a larger number of this item in the future, so maybe having them in their own namespace is better. What do you think @segfaultxavi, @bu5hm4n and @felipealmeida ?

This revision now requires changes to proceed.Jan 23 2019, 10:29 AM
SanghyeonLee added a comment.EditedJan 23 2019, 9:34 PM

about the grid... content_item can be useful, but default item, they might need different visualization in grid actaully.
the idea you told me can reduce the useless repeatation of item classes, so in some ways, I agreed.
we can go the common item styles,
Efl.Ui.Default_Item
Efl.Ui.Content_Item
Efl.Ui.Title_Item
(note: we cannot go with "View" term cause it still used in Efl.Ui.List and Efl.Ui.Grid)
but I think internally they might checking who is my parent class and find different edje styles based on parent class, like,
efl/list_item:default
efl/grid_item:default
efl/tree_item:default
in the edc styles.

after this dicussion,
i'll abandon this patch and recreate / split new patches with

rename empty_item to content_item(is this name okay?)
rename list_item to item
efl_part implements in item
add label part in default_item

hmm but still there are some doubt about idea...
what if we need different efl_part in different parent classes?
I think we still need list_item and grid_item for these cases,
and I'm agreed with combined item class for what we can provide.
same style name, same part properties..

To provide my input I need to understand what is going on here.
If anybody has the time, can you explain how all these classes are used? What are the empty and default items?

about the grid... content_item can be useful, but default item, they might need different visualization in grid actaully.
the idea you told me can reduce the useless repeatation of item classes, so in some ways, I agreed.
we can go the common item styles,
Efl.Ui.Default_Item
Efl.Ui.Content_Item
Efl.Ui.Title_Item
(note: we cannot go with "View" term cause it still used in Efl.Ui.List and Efl.Ui.Grid)
but I think internally they might checking who is my parent class and find different edje styles based on parent class, like,
efl/list_item:default
efl/grid_item:default
efl/tree_item:default
in the edc styles.

I very much like the idea of relying on the parent class at creation time to define the edc style. Still it might be tricky to manage to do that as the style is provided by the model and the model doesn't see the parent class. We could have a dedicated model for each type of View that generate the proper style information. If you have an idea on how to implement this, I will love it as I really think having a limited amount of item class would be best.

after this dicussion,
i'll abandon this patch and recreate / split new patches with

Could you use git phab in the future? It is easier to work on bigger series of patch with it.

rename empty_item to content_item(is this name okay?)

This is one of the item that really look like it could be generic for all view. What about efl.ui.view.content_item? For @segfaultxavi, the style of this item is completely neutral, no decoration and can only contain one object inside it.

rename list_item to item

I have a preference for the namespace efl.ui.view.*_item for as much as possible that can be made generic.

efl_part implements in item
add label part in default_item

Sounds good.

To provide my input I need to understand what is going on here.
If anybody has the time, can you explain how all these classes are used? What are the empty and default items?

Will try. Basically List_View, Tree_View and Grid_View need items to display information. Those item in efl world are themed by edje and are some child of Efl.Ui.Layout logically. So they have part, some even have slot that can swallow content. The View will ask the Factory by providing the Model that match a specific Item position to create the visual object that is somekind of Efl.Ui.Layout. To specialize this Item and make life for developer easier, we are providing a set of base Item class that have a known set of label and swallow/part. This is what this *item*.eo are.

empty is an item class that has only one unique swallow. The advantage over just creating a Widget from the Factory in this case is that the developer/system can provide a custom theme that will define margin, background and other visual effect to look uniform on all cell inside a View.
default is an item class that has an icon (maybe two, I don't remember) and a label if I remember correctly. It is the most expected item for any View, I would say (But as @SanghyeonLee pointed out, the layout might change depending on which View it is used. For example in a grid, it would be stacked, while in a list, it would be side by side).

OK, thanks @cedric for the explanation.

These are my two cents:

  • Regarding Empty_Item or Content_Item:
    • I do not like Empty because it looks like it has nothing inside.
    • I do not like Content because all other items have content too, this is not the only one.
    • How about Custom_Item, Customizable_Item or Placeholder_Item?
  • Regarding namespaces:
    • I agree that if an item can be used both in a List and in a Grid, it should not be in the List namespace.
    • I also understand that an item might look different when inside a List or when inside a Grid.
    • How about we name the items with a description of their content and put them outside of any particular view? Efl.Ui.View.Default_Linear_Item (contains text and buttons side by side), Efl.Ui.View.Default_Stacked_Item (contains text and buttons on top of each other), Efl.Ui.View.Default_Title_Item (contains only text), Efl.Ui.View.Placeholder_Item (contains only one swallow).

OK, thanks @cedric for the explanation.

These are my two cents:

  • Regarding Empty_Item or Content_Item:
    • I do not like Empty because it looks like it has nothing inside.
    • I do not like Content because all other items have content too, this is not the only one.
    • How about Custom_Item, Customizable_Item or Placeholder_Item?

I like both Cutomizable_Item and Placeholder_Item with a slight preference for Placeholder_Item.

  • Regarding namespaces:
    • I agree that if an item can be used both in a List and in a Grid, it should not be in the List namespace.
    • I also understand that an item might look different when inside a List or when inside a Grid.
    • How about we name the items with a description of their content and put them outside of any particular view? Efl.Ui.View.Default_Linear_Item (contains text and buttons side by side), Efl.Ui.View.Default_Stacked_Item (contains text and buttons on top of each other), Efl.Ui.View.Default_Title_Item (contains only text), Efl.Ui.View.Placeholder_Item (contains only one swallow).

Sounds good. I will just advocate that if the item could layout depending on there parent, we can go with simpler naming. Like Efl.Ui.View.Default_Item would be an Item that has space for a swallow and a label as a legend. Otherwise the proposed schema is nice as it extend without leaking into another namespace.

about the grid... content_item can be useful, but default item, they might need different visualization in grid actaully.
the idea you told me can reduce the useless repeatation of item classes, so in some ways, I agreed.
we can go the common item styles,
Efl.Ui.Default_Item
Efl.Ui.Content_Item
Efl.Ui.Title_Item
(note: we cannot go with "View" term cause it still used in Efl.Ui.List and Efl.Ui.Grid)
but I think internally they might checking who is my parent class and find different edje styles based on parent class, like,
efl/list_item:default
efl/grid_item:default
efl/tree_item:default
in the edc styles.

I very much like the idea of relying on the parent class at creation time to define the edc style. Still it might be tricky to manage to do that as the style is provided by the model and the model doesn't see the parent class. We could have a dedicated model for each type of View that generate the proper style information. If you have an idea on how to implement this, I will love it as I really think having a limited amount of item class would be best.

after this dicussion,
i'll abandon this patch and recreate / split new patches with

Could you use git phab in the future? It is easier to work on bigger series of patch with it.

rename empty_item to content_item(is this name okay?)

This is one of the item that really look like it could be generic for all view. What about efl.ui.view.content_item? For @segfaultxavi, the style of this item is completely neutral, no decoration and can only contain one object inside it.

I think it need to be Efl.Ui.~ not Efl.Ui.View~ cause it also can be an item of Efl.Ui.List and Efl.Ui.Grid. we still share this class with view and list both cases.

rename list_item to item

I have a preference for the namespace efl.ui.view.*_item for as much as possible that can be made generic.

efl_part implements in item
add label part in default_item

Sounds good.

OK, thanks @cedric for the explanation.

These are my two cents:

  • Regarding Empty_Item or Content_Item:
    • I do not like Empty because it looks like it has nothing inside.
    • I do not like Content because all other items have content too, this is not the only one.
    • How about Custom_Item, Customizable_Item or Placeholder_Item?

I like both Cutomizable_Item and Placeholder_Item with a slight preference for Placeholder_Item.

honestly my first proposal was custom_item and manual_item before @smohanty point out it sound awkward :p
now I can believe you guys opinion than him now :)
I know why you prefer customizable than custom but it sound to long... what about Decorate_Item?
placeholder is understandable for me, but I'm doubt that it is easy to understand who begins efl.

if you don't think decorate is good enough,
I'll go customizable.

  • Regarding namespaces:
    • I agree that if an item can be used both in a List and in a Grid, it should not be in the List namespace.
    • I also understand that an item might look different when inside a List or when inside a Grid.
    • How about we name the items with a description of their content and put them outside of any particular view? Efl.Ui.View.Default_Linear_Item (contains text and buttons side by side), Efl.Ui.View.Default_Stacked_Item (contains text and buttons on top of each other), Efl.Ui.View.Default_Title_Item (contains only text), Efl.Ui.View.Placeholder_Item (contains only one swallow).

Sounds good. I will just advocate that if the item could layout depending on there parent, we can go with simpler naming. Like Efl.Ui.View.Default_Item would be an Item that has space for a swallow and a label as a legend. Otherwise the proposed schema is nice as it extend without leaking into another namespace.

still I'm not sure it need to be Efl.Ui.View_~..
we could go...
Efl.Ui.Default_Layout_Item,
Efl.Ui.Customizable_Layout_Item

I'm not certain which case need other type of items.
in toolbar or pager, naviframes,
make them restrict the type of items for their own series..

icon, endicon and label,
content for customable layout,
sounds quite generic to me, so I could be useful for all other item packable widgets.

If we call it Decorated_Item, then it looks like the other items are not decorated :(
Damn! This is difficult!
The name has to explain that this item has just one swallow, and is decorated just like the rest...
What about Single_Item or Swallow_Item? I do not think Placeholder_Item is bad, and people working on UI should be familiar with the concept.

yeah naming is really difficult thing always :p
though I like the Swallow_Item idea,
but if you two are voted Placeholder, so I'll go with it.