Page MenuHomePhabricator

efl.input.clickable
Closed, ResolvedPublic

Description

mixin Efl.Input.Clickable @beta
├ (M) press :: @protected
├ (M) unpress :: @protected
├ (M) button_state_reset :: @protected
├ (E) clicked
├ (E) clicked,any
├ (E) pressed
├ (E) unpressed
├ (E) longpressed

Action Image

  • rename to efl.input.clickable and locate it to "src/lib/evas/canvas"(@woohyun)
  • add "efl_input_clickable_longpress_abort" method (@woohyun)
  • create "efl.ui.action_connector", and add features of "efl.ui.clickable_util" here (@woohyun)
bu5hm4n created this task.May 3 2019, 10:54 AM
bu5hm4n triaged this task as TODO priority.
bu5hm4n updated the task description. (Show Details)May 18 2019, 12:39 PM
zmike added a comment.Jun 12 2019, 9:52 AM

It feels a bit weird to split the clicked event between [primary button] and [any button]?

zmike moved this task from Backlog to Evaluating on the efl: api board.Jun 12 2019, 9:52 AM

Everything else requires you to check the button that you have clicked *every time* you attach to this event. Where our main usage is, that only the primary button is interesting. This is sort of a compromise between usability and smallness.

zmike added a comment.Jun 14 2019, 7:10 AM

After some thought, I think maybe an improvement on this could be using changed and changed,primary, where the latter indicates the left button has been pressed on a standard mouse configuration. This more definitively handles the case of e.g., left-handed mouse configuration for Xorg/Wayland and we can mention that the primary event will continue to work for right-handed configurations.

I like "primary" more than "any", yep.

It was also brought up that it is confusing to have this in the ui namespace, we should probebly move this to the gfx namespace ...

@zmike changed and changed,primary is more obvious than clicked and clicked,any ???

Hm I suppose it makes sense to move it, but then why not have it be efl.input.clickable to go with efl.input.interface?

I meant clicked and clicked,primary, I'm in too many tickets at once...

efl.input.clickable sounds reasonable, same for the new event names. @woohyun, also okay for you ?

@bu5hm4n

I also feel good with "efl.input.clickable" :)
It should be better than a name with gfx.

bu5hm4n updated the task description. (Show Details)Jun 25 2019, 9:33 AM
bu5hm4n updated the task description. (Show Details)
bu5hm4n updated the task description. (Show Details)
woohyun updated the task description. (Show Details)Jul 15 2019, 1:43 AM
woohyun updated the task description. (Show Details)Jul 15 2019, 6:31 PM

@bu5hm4n

Before beginning to modify codes, I have some questions.

  1. Can this class be existed under elementary folder after changing to efl_input_clickable ? OR need to move to another folder such as Evas ?
  1. Does efl_ui_clickable_util also need to be changed to efl_input_clickable_util ?
  1. Is "efl_input_clickable_longpress_abort" good as a new method's name ?

to 1.: i think moving it back to src/lib/efl for this, just for the sake of having efl_input_* in similar places.

to 2.: i think keeping it in efl_input_clickable_util sounds fine to me. It defines the interaction via a theme, which is kind of high-level. But this is my opinion @segfaultxavi @zmike what are yours? I think we can just leave this class for now where it is, moving it later on should be easy :)

to 3.: Sounds good to me :) renaming it later on should also be easier. @segfaultxavi you are always good at naming, opinions ?

segfaultxavi added a comment.EditedJul 16 2019, 3:27 AM
  1. I think @bu5hm4n meant that src/lib/evas/canvas is the right place now, along with the rest of efl_input_* files.
  2. I think having Efl.Input.Clickable but Efl.Ui.Clickable_Util is pretty confusing. We should either put them in the same namespace or rename Clickable_Util to something else.
  3. Yeah, this is a good name, since I managed to find out the method's purpose just from the name :)

@bu5hm4n @segfaultxavi @zmike

