Page MenuHomePhabricator

widget style property
Closed, ResolvedPublic

Description

Right now the style property is protected, we might want to have that available for external users (see the spinner example for that)

bu5hm4n created this task.Aug 1 2019, 1:18 AM
bu5hm4n triaged this task as High priority.
bu5hm4n added subscribers: zmike, cedric.
bu5hm4n added a subscriber: segfaultxavi.
segfaultxavi added a comment.EditedAug 1 2019, 2:40 AM

I don't know yet how styles work, so here's my less than two-cents worth:

  1. It seems obvious that the user should be able to change a widget's style. No? Or is that meant to be changed through other APIs, like set_layout_direction()?
  2. style_set is meant to be called inside the constructor. How's the spinner cxx example supposed to work?
  3. All C# widgets can change style at construction time through a constructor parameter. Does something similar exist for cxx?
  4. As I recall, styles are free-form strings, defined by themes, widgets, extensions and whatnot. How are we gonna document that? Do we need a runtime tool that lists available styles?

My feeling is that cxx bindings have been neglected for a long time and it might take a while to get them up to speed... time we don't have right now.

  1. Yes, but there is something like the "toggle" style for check, which has nothing to do with spinner.
  2. style is protected, you cannot call it in the constructor, because you are not allowed to call it.
  3. That is a little bit concerning, the style parameter is protected, how is it getting into the constructor list ? style is not a constructor property
  4. I have no idea

Sure, but this shows that IMO style should not be protected.

If my memory is correct -
when we began these interface works, upstream developers discussed together about STYLE.

And, we got the conclusion that -

  1. efl_ui_xxx classes never use STYLE
  2. if specific class needs to change it's appearance, it should be covered by property of the class
  3. if the change can not be covered by properties, then class should be extended newly

These ideas came from the discussion that -

  1. legacy widgets' style is doing too many things, and changing style gives almost new widget functionality
  2. theme's role should be limited to "layouting for the widget's appearance"

I'm still scratching my head to recall the more information about this STYLE.
If I get more, then will share that here.

Okay, but with this it sounds like @protected should be definitly removed from the style property of widget ?

zmike added a comment.Aug 22 2019, 4:45 AM

I don't see a particular reason why it should be @protected other than preventing people from potentially footgunning themselves by overriding it improperly.

To clarify @woohyun's comment, I think the idea was that we should not be using style for widget features; style should still be used by widgets to change their appearance.

zmike closed this task as Resolved.Aug 29 2019, 7:46 AM
zmike claimed this task.

This is no longer @protected so the ticket can be closed.

bu5hm4n reopened this task as Open.Aug 29 2019, 7:52 AM
@property style {
         set @protected {
         }

Still protected.

zmike added a comment.Aug 29 2019, 9:26 AM

Oh I misread. That can be removed.

zmike added a comment.Sep 3 2019, 10:08 AM

Just waiting for the patch to land I guess