Page MenuHomePhabricator

Change Single Line Mode behaviour in Entry
Needs RevisionPublic

Authored by ali.alzyod on Apr 14 2019, 6:36 AM.

Details

Summary

In simple words :
Instead of change text inside elm to remove all break lines, why dont we just use this feature from TextBlock.

This will fix
1- If user toggle state of single line mode, text will return as it is.

old behavour: text will miss all its line and paragraphs breaks.

2- Enhance performance
3- Use single function for Line Mode, that all components uses.

Diff Detail

Repository
rEFL core/efl
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10921
Build 8493: arc lint + arc unit
ali.alzyod created this revision.Apr 14 2019, 6:36 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.Apr 14 2019, 6:36 AM
ali.alzyod edited the summary of this revision. (Show Details)Apr 14 2019, 6:40 AM
ali.alzyod added reviewers: bowonryu, woohyun.
ali.alzyod edited the summary of this revision. (Show Details)Apr 14 2019, 7:09 AM
woohyun requested changes to this revision.EditedApr 18 2019, 12:49 AM

I don't think your patch is not working fine.

I did call "elm_entry_single_line_set(en, EINA_TRUE);" to elm_entry in test_entry() func (of test_entry.c).

diff --git a/src/bin/elementary/test_entry.c b/src/bin/elementary/test_entry.c
index 56264cc..4e24df8 100644
--- a/src/bin/elementary/test_entry.c
+++ b/src/bin/elementary/test_entry.c
@@ -202,6 +202,8 @@ test_entry(void *data EINA_UNUSED, Evas_Object *obj EINA_UNUSED, void *event_inf
    evas_object_show(en);
    elm_object_focus_set(en, EINA_TRUE);
 
+   elm_entry_single_line_set(en, EINA_TRUE);
+

Previously, it changed well to single line, but nothing happened with your patch.

Could you check this with proper test case ?
(And please add that test case to "src/tests/elementary".)

This revision now requires changes to proceed.Apr 18 2019, 12:49 AM
ali.alzyod updated this revision to Diff 21454.EditedApr 18 2019, 5:49 AM
ali.alzyod edited the summary of this revision. (Show Details)

Move logic to edje_entry

Where efl_ui_widget_theme_apply will cause recreation for Entry Object with new textblock.
So in the new TextBlock we will set properties for Line Mode.

+ This what if you have text with multiline and enable single line mode, try copy the text and paste it in any editor you will notice lines it paste as multi line.
+ when return back to multiline mode from single mode, text will return as it was originally

bu5hm4n requested changes to this revision.Apr 20 2019, 5:52 AM
bu5hm4n added a subscriber: bu5hm4n.
bu5hm4n added inline comments.
src/lib/edje/edje_entry.c
3014

If this textobject is not a legacy object then this will throw an error.

This revision now requires changes to proceed.Apr 20 2019, 5:52 AM

@bu5hm4n what is the error you got ?

bu5hm4n added a subscriber: zmike.Apr 21 2019, 1:35 AM

I have not tested, its a theoretical assumption. Calling legacy functions on a non-legacy object is a not so good idea i guess, you should check here if this is a legacy object or not (@zmike do you know how we do that in evas ?)

I though you had an error, Will text block added with evas_text_block_add is actually is EFL_CANVAS_TEXT_CLASS object

I am not sure i understand your last comment.
However, other things:

  • With this revision we set evas_object_textblock_legacy_newline_set to rp->part->multiline, have you checked that this does not have some unintended sideeffect ? That looks a little bit dangerous to me tbh.
  • Setting efl_text_multiline here also could have some unintended sideeffects, are those cases checked ?

Don't get me wrong, i like simplifying code, but if we break the behavior of text handling in edje, then we are in big trouble.

@bu5hm4n we were talking about this comment "If this textobject is not a legacy object then this will throw an error.", I reply that if you added TextBlock using evas_object_textblock_add then it is will construct it as EFL_CANVAS_TEXT_CLASS object, where efl_text_multiline_set is implemented.

are new comments you reply related to your first comment?

@bu5hm4n we were talking about this comment "If this textobject is not a legacy object then this will throw an error.", I reply that if you added TextBlock using evas_object_textblock_add then it is will construct it as EFL_CANVAS_TEXT_CLASS object, where efl_text_multiline_set is implemented.

Yes, however, multiline was not part of the legacy object. But part of the new efl.canvas.text object. In the function evas_object_textblock_add the legacy constructor is executed, which is used to setup legacy things in the objects of evas. I would not claim that in such a object in legacy mode new API does work as expected. (And I for my part expected a error for that, however, it seems there is none.)

are new comments you reply related to your first comment?

No.

ali.alzyod added a comment.EditedApr 21 2019, 4:37 AM

@bu5hm4n sorry but what part exactly Needs Revision

If you are concerned about using efl_text_multiline_set with textblock(created with evas_textblock_add), then I think it is normal since textblock created using evas_object_textblock_add() already call this function

ali.alzyod added inline comments.Apr 22 2019, 3:50 AM
src/lib/edje/edje_entry.c
3014
EAPI Evas_Object *
evas_object_textblock_add(Evas *e)
{
   Efl_Canvas_Text_Data *o;
   MAGIC_CHECK(e, Evas, MAGIC_EVAS);
   return NULL;
   MAGIC_CHECK_END();
   Evas_Object *eo_obj = efl_add(MY_CLASS, e,
         efl_text_multiline_set(efl_added, EINA_TRUE),
         efl_canvas_object_legacy_ctor(efl_added));
   o = efl_data_scope_get(eo_obj, MY_CLASS);
   o->legacy_newline = EINA_TRUE;
   o->auto_styles = EINA_FALSE;
   return eo_obj;
}

calling efl_text_multiline_set is safe call, since it is already called inside evas_object_textblock_add

Okay.

Then we have still one case that does not work:

"aaaa aaaa aaa aa aaa<ps/>"
         "aaaa aaa aaa aaa aaa<ps/>"
         "a aaaaa aaaaaaaaaaaaaa<br/>aaaaa<ps/>"
         "aaaaaa"

Copying this text into the entry of elementary_test, before your patch, it gets wrapped into a single line. After your patch, the entry gets multiple lines.

ali.alzyod added a comment.EditedApr 22 2019, 7:04 AM

This is strange, Why efl_text_multiline_set(EINA_FALSE) break lines if it find <ps/> !!

============================

efl_text_multiline_set(textblock,Eina_True or Eina_False);

As descriped in documentation :
Multiline is enabled or not
https://www.enlightenment.org/develop/api/ref/c/key/efl_text_multiline_set

but we found that if we pass false, line will keep breaking to multiple lines if it found <ps> tag, So what do you think ? Is this behaviour right or wrong ?