Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LV_EVENT_CLICKED not generated when it is supposed to be. #6123

Open
kdschlosser opened this issue Apr 24, 2024 · 20 comments
Open

LV_EVENT_CLICKED not generated when it is supposed to be. #6123

kdschlosser opened this issue Apr 24, 2024 · 20 comments

Comments

@kdschlosser
Copy link
Contributor

LVGL version

master branch

What happened?

I don't know if this is with just the slider or if it is with all widgets I have not looked into it that far.

I have created a slider and I am outputting all of the events that happen when that slider is running. The output seen here if from a single touch, hold and then release. Notice the alignment of the events specifically the CLICKED event

event: REFR_EXT_DRAW_SIZE
event: SIZE_CHANGED
event: REFR_EXT_DRAW_SIZE
event: GET_SELF_SIZE
event: GET_SELF_SIZE
event: DRAW_MAIN_BEGIN
event: DRAW_TASK_ADDED
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: PRESSED                 <===========The CLICKED event should take place near the PRESSED event
event: FOCUSED
event: PRESSING
event: DRAW_MAIN_BEGIN
event: DRAW_TASK_ADDED
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: PRESSING
event: DRAW_MAIN_BEGIN
event: DRAW_TASK_ADDED
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END
event: PRESSING
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: PRESSING
event: DRAW_MAIN_BEGIN
event: DRAW_TASK_ADDED
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: PRESSING
event: DRAW_MAIN_BEGIN
event: DRAW_TASK_ADDED
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED_REPEAT
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED_REPEAT
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED_REPEAT
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED_REPEAT
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED_REPEAT
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED_REPEAT
event: PRESSING
event: PRESSING
event: PRESSING
event: PRESSING
event: LONG_PRESSED_REPEAT
event: PRESSING
event: PRESSING
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: VALUE_CHANGED
event: RELEASED
event: CLICKED        <=============== Happening after the RELEASE event. Not sure why.. 
event: DRAW_MAIN_BEGIN
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: DRAW_MAIN_BEGIN
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END
event: REFR_EXT_DRAW_SIZE
event: REFR_EXT_DRAW_SIZE
event: DRAW_MAIN_BEGIN
event: DRAW_MAIN
event: DRAW_MAIN_END
event: DRAW_POST_BEGIN
event: DRAW_POST
event: DRAW_POST_END

How to reproduce?

make a slider and attach an event callback with the "ALL" event and output the events.

@kdschlosser
Copy link
Contributor Author

I am now looking at this and I am thinking that "CLICKED" means a press and release has occurred.

@kisvegabor
Copy link
Member

A Click is really a Press followed by a Release. Is there any issue with that?

@kdschlosser
Copy link
Contributor Author

Docs should be updated to say that. I realized that after I made this issue but I left the issue because the docs need to be updated.

PRESSED gets fired on mouse button down or touch
RELEASED gets fired on mouse button up and touch release
CLICKED is fired on mouse button up and touch release

CLICKED is redundant to RELEASED because you cannot have a RELEASED unless there was a PRESSED which infers CLICKED.

If PRESSED and RELEASED can occur if any part of a widget is touched or clicked on and CLICKED only occurs on a part of the widget that is supposed to allow user interaction.. That would make sense, I don't think this is the case tho.

As an example say we have a slider and the only part of the widget that has any kind of user interaction would be the knob and nothing else. If I clicked on the indicator there would be a PRESSED and RELEASED event but no CLICKED event. Now if touch the knob all 3 events would get fired.

I know that the slider doesn't work like that but if it did work like that...

Another example is an image. an image doesn't have any user control so it should never generate a CLICKED event if it is touched. It should generate the PRESSED and RELEASED events.

CLICKED should only get fired if what is being touched has some kind of mechanics to it for user interaction.

@kisvegabor
Copy link
Member

kisvegabor commented Apr 29, 2024

CLICKED is redundant to RELEASED because you cannot have a RELEASED unless there was a PRESSED which infers CLICKED.

They are almost the same, but e.g. if you scroll a list, by dragging a button and release it, RELEASED will be triggered but CLICKED won't.

The user can attach any kind of events to the widgets, They can do something image click too. Why not?

@kdschlosser
Copy link
Contributor Author

I get what you are saying. The documentation needs to be updated to explain the CLICKED event better. It needs to describe when the CLICKED event get fired. So when dragging no clicked event occurs only when it is a press and release with no position change.

@kisvegabor
Copy link
Member

It's documented here and here too.

@kdschlosser
Copy link
Contributor Author

It should be a little more explicit. Something to the effect of "This event on differs from the RELEASED event in that it will not get triggered if anything was scrolled." This tells us that all aspects of the event will work the same as the RELEASED event except for not being triggered if scrolling occurred.

I personally think this even should only get triggered if a user controllable part of a widget is what got clicked on. If a user wanted to react to an lv_image being clicked on they can use the RELEASED event for that purpose.

Question tho. If there a function that can be used that takes an event as a parameter to determine if any scrolling has occurred? For the purposes of the RELEASED event if any scrolling has taken place from the time the PRESSED event occurred and the RELEASED event occurred the function would return true

I know that it would not be something you would want to change the behavior of but it does make more sense to have it function in this manner. It is something that the user would be able to use when using lv_obj_t as a container to build a kind of custom widget. It would allow them to react to user input that matters for all of the children in that container without having to target specific objects.

Example:

