Page MenuHomePhabricator

elementary entry: add elm_entry_cursor_coord_set/get
Needs RevisionPublic

Authored by id213sin on Sep 29 2016, 4:07 AM.

Details

Summary

These are necessary functions for controlling cursor
based on coordinate. It is useful when a touch event
position on another object has to be passed to entry manually.
@feature

Test Plan

Run [elementary_test -to "entry cursor coord"]

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2602
Build 2667: arc lint + arc unit
id213sin updated this revision to Diff 9942.Sep 29 2016, 4:07 AM
id213sin retitled this revision from to elementary entry: add elm_entry_cursor_coord_set/get.
id213sin updated this object.
id213sin edited the test plan for this revision. (Show Details)
id213sin added reviewers: raster, cedric, herdsman, tasn.
id213sin added a subscriber: Blackmole.
id213sin updated this object.Sep 29 2016, 4:23 AM
cedric edited edge metadata.Nov 28 2016, 11:16 AM

@herdsman and @tasn: could you review this ?

herdsman edited edge metadata.EditedNov 29 2016, 12:59 AM

Hi, thanks for the hard work.
Can you elaborate on the changes made in _edje_entry_cursor_coord_set?
It's already being used, and I'm wondering how this is going to affect current behavior.

@herdsman
Sorry for late response. :(
I made change on _edje_entry_cursor_coord_set() to update cursor informations in IMF (software keyboard) and
call "cursor,changed" smart callback from elm_entry.

@herdsman
Please, review this patch when you have time. :)
Currently, we need this APIs for Tizen 4.0.

@id213sin , I want to understand your usecase better.
Why do we need this so high-up in Elementary?
The "touch events" sounds low-level enough for this to be handled in Edje.
Please elaborate.

jpeg requested changes to this revision.Jun 14 2017, 10:07 PM

Looks like you just want to implement Efl.Text.Cursor.cursor_coord.
That being said, that elm_entry is not public EO, only legacy API.

src/lib/elementary/elm_entry.c
4350

Looks like it should return x+w/2, y+h/2, no?

This revision now requires changes to proceed.Jun 14 2017, 10:07 PM

@herdsman
In Tizen, we already have some use cases for changing cursor position according to mouse touch event on other widget or rect.
I didn't provided public API for elm_entry about this use cases.
But, application did this job with getting Textblock from elm_entry. And It was dirty.
I thought the Cursor things should be inherited from Textblock to elm_entry when we build interfaces.
Then, cursor_coord_set also would be inherited to elm_entry.
Why should we block or hide this thing?

If the API is not accepted, we are going to make Tizen only APIs for this requirements for Tizen 4.0. :(

@jpeg
Then, should I need to implement these things to Efl.Text.Cursor.cursor_coord and make some legacy wrapper function?

src/lib/elementary/elm_entry.c
4350

I think we don't need to modify cursor coordinates to place it a middle of cursor.
As I know cursor coord is same as cursor geometry's xy.
If we modify this value, it means we are going to make new concept of cursor coord.

jpeg added a comment.Jul 6 2017, 10:13 PM

@herdsman
In Tizen, we already have some use cases for changing cursor position according to mouse touch event on other widget or rect.
I didn't provided public API for elm_entry about this use cases.
But, application did this job with getting Textblock from elm_entry. And It was dirty.
I thought the Cursor things should be inherited from Textblock to elm_entry when we build interfaces.
Then, cursor_coord_set also would be inherited to elm_entry.
Why should we block or hide this thing?

If the API is not accepted, we are going to make Tizen only APIs for this requirements for Tizen 4.0. :(

And that is a poor decision.

@jpeg
Then, should I need to implement these things to Efl.Text.Cursor.cursor_coord and make some legacy wrapper function?

I think @herdsman knows better what to do here. But to me this API in elm_entry does not matter too much, and it's better to keep it only for elm_entry for now (like this patch is doing).

@jpeg
As I know, Efl.Ui.Text implements all of cursor interfaces and it will include cursor_coord_set.
Why it shouldn't be in legacy?

Hermet added a subscriber: Hermet.Jun 6 2018, 6:49 PM

hey, @herdsman no further opinions?