Page MenuHomePhabricator

Add style description in .eo
Open, WishlistPublic

Description

Edje style used in elementary actually define the API you can use. It is theme specific, but for ease of use and improve developers experience, we should document the minimum expectation that a developer should have from any theme. Additionnaly, this would generate helpers automatically for dev that hide edje call to them. Bindings would use the C generated function to improve there own usage.

An examples for a button :

style {
  rounded {
    @message radius {
      [[ Allow to set how much rounded are the corner of the button ]]
      params {
         rx: uint; [[ Number of pixels horizontally taken for rounded corner ]]
         ry: uint; [[ Number of pixels vertially taken for rounded corner ]]
      }
     @text label; [[ Set button label ]]
     @content icon; [[ Set an object for icon ]]
    }
  }
}

This would generate the following helper I guess :

bool efl_ui_property_rounded_radius_set(obj, rx, ry); // Which would call edje_object_message_send with an INT_SET message
bool efl_ui_property_rounded_label_set(obj, "label");
bool efl_ui_property_rounded_icon_set(obj, efl_add(...));

The generated function would check that the style has been set on the object correctly to the right name, return false and failed if it hasn't. Return true and send the information properly to edje. Ideally we would be able to extract this style definition from the .eo, maybe generate the code somehow, that would make it possible to check that when we set a style, it does properly provide what the API said it should and fail if it doesn't.

cedric created this task.Mar 29 2017, 6:40 PM
cedric triaged this task as TODO priority.Mar 29 2017, 6:53 PM
cedric added projects: efl, Restricted Project.
q66 added a comment.Apr 6 2017, 5:42 AM

This is also something we could look into, however I've been doing some prototyping in my local tree and still not sure how I'd approach this. We should probably discuss this more and find some reasonable way for 1.20.

my take on this is we'll stick some bad design just to cope with legacy... Eo on its own is good enough without extensions to do that. What is different from a new button class per style (ie: 1 style = 1 subclass)? The base class can then use introspection to automatically provide methods that convert to message or part_text_set or swallow.

If we have to add some kind of support, that needs to be on "class.constructor", or the meta-class... that allows these to be easily provided. Would be useful for Efl_Models that convert to/from Eet, DBus...

Why would we put anything legacy related in ? I see no reason for it. Most of the legacy generated code is being turned off and wrapped or widget become legacy only widget.

The idea to make style an automatic subclass is maybe an interesting concept (From a defined style we should be able to easily generate a constructor that does load the asked style and the property function can also be automatically generated). I am just not to sure. I was expecting the binding to have something like button.rounded.radius = { 5, 5 }, but I don't see much problem with button_rounded.radius = { 5, 5 }.

The question would be what to do if we fail to find the proper theme to set the style. Should we fail in the constructor and destroy the object ? Basically returning NULL when we can't find a style for a generated style class ?

You're keeping "part" as "part" just because of legacy :-) That's what I mean.

As for fallback, I'd say you can always log and try to use the default style (known to be there). Then part/messages may fail... but that may happen anyway due a bug in the theme...

jpeg added a comment.EditedApr 10 2017, 9:10 PM

I also tend to think that exposing styles as different subclass names would be good. New developers would find them easily, our doc would be simpler to express, etc... And then indeed no need for the list of style names in EO.

Internally the widgets would still rely on styles from EDC though, so styles will have to be exposed in some way for people to create their own widgets, as well as for elementary's internals.

The only question really is whether we need to change a widget's style after construction (I believe we don't need).

cedric added a comment.May 1 2017, 3:23 PM
In T5307#85262, @jpeg wrote:

I also tend to think that exposing styles as different subclass names would be good. New developers would find them easily, our doc would be simpler to express, etc... And then indeed no need for the list of style names in EO.

Well, the point of having it in eo is that a style subclass can be 100% generated from it. The .eo file just describe what to add to the class. An alternate solution is to put that into another file in which we say we inherit from an eo object and have that file processed. I do think it would be easier for maintenance and to not forget style over time to have them in the same .eo file. On the other side, I think it could generate a new .h file per style as I think it would be cleaner.

Internally the widgets would still rely on styles from EDC though, so styles will have to be exposed in some way for people to create their own widgets, as well as for elementary's internals.

