Page MenuHomePhabricator

How to handle longpress in efl_ui_text
Open, Incoming QueuePublic

Description

After applying clickable_util to efl_ui_text, longpress is not working properly.
That's because longpress_timer is not deleted when it needs to be.

There can be two ways to solve this problem.

  1. Revert back to original lonpress handling (i.e. all done by efl_ui_text by its own)
    • should work
    • be difficult to keep consistency with other widget's clicking (+ longpress) policy which is based on efl.ui.click_util.
  1. Add method for aborting longpress timer
    • need to check whether it works perfectly
    • needs to discuss that this method is proper as a part of efl.ui.clickable class (needs to think about other classes' usages together)

Related Objects

StatusAssignedTask
ResolvedNone
ResolvedNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Opencedric
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone

After reading the code of efl_ui_text:

  • One case cancels the longpress in case that a mouse move to some other element happend, i think that is equal to what @Jaehyun_Cho implemented recently. Entry how ever does that a little more "high quality" and check for the finger size of elm, we can also implement that if this is generally needed
  • The other case is in case that there is a selection in progress the longpress timer is stopped, which is IMO a perfect case of where this does make sense. So i would totally agree on adding such a API to the mixin.

After reading the code of efl_ui_text:

  • One case cancels the longpress in case that a mouse move to some other element happend, i think that is equal to what @Jaehyun_Cho implemented recently. Entry how ever does that a little more "high quality" and check for the finger size of elm, we can also implement that if this is generally needed

I will talk with @Jaehyun_Cho and try to verify whether "entry's longpress logic" can be applied to other widgets together.

  • The other case is in case that there is a selection in progress the longpress timer is stopped, which is IMO a perfect case of where this does make sense. So i would totally agree on adding such a API to the mixin.

I focused on the "definition of Efl.Ui.Clickable class", and felt little bit ambiguous that there would be a method for "longpress" for clickable feature.
Just my opinion :)

woohyun added a comment.EditedSat, Jul 13, 1:12 AM

I don't like this way in the aspect of maintenance. (Plus, I'm not sure this works for all cases with user test cases)
This is because efl_ui_text never know when the longpress callback is called and aborted.
If there would be a change on "when the longpress callback is called", your logic needs to be modified.
That is, "sd->long_pressed" can be set to EINA_TRUE almost randomly in efl_ui_text.

So, I think that "supporting a way to abort longpress timer" would be better if we will decide to use efl.ui.clickable in efl.ui.text.

How do you think about this ?

@woohyun After sleeping one night over it, I think we should add something like "abort_longpress()" to "Efl.Ui.Clickable". Other usecases:

  • Rect selection in something like item_container. When the rect selection starts, longpress should be aborted.
  • Drag and Drop: when a drag starts, we should abort the longpress.

[...]

So i like the idea of "abort_longpress" :)

ali.alzyod added a comment.EditedSat, Jul 13, 3:44 AM

I don't like this way in the aspect of maintenance. (Plus, I'm not sure this works for all cases with user test cases)
This is because efl_ui_text never know when the longpress callback is called and aborted.
If there would be a change on "when the longpress callback is called", your logic needs to be modified.
That is, "sd->long_pressed" can be set to EINA_TRUE almost randomly in efl_ui_text.

So, I think that "supporting a way to abort longpress timer" would be better if we will decide to use efl.ui.clickable in efl.ui.text.

How do you think about this ?

Names are confusing.

Do you mean cancel Long-Press Event or Timer created inside efl_text_ui?
because created timer (old timer) never initialized so removing it make sense.

This is because efl_ui_text never know when the longpress callback is called and aborted.
inside the constructor efl_event_callback_add(obj, EFL_UI_EVENT_LONGPRESSED, _long_press_cb, obj); so it know when callback is called.

If there would be a change on "when the longpress callback is called", your logic needs to be modified.
That is, "sd->long_pressed" can be set to EINA_TRUE almost randomly in efl_ui_text.

The Issue here that for desktop environment, even if long press is called, it will wait until mouse-up happened after long press.
If this behavior is changed it will be very simple to fix.

So, I think that "supporting a way to abort longpress timer" would be better if we will decide to use efl.ui.clickable in efl.ui.text.
I think right now we can cancel the event, or at least unregister/register longpress event.

How do you think about this ?
Maybe I do not understand exactly what is main problem, but if you mean ability to cancel event. I think it is good feature.

@woohyun After sleeping one night over it, I think we should add something like "abort_longpress()" to "Efl.Ui.Clickable". Other usecases:

  • Rect selection in something like item_container. When the rect selection starts, longpress should be aborted.
  • Drag and Drop: when a drag starts, we should abort the longpress. [...]

    So i like the idea of "abort_longpress" :)

Then, let'g go with that ~~~ :)

I don't like this way in the aspect of maintenance. (Plus, I'm not sure this works for all cases with user test cases)
This is because efl_ui_text never know when the longpress callback is called and aborted.
If there would be a change on "when the longpress callback is called", your logic needs to be modified.
That is, "sd->long_pressed" can be set to EINA_TRUE almost randomly in efl_ui_text.

So, I think that "supporting a way to abort longpress timer" would be better if we will decide to use efl.ui.clickable in efl.ui.text.

How do you think about this ?

Names are confusing.

Do you mean cancel Long-Press Event or Timer created inside efl_text_ui?
because created timer (old timer) never initialized so removing it make sense.

This is because efl_ui_text never know when the longpress callback is called and aborted.
inside the constructor efl_event_callback_add(obj, EFL_UI_EVENT_LONGPRESSED, _long_press_cb, obj); so it know when callback is called.

If there would be a change on "when the longpress callback is called", your logic needs to be modified.
That is, "sd->long_pressed" can be set to EINA_TRUE almost randomly in efl_ui_text.

The Issue here that for desktop environment, even if long press is called, it will wait until mouse-up happened after long press.
If this behavior is changed it will be very simple to fix.

So, I think that "supporting a way to abort longpress timer" would be better if we will decide to use efl.ui.clickable in efl.ui.text.
I think right now we can cancel the event, or at least unregister/register longpress event.

**How do you think about this ?**

Maybe I do not understand exactly what is main problem, but if you mean ability to cancel event. I think it is good feature.

Sorry for giving confusion ~
Anyway, my idea was that we didn't need to make a temp version of patch.
I did think we can make a perfect patch after having discussion on "aborting longpress".

After @bu5hm4n adds "abort_longpress", let's use it in efl_ui_text :)

(Thanks for sharing your idea)