Page MenuHomePhabricator

evas_textblock: change font-size/font-family only using EFL.Text.Font Interface
Needs ReviewPublic

Authored by ali.alzyod on Sun, Jun 23, 6:55 AM.

Details

Summary

Currently:
User cannot change font size only, he needs to set both font and font size with (efl_text_font_font_set)
To change size only, you need to make two calls, one to get font (efl_text_font_font_get) , then pass it again with new size to (efl_text_font_font_set).

New Behaviour:
If user want to change size only, then he passes NULL as font argument to keep same font.
If user want to change font only, then he passes 0 as font-size argument, to keep same font-size.

Notes:
This is not best solution, but it better than current behaviour.
I think best solution to have separate function to set font size, but It might break current api or duplicate functions.

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D9158
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 12012
Build 8846: arc lint + arc unit
ali.alzyod created this revision.Sun, Jun 23, 6:55 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/

ali.alzyod requested review of this revision.Sun, Jun 23, 6:55 AM
ali.alzyod edited the summary of this revision. (Show Details)Sun, Jun 23, 6:57 AM
ali.alzyod edited the summary of this revision. (Show Details)Sun, Jun 23, 7:20 AM
cedric requested changes to this revision.Tue, Jun 25, 4:37 PM

I have one concern here, but I think it is something that can be addressed. Efl.Canvas.Layout does use efl_text_font_set on part that are actually using legacy object and so will rely on legacy behavior (where font == NULL) is usually ignored as a non action behavior (Ignoring the new value completely). I think it would be nice to have all the implementation of efl_text_font_font_set be providing this behavior, even for legacy, so that Efl.Canvas.Layout does provide the proper behavior in all case.

This revision now requires changes to proceed.Tue, Jun 25, 4:37 PM

@cedric I do not get exactly understand your concern, can you please help ?

Also please note efl_text_font_font_set does not work with legacy objects

@cedric I do not get exactly understand your concern, can you please help ?

Also please note efl_text_font_font_set does not work with legacy objects

It actually does, edje rely on it in src/lib/edje/edje_text.c and src/lib/edje/edje_part_text.c and they are applied on evas_object_text (_efl_canvas_text_efl_text_font_font_set) and evas_object_textblock (_evas_text_efl_text_font_font_set). As _efl_canvas_layout_part_text_efl_text_font_font_set do expose a way to call efl_text_font_set on a part from the application code, it would be best that its behavior was the same (As in most case, people will actually be accessing a text through an edje part). I hope that clarify a bit more the flow of the code. Let me know if you have more question here.

Please notice sample code that font_set does not work on legacy object

#define EFL_EO_API_SUPPORT 1
#define EFL_BETA_API_SUPPORT 1
#include<Eina.h>
#include<Efl.h>
#include <Elementary.h>

EAPI_MAIN int
elm_main(int argc, char **argv)
{
   Evas_Object *win,*textblock;

   elm_policy_set(ELM_POLICY_QUIT, ELM_POLICY_QUIT_LAST_WINDOW_CLOSED);

   win = elm_win_util_standard_add("Main", "App");
   elm_win_autodel_set(win, EINA_TRUE);

  char * style = "DEFAULT='font=arial font_size=30 color=#F00'";
  textblock = evas_object_textblock_add(win);
  Evas_Textblock_Style * ts =  evas_textblock_style_new();
  evas_textblock_style_set(ts,style);
  evas_object_textblock_style_set(textblock,ts);
  evas_object_textblock_text_markup_set(textblock,"Hello World");
  efl_text_font_set(textblock,"Sans",100);
  
   evas_object_size_hint_weight_set(textblock,EVAS_HINT_EXPAND,EVAS_HINT_EXPAND);
   evas_object_size_hint_align_set(textblock,EVAS_HINT_FILL,EVAS_HINT_FILL);
   evas_object_show(textblock);
   
 
   evas_object_move(textblock,0,0);
   evas_object_resize(textblock,320,480);
   evas_object_resize(win,320,480);
   
   evas_object_show(textblock);
   evas_object_show(win);
   elm_run();

   return 0;
}
ELM_MAIN()

do you mean change _evas_text_efl_text_font_font_set inside evas_object_text.c ?

In that part I notice following code

if ((!font) || (size <= 0)) return;

where it drop all the change if font or size are 0, I think we can change this to be same as textblock

do you mean change _evas_text_efl_text_font_font_set inside evas_object_text.c ?

In that part I notice following code

if ((!font) || (size <= 0)) return;

where it drop all the change if font or size are 0, I think we can change this to be same as textblock

Yes, exactly.

ali.alzyod updated this revision to Diff 23000.Wed, Jun 26, 8:43 PM

change behaviour in evas_object_text to be same as evas_object_textblock

ali.alzyod updated this revision to Diff 23008.Thu, Jun 27, 3:36 AM
  • add remove commit

Good. One last bit, can we have a test for this one? Should be straight forward as it is just a change in the getter/setter behavior.

  • add test, update documentation
ali.alzyod edited the summary of this revision. (Show Details)Sat, Jun 29, 12:51 PM
ali.alzyod retitled this revision from evas_textblock: change font size only using EFL.Text.Font Interface to evas_textblock: change font-size/font-family only using EFL.Text.Font Interface.Sat, Jun 29, 11:03 PM
ali.alzyod added a comment.EditedSun, Jul 7, 11:34 PM

@cedric can you please take a look at this?

@cedric please let me know if you have any remaining issues ?

@cedric kindly reminder :)