void event_cb(lv_event_t *e) 
{
    if (lv_obj_has_class(lv_event_get_target(e), lv_button_class)) {
        // CLICKED EVENT OCCURRED
        // If the CLICKED event only happened on things that were able to be interacted with by the user
        // The above test would not need to take place. 
    }
}


lv_obj_t *create_widget(void) 
{
     lv_obj_t * container = lv_obj_create(lv_screen_active(NULL));

    lv_obj_t *text = lv_label_create(container);
    lv_obj_t *button = lv_button_create(container);
    lv_obj_t *label = lv_label_create(button);
    lv_obj_set_text(label, "BUTTON");
    lv_obj_center(label);
    lv_obj_center(button);
    lv_obj_set_text(text, "THIS IS SOME TEXT");
    lv_obj_align(text, LV_ALIGN_CENTER, 0, -25);
    lv_obj_add_event_cb(container, &event_cb, LV_EVENT_CLICKED, NULL);
    
    return container;
}

lv_obj_t *widget = create_widget();

@kisvegabor
Copy link
Member

The logic of the CLICKED event wasn't criticized so far, so wouldn't touch it now.

You can use lv_indev_get_scroll_obj(lv_indev_active()) to get scrolled object in an event if any.

@kdschlosser
Copy link
Contributor Author

I know it was done how it was done a long time ago..

I know that LVGL tries to model it's behavio inline with what is seen in a web browser. Are there clicked events that get triggered when a user clicks on an object that isn't "clickable".... Even if that object has a callback registered to the clicked event??

Take a look at the DIV object. It is an HTML object, You can set it's size and apply styles to it. for all intents and purposes it is the equivalent of an lv_obj_t. There is something it doesn't do, it doesn't generate clicked events. You cannot even force it to programmatically, it will give you an error. That is because it is not an object that the user can interact with. Now I am not saying to lock it out all together. There should be a way for widgets to define what can and cannot be clicked and the areas that cannot be clicked when clicked on don't generate the clicked event.

@kisvegabor
Copy link
Member

It seems an onclick event can be added to a div without any issues: https://codepen.io/kisvegabor/pen/oNRvGEJ

@kdschlosser
Copy link
Contributor Author

I misread. a div element doesn't have the click function.

Let me try and attack this from a different angle...

If I remove the LV_OBJ_FLAG_CLICKABLE flag from an object does it stop the following events from getting triggered??

  • LV_EVENT_PRESSED
  • LV_EVENT_PRESSING
  • LV_EVENT_LONG_PRESSED
  • LV_EVENT_LONG_PRESSED_REPEAT
  • LV_EVENT_SHORT_CLICKED
  • LV_EVENT_CLICKED
  • LV_EVENT_RELEASED

or does it only stop only these events from occurring?

  • LV_EVENT_SHORT_CLICKED
  • LV_EVENT_CLICKED

@kisvegabor
Copy link
Member

From the indev's point of view the object will be invisible. So none of the touch related events will be fired.

@kdschlosser
Copy link
Contributor Author

Essentially what you are saying is if CLICKED gets disabled the PRESSED event which could be for the purposes of scrolling would not get triggered?

@kisvegabor
Copy link
Member

True. Something is either touchable or not.

@kdschlosser
Copy link
Contributor Author

I am going to have to run some tests to see what the behavior is.

@kdschlosser
Copy link
Contributor Author

OK so if I clear the clickable flag I am not able to scroll. hover events stop, pressing a dragging stops. That is not correct. A click is defined as pressing and releasing without changing position. It has nothing to do with scrolling, or hovering or pressing and dragging. Those events should not stop when the CLICKABLE flag has been removed.

@kisvegabor
Copy link
Member

I think the root of the confusion is bad naming of CLICKABLE. I think it should be TOUCHABLE.

@kdschlosser
Copy link
Contributor Author

kdschlosser commented May 21, 2024

if it stops the object from being touchable at all then yes that is what it should be named. But there still should be a clickable flag so that say a button cannot be "click" but it can be touched, something like say scrolling. This would be a handy thing to use to control the scrolling of something like say a spinning list where the only thing you want to be clickable would be a button that is directly in the center. the rest that are visible you would unset the clickable flag so those buttons cannot be pressed but a user would still be able to do things like touch it to scroll or touch it to move it.

It would be a good way for a user to disable the ability to press something but still allow the ability touch to move an icon or something along those lines.

@kisvegabor
Copy link
Member

If we added the CLICKABLE flag like this we needed to add PRESSABLE, LONG_PRESSABLE, etc too. Therefore I think the logic that you have described should be handled in the event callback instead. E.g.

void button_click_event_cb(lv_event_t * e)
{
  if(is_in_right_position(lv_event_get_target(e) == false) return;

  printf("Clicked\n");
} 

@kdschlosser
Copy link
Contributor Author

The issue is when you turn off the clicking it turns off all interactions including hovering. All events stop not just the click event.

if clicking gets turned off I should still be able to scroll as that is not a click. a click is defined as a press and release without any movement. Scrolling is a press and drag which means it is not a click.

Clicking is the correct event name for what is occurring but it is like the parent event and all other events are a child of it so when the parent is disables the children also are disabled. It should not be that way. Click should be the parent of long click, double click, left click, right click, etc... It should not be the parent of click and drag or scroll up, down, left or right. Even tho the name click and drag infers a click is taking place that's not really what is happening. It should be called "press" and drag but that doesn't sound as good as click and drag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants