Page MenuHomePhabricator

efl_ui_selectable: separate efl_ui_selectable into item and text
AcceptedPublic

Authored by Jaehyun_Cho on Mar 21 2019, 7:14 PM.

Details

Summary

Previously, Efl.Ui.Selectable interface contained item selection events
and text selection events.

Now, Efl.Ui.Selectable interface is separated into
Efl.Ui.Item_Selectable and Efl.Ui.Text_Selectable.

T7766

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10536
Build 8264: arc lint + arc unit
Jaehyun_Cho created this revision.Mar 21 2019, 7:14 PM
Jaehyun_Cho requested review of this revision.Mar 21 2019, 7:14 PM
Jaehyun_Cho edited the summary of this revision. (Show Details)Mar 21 2019, 7:14 PM

Please give some comments on this patch.

Especially I need feedback for naming. (e.g. Efl.Ui.Selectable, Efl.Ui.Text_Selectable, Efl.Ui.Cursor_Selectable, Efl.Ui.Text_Cursor_Selectable)

bu5hm4n accepted this revision.Mar 22 2019, 8:40 AM

I 100% agree with this change. However, can we wait with this change until we are open for the next release ?

This revision is now accepted and ready to land.Mar 22 2019, 8:40 AM

I also agree this change is necessary.
I Like Item_Selectable (or maybe Selectable_Item?) but I am not so sure about Text_Cursor_Selectable. Isn't Text_Selectable (or Selectable_Text) enough?

@bu5hm4n
Thank you for the feedback :)
I wanted to apply this change before release but as you say let's wait until release because these interfaces are @beta ;)

@segfaultxavi
Thank you for the feedback :)
Yes, you got my point! I totally agree with you. I totally agree with you I will change it to Efl.Ui.Text_Selectable (Efl.Ui.Selectable_Text).

About the position of Selectable, I wanted to follow other "able" class and interface.
e.g. Efl.Ui.Text_Editable, Efl.Ui.Image_Zoomable, Elm.Interface_Scrollable

But I also feel uncomfortable about the position of Selectable. How about changing the name of all "able" class and interface?
e.g. Efl.Ui.Text_Editable -> Efl.Ui.Editable_Text, Efl.Ui.Image_Zoomable -> Efl.Ui.Zoomable_Image

But we can keep Efl.Ui.Scrollable -> Efl.Ui.Scrollable (Because we can omit "Object". i.e. Scrollable_Object -> Scrollable)

BTW, I think Efl.Ui.Item_Selectable should not be Selectable_Item because this interface is applied to a container object not to an item object.
It seems that Selectable_Item should belong to item object.

Jaehyun_Cho edited projects, added Restricted Project; removed efl.Mar 24 2019, 7:33 PM

change Efl.Ui.Text_Cursor_Selectable to Efl.Ui.Text_Selectable

Let's use Text_Selectable for now and maybe in the future we change all classes to Selectable_* instead.

Jaehyun_Cho retitled this revision from efl_ui_selectable: separate efl_ui_selectable into item and text_cursor to efl_ui_selectable: separate efl_ui_selectable into item and text.Mar 25 2019, 4:51 AM
Jaehyun_Cho edited the summary of this revision. (Show Details)

@segfaultxavi

OK, I will make a patch to change the name of all "able" classes/interfaces/mixins later :)
BTW, since @bu5hm4n requested to wait for 1.22 release, I think I should not push this patch until 1.22 release.

Yes, maybe you should add the DO NOT MERGE project tag so nobody merges it by accident.

Sorry i forgot about this, i ironically redid the same, and just found your ticket, and this revision. I could kick my own ass for that ... :|