efl.ui.textbox: replace hoversel with popup
I think this may need some changes, please let me know what you think
Details
Details
- Reviewers
woohyun bu5hm4n segfaultxavi zmike - Maniphest Tasks
- T8522: Efl.Ui.Textbox class
- Commits
- rEFL1a1868ce57a3: efl.ui.textbox: replace hoversel with popup
Diff Detail
Diff Detail
- Repository
- rEFL core/efl
- Branch
- arcpatch-D11072
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 15377 Build 10568: arc lint + arc unit
Comment Actions
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/
Comment Actions
That's a good start, a few things should be fixed however.
| src/lib/elementary/efl_ui_textbox.c | ||
|---|---|---|
| 1060 | This will not work with translation, text will not be Copy for example. | |
| 1109 | can you rename to hoversel to popup ? | |
| 1123 | Mhm, what do you think of listening to the selection changed events of the items in the list, that would safe you the code above that needs to do string compares. | |
| src/lib/elementary/efl_ui_textbox.c | ||
|---|---|---|
| 1123 | are their events for Item click? | |
| src/lib/elementary/efl_ui_textbox.c | ||
|---|---|---|
| 1123 | EFL_UI_EVENT_SELECTED_CHANGED is emitted on the *item* object. | |
Comment Actions
I notice that EFL_UI_POPUP_EVENT_BACKWALL_CLICKED does not work with right mouse click
Comment Actions
I am not sure if the backwall should emit clicked when rightclicked, it normally is just meant to react on left click.
This looks like a good start, a few more things then we are good to go i guess. With the changes now mentioned this should also cleanup a fair amount of code.
| src/lib/elementary/efl_ui_textbox.c | ||
|---|---|---|
| 911 | @zmike is it possible that this positioning code is basically available via popup internals ? | |
| 940 | Do you know if this job is really needed, i am not sure why we have it, the popup will be deleted in _backwall_clicked anyways. | |
| 1081 | Can you refactor EFL_UI_TEXT_DATA_GET(data, sd); efl_del(sd->popup_list); efl_del(sd->popup); sd->popup_list = NULL; sd->popup = NULL; Into a static inline void function ehre and call it in every _cb where they are the same, that would make the code a little bit smaller :) | |
Comment Actions
it seems EFL_UI_SELECTABLE_EVENT_SELECTION_CHANGED is not fired on EFL_UI_LIST_DEFAULT_ITEM_CLASS
| src/lib/elementary/efl_ui_textbox.c | ||
|---|---|---|
| 1049 | this cause error in log when called on EFL_UI_EVENT_SELECTED_CHANGED ERR<1606>:eina_safety ../src/lib/elementary/efl_ui_layout.c:1166 _efl_ui_layout_base_efl_layout_signal_signal_emit() safety check failed: efl_invalidated_get(obj) is true ERR<1606>:eina_safety ../src/lib/evas/canvas/evas_focus.c:226 _efl_canvas_object_seat_focus_add() safety check failed: !efl_invalidating_get(eo_obj) && !efl_invalidated_get(eo_obj) is false | |
| src/lib/elementary/efl_ui_textbox.c | ||
|---|---|---|
| 1049 | Mhm, crap, i will check it tomorrow. | |
Comment Actions
I think we can go ahead and land it the way it is, the fix for the error will be outside this here i guess.
| src/lib/elementary/efl_ui_textbox.c | ||
|---|---|---|
| 1049 | https://phab.enlightenment.org/D1913 This is causing it, i wrote all the details to T8576, lets wait for answers. | |