As a class for the new API and we maintain the legacy API on the side.

The only question really is whether we need to change a widget's style after construction (I believe we don't need).

This goes along with my question regarding what to do when we can't find a style. I feel like changing the style after construction is not a necessary behavior. As for handling the fallback, I would say as long as we can find a compatible fallback, we should go with it otherwise return NULL if nothing is good enough in the finalize function.

jpeg added a comment.May 10 2017, 3:44 AM
In T5307#86253, @cedric wrote:
In T5307#85262, @jpeg wrote:

I also tend to think that exposing styles as different subclass names would be good. New developers would find them easily, our doc would be simpler to express, etc... And then indeed no need for the list of style names in EO.

Well, the point of having it in eo is that a style subclass can be 100% generated from it. The .eo file just describe what to add to the class. An alternate solution is to put that into another file in which we say we inherit from an eo object and have that file processed. I do think it would be easier for maintenance and to not forget style over time to have them in the same .eo file. On the other side, I think it could generate a new .h file per style as I think it would be cleaner.

Good point. But a separate .h is probably too much work for very little gain (think of eolian_gen point of view).

[...]

The only question really is whether we need to change a widget's style after construction (I believe we don't need).

This goes along with my question regarding what to do when we can't find a style. I feel like changing the style after construction is not a necessary behavior. As for handling the fallback, I would say as long as we can find a compatible fallback, we should go with it otherwise return NULL if nothing is good enough in the finalize function.

I agree.

DaveMDS added a subscriber: DaveMDS.Jun 2 2017, 1:05 AM

For the moment I'm not a fan of the idea: 1 style = 1 class

What about when an app define it's own new style for a widget?
in this case there cannot be a new class generated of course...

For the moment I'm not a fan of the idea: 1 style = 1 class

What about when an app define it's own new style for a widget?
in this case there cannot be a new class generated of course...

of course you can, you can inherit from an .eo that is defined in the lib and create your own, actually it works as it should, since you also need to do that for real widgets, models, etc

any idea how this class inheritance should be done in bindings?

you better reach @q66 for details.

in my mind you should have enough introspection information to make it transparent, but I may be wrong and you may need to use eolian to dynamically parse .eo to get that...

however all in all, what's done in an ".eo.c", regarding creating the actual class, is to be done in your Python metaclass to create the eo class, that is later used by objects.

@DaveMDS it works in the language itself. For C++ through template meta programming, for C# by inheriting to a different base class which is a mirror for inheritance and in JavaScript we should use eo_override but we are not there yet.

cedric added a comment.Jun 6 2017, 9:35 PM

What about when an app define it's own new style for a widget?
in this case there cannot be a new class generated of course...

The style API is just an helper around edje feature to define a theme API basically. The idea would be to define events and messages in elementary style that an application can always rely on. If you do your own style in your application, you can still load it the same way as before with style set and the helper being automatically generated is a bonus in that case (If the application goes with using .eo, they would have that autogenerated, but they could also do the same manually in there language of choice).

cedric lowered the priority of this task from TODO to Wishlist.Jul 10 2017, 3:43 PM

Forgetting the auto-generation side of things, there are a few things we can do for now:

  • See D4197: reorganise style_set() and theme_set() to allow style_set() inside efl_add
  • Limit support in EO API to only the default styles for now
  • Prevent "style_set" after finalize for EO API
  • If needed, manually create the subclasses for the non-default styles that we absolutely need to support

All these rely on detecting the use of efl_add() as opposed to elm_something_add().

jpeg added a comment.Aug 9 2017, 2:14 AM
In T5307#91430, @jpeg wrote:
  • Limit support in EO API to only the default styles for now

We can't do that now as some widgets are composed (like combobox) and use specific sub styles.

  • Prevent "style_set" after finalize for EO API

This is done in e2fca6c454c182d4666c2f081697ed6f917ff645.

bu5hm4n moved this task from Backlog to Eolian on the efl board.Sep 30 2017, 1:47 AM
q66 added a project: Restricted Project.Apr 13 2018, 3:44 AM
zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:58 AM
q66 removed a project: Restricted Project.Jun 11 2018, 7:38 AM