For #2, we can choose one of below candidates.

  1. "Efl.Input.Clickable_Util" : Put this in the same namespace with "Efl.Input.Clickable"
  2. "Efl.Ui.Click_Event_Binder" : Put this in elementary (i.e. locate it away from "Efl.Input.Clickable"

I know "Efl.Ui.Click_Event_Binder" is not good name (plz help me), but I think the second way looks better.

How do you think about this ?

I totally agree. #2 sounds good. Some names with Bridge, Bind & Theme/Object sound good ?

We already have an Efl.Ui.Property_Bind. This should be something like Efl.Ui.Click_Event_Bind.
Or, we can try to be more generic and call it Efl.Ui.Event_Bind, so later we can add other "event binders" if we find any. If we do so, the methods have to be renamed to bind_clickable_to_theme and bind_clickable_to_object.

bu5hm4n added a comment.EditedJul 17 2019, 2:11 AM

Mhm, a second thought about clickable util:
The class only really connects objects and and themes to the implementation, saying that such a util is bound to clickable is not really true, scrollable_util is also doing the same thing.
How about we create a class "Action_Connector", which contains such methods for clickable, scrollable, and whatever comes next? We would have a central place for such "connectors" ?

@bu5hm4n @segfaultxavi

I agree with that idea - and I hope it could be generated as STATIC class in other bindings (such as C#).

(I'm not sure whether discussing about "new eo class type for STATIC class" here is proper)

Anyway, I also like the idea to create "Action_Connector".

woohyun added a comment.EditedJul 24 2019, 9:15 PM

@bu5hm4n @segfaultxavi

I have been talking about STATIC class with @felipealmeida, and will create another task to talk about this deeply.

So, before doing implementation, I want to get define the action items like -

  1. Replace the name to "efl.input.clickable" and locate it to "src/lib/evas/canvas"
  2. Add new method (= "efl_input_clickable_longpress_abort")
  3. Create "efl.ui.action_connector", and add features of "efl.ui.clickable_util" here

Absolutly correct, feel free to add them to the task description, the not so exact ones are already there ... :)

woohyun updated the task description. (Show Details)Jul 25 2019, 1:46 AM

@bu5hm4n @segfaultxavi @zmike

While doing the work items, I have two questions.

  1. clicked -> clicked,primary clicked,any -> clicked

    OR

    clicked -> clicked clicked,any -> clicked,primary

    If the first one is right, then I'm worrying that can give confusion to developers who are much familiar with "Clicked" event.
  1. Are "item,clicked + item,clicked,primary" needed to be changed together ? I think they should be ~

To 1.: The first one is what was intended. primary here refers to the primary button of your mouse.

To 2.: If you want you can, but I can also do that, it should only be in efl_ui_collection.

@bu5hm4n

Oh, I was confused a bit.

There would be no problem that a touch based application would use "Clicked" for their button or something else (instead of Clicked_Primary),
because any button's press + unpress will give "Clicked" well.

Is my understanding right ?

Yes, that is correct. However: the event Clicked will also be emitted when a secondary button event is emitted.

@bu5hm4n

Right.
So, my question was that it could be confused to the developers who are familiar with "clicked" (used as clicked,primary in the other platforms) .
Because they will use "Clicked" event without any doubt.

Because of lack of knowledge, I'm not sure how other platforms are supporting "clicked".
But, I have not seen "Clicked,Primary" similar events from other platforms ever.

Ah! That is actually a good point. Maybe we should discuss that event name a little bit more ? @segfaultxavi @zmike

Yeah, it's a good point. Maybe we should not rename clicked after all, and leave clicked,any as it is too.
Pressing on clickable objects (like buttons) with anything other than the primary mouse button should be a rare operation, so the clicked,any event won't be used much.

woohyun updated the task description. (Show Details)Jul 28 2019, 3:20 PM

I made a patch for work item #1. (D9427)
After this patch, I'll make two patches additionally for #2 and #3 at the same time.

woohyun updated the task description. (Show Details)Jul 29 2019, 5:59 PM
bu5hm4n renamed this task from efl.ui.clickable to efl.input.clickable.Jul 31 2019, 4:08 AM
bu5hm4n updated the task description. (Show Details)
woohyun updated the task description. (Show Details)Aug 1 2019, 6:23 PM
bu5hm4n updated the task description. (Show Details)Aug 5 2019, 8:46 AM

this looks fine now ?

bu5hm4n moved this task from Evaluating to Stabilized on the efl: api board.Aug 5 2019, 11:06 AM

I hope so :) no more feedback ?