Page MenuHomePhabricator

efl.ui.text : Fixing cursor movement using keyboard arrows/mouse click
ClosedPublic

Authored by AbdullehGhujeh on Apr 22 2020, 2:34 AM.

Details

Summary

if we have an emoji or a cluster combining multiple Unicode inside ui textbox, we can move the cursor inside the emoji/cluster using keyboard arrows/mouse click.
so we should use cluster movement instead of character movement (same as entry).

this should resolve T8666

Test Plan
#define EFL_EO_API_SUPPORT 1
#define EFL_BETA_API_SUPPORT 1

#include <Efl_Ui.h>

static void
_gui_quit_cb(void *data EINA_UNUSED, const Efl_Event *event EINA_UNUSED)
{
   efl_exit(0);
}

static void
_gui_setup()
{
   Eo *win, *box;

   win = efl_add(EFL_UI_WIN_CLASS, efl_main_loop_get(),
                 efl_ui_win_type_set(efl_added, EFL_UI_WIN_TYPE_BASIC),
                 efl_text_set(efl_added, "Hello World"),
                 efl_ui_win_autodel_set(efl_added, EINA_TRUE));

   // when the user clicks "close" on a window there is a request to delete
   efl_event_callback_add(win, EFL_UI_WIN_EVENT_DELETE_REQUEST, _gui_quit_cb, NULL);

   box = efl_add(EFL_UI_BOX_CLASS, win,
                efl_content_set(win, efl_added),
                efl_gfx_hint_size_min_set(efl_added, EINA_SIZE2D(360, 240)));

   efl_add(EFL_UI_TEXTBOX_CLASS, box,
           efl_gfx_hint_weight_set(efl_added, 1.0, 1.0),
           efl_gfx_hint_align_set(efl_added, 1.0, 1.0),
           efl_text_markup_set(efl_added, "A&#x262a;&#xfe0f;"),
           efl_pack(box, efl_added));
}

EAPI_MAIN void
efl_main(void *data EINA_UNUSED, const Efl_Event *ev EINA_UNUSED)
{
   _gui_setup();
}
EFL_MAIN()

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
AbdullehGhujeh created this revision.Apr 22 2020, 2:34 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/

AbdullehGhujeh requested review of this revision.Apr 22 2020, 2:34 AM
AbdullehGhujeh edited the summary of this revision. (Show Details)Apr 22 2020, 5:21 AM
AbdullehGhujeh edited the test plan for this revision. (Show Details)
AbdullehGhujeh added a reviewer: ali.alzyod.
AbdullehGhujeh edited the test plan for this revision. (Show Details)
zmike added a subscriber: zmike.Apr 22 2020, 6:10 AM

It seems like the test case provided could be made into a unit test which programmatically moves the text cursor?

It seems like the test case provided could be made into a unit test which programmatically moves the text cursor?

to reproduce you need to run the code then click or use arrow from keyboard.
I changed the function that will be called when this happens(from character function to cluster function).

zmike added a comment.Apr 22 2020, 7:21 AM

We already have unit tests which trigger key and mouse events.

ali.alzyod accepted this revision.EditedApr 24 2020, 4:58 AM
ali.alzyod added reviewers: zmike, woohyun, bu5hm4n.

This seems fine to me, thank you
I hope the test will work for all machines (they have emoji font)

This revision is now accepted and ready to land.Apr 24 2020, 4:59 AM
zmike requested changes to this revision.Apr 24 2020, 8:53 AM
zmike added inline comments.
src/tests/elementary/efl_ui_test_text.c
496

The win object should have a matching size here since it's impossible to click a 1x1 canvas.

498

Objects are visible by default with unified API, so these are unnecessary.

508

It seems to me like this doesn't actually need a mainloop iteration since the event processing and changes here are all immediate?

518

click_object_at(win, rc.x + (rc.w/2), rc.y+(rc.h/2)); instead of all this for consistency.

522

This seems to be the same as above; if there's no specific need for evas_render to be called, then this is unnecessary, but if evas_render is necessary then this needs to use get_me_to_those_events();

This revision now requires changes to proceed.Apr 24 2020, 8:53 AM
zmike added a comment.Apr 24 2020, 8:55 AM

Good work on the test, some minor cleanups and it looks ok to me

update test per requested changes

AbdullehGhujeh marked 5 inline comments as done.Apr 27 2020, 4:41 AM
AbdullehGhujeh added inline comments.
src/tests/elementary/efl_ui_test_text.c
496

set same size for win.

498

removed.

508

removed

518

I have replaced it.

522

seems like I need it after setting the size & replaced by get_me_to_those_events();

zmike added a comment.Apr 27 2020, 7:12 AM

I get this error if I try to run the test:

../src/tests/elementary/efl_ui_test_text.c:494:F:efl_ui_text:text_emoji_cursor_movement:0: Assertion '3 == efl_text_cursor_object_position_get(cursor)' failed: 3 == 3, efl_text_cursor_object_position_get(cursor) == 1
AbdullehGhujeh marked 5 inline comments as done.Apr 28 2020, 3:51 AM

@zmike
it is working fine on my machine,
do you have emoji font like NotoColorEmoji.ttf ?

zmike added a comment.Apr 28 2020, 6:58 AM

Seems to be installed as part of google-noto-emoji-color-fonts-20190829-1.fc31.noarch on my machine. I'm running it through travis now to confirm https://travis-ci.org/github/Enlightenment/efl/builds/680581257

AbdullehGhujeh retitled this revision from efl.ui.text : Fixing cursor movement with emoji using keyboard arrows/mouse click to efl.ui.text : Fixing cursor movement using keyboard arrows/mouse click.May 5 2020, 12:08 AM
AbdullehGhujeh edited the summary of this revision. (Show Details)
  • update test to use text cluster instead of emoji for more consistency

@zmike can you please confirm if latest update fix the test failing

ali.alzyod accepted this revision.Jun 11 2020, 12:24 AM

this seems fine to me, as long it pass test for others (I think it will pass since it is not dependent on emoji font anymore)

woohyun accepted this revision.Oct 20 2020, 3:21 AM

This also seems ok for me :)

This revision was not accepted when it landed; it landed in state Needs Review.Oct 20 2020, 3:31 AM
Closed by commit rEFLd6a6dd54a13b: efl.ui.text : Fixing cursor movement using keyboard arrows/mouse click (authored by abdulleh Ghujeh <a.ghujeh@samsung.com>, committed by woohyun). · Explain Why
This revision was automatically updated to reflect the committed changes.