Page MenuHomePhabricator

elementary: implement eo_finalize to set theme
AbandonedPublic

Authored by conr2d on Jul 28 2016, 6:37 AM.

Details

Reviewers
jpeg
cedric
Summary

Implement eo_finalize to set theme for bg, button, check and radio.
This will allow setting style during object creation.

eo_add(ELM_BUTTON_CLASS, parent,
       elm_obj_widget_style_set(eo_self, "anchor"));
Test Plan

run elementary_test (No side effect)

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2369
Build 2434: arc lint + arc unit
conr2d updated this revision to Diff 9662.Jul 28 2016, 6:37 AM
conr2d retitled this revision from to elementary: implement eo_finalize to set theme.
conr2d updated this object.
conr2d edited the test plan for this revision. (Show Details)
conr2d added reviewers: cedric, jpeg.
conr2d added a subscriber: taxi2se.
conr2d planned changes to this revision.Jul 28 2016, 7:28 AM

DO NOT MERGE THIS YET.

This patch doesn't allow API calls during object creation like text_set. (theme is not loaded)
We can instruct that style_set should be applied first, but when developer want to use "default" style, explicit style_set should not be mandatory.
I will investigate further to improve the structure.

jpeg edited edge metadata.Jul 28 2016, 7:11 PM

I guess the theme should be set during finalize() if it's not already been set, and it should also be set (to the default) whenever a function call is made that requires the theme (eg. text_set, etc...). The only problem is to know which functions need the theme and which do not. Another solution is to make all calls async, postponing their effects until after the theme is loaded. This would fundamentally change the API though.

cedric edited edge metadata.Jul 28 2016, 8:44 PM
In D4197#69987, @jpeg wrote:

I guess the theme should be set during finalize() if it's not already been set, and it should also be set (to the default) whenever a function call is made that requires the theme (eg. text_set, etc...). The only problem is to know which functions need the theme and which do not. Another solution is to make all calls async, postponing their effects until after the theme is loaded. This would fundamentally change the API though.

The idea of making everything async is a good idea. As we want to push for more asynchronous work, it would be great if we could actually push even for edje to be loading file asynchronously and have all the swallow and friend working without blocking until we read values back. I think that's doable as edje does imply things to be calculated and correct only on edje_calc_do. So should we make swallow, text set and friends asynchronous ?

As with the first solution, making function synchronous if the theme is not set, can be easily handle by just forcing a theme set to default if their no theme set yet. In which case it is the application developper duty to call the function in the right order.

I still do prefer the idea of making everything async.

jpeg added a comment.Jul 28 2016, 10:01 PM

I like async too, but errors need to be well reported. Be it through a promise or an event.
Most apps will not care about errors, they simply need to exist for apps that write more robust code :)

In D4197#70032, @jpeg wrote:

I like async too, but errors need to be well reported. Be it through a promise or an event.
Most apps will not care about errors, they simply need to exist for apps that write more robust code :)

Promise open the way of chaining and globering all the swallow.

I think if we use edje defined content and its ability to reload content, this feature should be able to work completely. We would just need a new set of API for setting content on edje in an async way and switch all elementary internal to it and maybe even external too. I need to land the new promise soon.

jpeg added a comment.Jul 12 2017, 12:20 AM

This was a good idea. Now the patch obviously can't apply anymore, though. And this didn't cover all widgets, why?
Anyway I think this needs to be abandoned.

@jpeg Some widgets set text or contents in their constructor, so theme should be loaded before then. (efl_costructor -> efl_canvas_group_add -> efl_finalize)
Or, edje object should have the ability to queue setting text or contents before theme load.
I planned to update this revision with new patch which changes edje rather than elementary or I would create a new revision so that you can abandon this.

jpeg added a comment.Jul 12 2017, 12:30 AM
In D4197#84662, @conr2d wrote:

@jpeg Some widgets set text or contents in their constructor, so theme should be loaded before then. (efl_costructor -> efl_canvas_group_add -> efl_finalize)
Or, edje object should have the ability to queue setting text or contents before theme load.
I planned to update this revision with new patch which changes edje rather than elementary or I would create a new revision so that you can abandon this.

I am mentionning this idea in T5307. Please abandon this patch. Thanks :)

conr2d abandoned this revision.Aug 1 2017, 6:56 AM