Page MenuHomePhabricator

efl_ui_text: avoid infinite loop when long pressing
ClosedPublic

Authored by woohyun on Wed, Jul 10, 3:03 PM.

Details

Summary

Long pressing on efl_ui_text gave inifinte loop. So, I removed
event_call in the event callback function.
I think long press functionality is almost broken after applying
clickable_util. There is no way to remove long press timer based
on the action on efl_ui_text.

Test Plan
  1. elementary_test
  2. Efl.Ui.Text
  3. long press on the text

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.
woohyun created this revision.Wed, Jul 10, 3:03 PM

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/

woohyun requested review of this revision.Wed, Jul 10, 3:03 PM

@bu5hm4n

If efl_ui_text needs to control its own long-press logic, does it need to implement efl_ui_clickable by itself ?
(I'm not sure there is a way to implement this because efl_ui_clickable is not an interface but a mixin.)

I don't think efl_ui_clickable_util cannot support long-press feature in efl_ui_text perfectly.

Mhm, text indeed looks weird in that regard.

Maybe we should revert my commit that made it use efl_ui_clickable_util ... ? If we revert my commit, then text basically implements its own clickable logic.

The other possibility is to add a method to clickable to abort the longpress timer, would that be okay for you ?

meson_options.txt
129 ↗(On Diff #23222)

? :-D

I noticed one issue, where after long press.
Any click will show the context menu (I think it detects sinlge click as long press), may be this is old issue

woohyun added inline comments.Thu, Jul 11, 2:55 AM
meson_options.txt
129 ↗(On Diff #23222)

Woops. I'm sorry. :(

Mhm, text indeed looks weird in that regard.

Maybe we should revert my commit that made it use efl_ui_clickable_util ... ? If we revert my commit, then text basically implements its own clickable logic.

The other possibility is to add a method to clickable to abort the longpress timer, would that be okay for you ?

I think "aborting the longpress timer" will work for efl_ui_text.
But, I am not sure whether it's something proper for efl.ui.clickable class.

I noticed one issue, where after long press.
Any click will show the context menu (I think it detects sinlge click as long press), may be this is old issue

That is the issue that @bu5hm4n and I are talking together :)

That is happened because longpress_timer is not deleted properly.

woohyun updated this revision to Diff 23236.Thu, Jul 11, 3:17 AM

removed change on meson_option.txt

woohyun updated this revision to Diff 23237.Thu, Jul 11, 3:18 AM

remove changes on meson_options.txt

@woohyun

The Issue I am taling about is after applying the patch. (So are there more things to do to clear the times ?)

@woohyun

The Issue I am taling about is after applying the patch. (So are there more things to do to clear the times ?)

Right - those more things are related to the above discusstion about "how to remove longpress_timer" :)

bu5hm4n accepted this revision.Thu, Jul 11, 4:36 AM
This revision is now accepted and ready to land.Thu, Jul 11, 4:36 AM

@woohyun Can you create a task so we can discuss that further ?

Closed by commit rEFL3798356819bf: efl_ui_text: avoid infinite loop when long pressing (authored by woohyun, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyThu, Jul 11, 4:40 AM
This revision was automatically updated to reflect the committed changes.

@woohyun Can you create a task so we can discuss that further ?

Let's discuss this in T8045.