efl_ui_clickable is now a mixin. The mixin now brings two APIs the press
and unpress API can be used to tell the implementation the state of the
presses. Within the implementation the calls to press / unpress are then
converted to longpress / clicked events.
Details
Details
Diff Detail
Diff Detail
- Repository
- rEFL core/efl
- Branch
- devs/bu5hm4n/work
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 11368
Comment Actions
I see the changes to Efl.Ui.Button are in the next commit.
| src/lib/elementary/efl_ui_clickable.c | ||
|---|---|---|
| 50 | I'd feel safer if you check first if state->timer is not NULL. Users of this mixin might call you twice without calling _unpress... | |
| 51 | This interval (1 second) is hardcoded here? shouldn't it be #defined at the top of the file, or be a configurable value? | |
| 68 | Again, I do not like hardcoded values hidden in the code :) | |
| 75 | I'd feel safer if you checked first that this is not NULL, in case the user called _unpress twice. | |
| src/lib/elementary/efl_ui_clickable.eo | ||
| 2 | Event structures should be suffixed _Event. | |
| 25 | Docs for this event? I do not see the difference with clicked. | |
| src/lib/elementary/efl_ui_image_zoomable.c | ||
| 864–869 | Why don't you use the new _press and _unpress method from the mixin instead of manually emitting the new event? | |
| src/lib/elementary/efl_ui_clickable.eo | ||
|---|---|---|
| 2 | You are right, we have released event structs without _Event and I now think that's bad. | |
Comment Actions
Rebase.
| src/lib/elementary/efl_ui_clickable.c | ||
|---|---|---|
| 24 | We measure *everything* in efl in seconds ... + this is a double. i don't think anyone will measure ms in doubles ... | |
Comment Actions
So, it is now commented that it is seconds. Additionally, there is now a info printing on non-legacy widgets. Which is mega usefull for debugging.
| src/lib/elementary/efl_ui_clickable.c | ||
|---|---|---|
| 57 | This should also add a mouse-out event on the object and terminate the longpress timer if it is reached. | |
| 70 | Unpress events can occur on objects which weren't originally pressed, this check should be removed. | |
| 92 | This event should only occur if the object was previously pressed. | |
| src/lib/elementary/efl_ui_clickable.eo | ||
| 32 | The descriptions for these should be more explicit like "when an object is pressed and then unpressed by the same button". | |
Comment Actions
The Overall issue here is that only the text widget / button / panes was protected against the "Press->Mouse-out-of-object->Unpress" scenario. The others will do weird things, and for example emit the longpress event, for no reason. I will solve this with adding a abort method to the clickable mixin. This can then be called when the mouse leaves a object. This will reset the internal state. I don't want to solve it with a event handler in the mixin because of two reasons:
- Only works for non-theme based listening
- Clickable does not use events at all.
| src/lib/elementary/efl_ui_clickable.c | ||
|---|---|---|
| 57 | Mhm i am not so sure if i want to solve it that way, right now the clickable implementation only contains the state. No event handling code or so. I think i will keep it that way, and add another function to it. Which must be called when the longpress stuff etc. should be canceled | |
| 70 | True. | |
| 92 | True. | |
Comment Actions
So, with the latest update, most widgets do work again correctly.
So when a mouse down is followed by the mouse going out of the element, then press and unpress will be emitted (from edje) but no clicked event is emitted from the implementation. Which seems correct.
Additionally, this here now also brings to more class functions. Which can be used to wire up the events from the theme to the object, or from a input object to the object. The input object right now has some issues, but i want to first speak about this approache and the problems i am having. I don't know if i just hit a evas bug, or if i made a general mistake.
Comment Actions
Looks better, some minor changes still.
| src/lib/elementary/efl_ui_clickable.eo | ||
|---|---|---|
| 31 | This should be renamed button_state_reset or similar. abort is a function call which stops program execution, and we don't want any confusion related to that function. | |
| src/lib/elementary/efl_ui_clickable_util.eo | ||
| 3 ↗ | (On Diff #22169) | I think this should use bind instead of link. Also this should state explicitly which signals are being handled. |
| 13 ↗ | (On Diff #22169) | Same as above. |
| src/lib/elementary/efl_ui_clickable_util.eo | ||
|---|---|---|
| 13 ↗ | (On Diff #22173) | Shouldn't this be bind_to_object? How does this even build? |
