Page MenuHomePhabricator

RFC: Efl.Ui.Selectable
Closed, ResolvedPublic

Description

Currently, Efl.Ui.Selectable only has events and it supports both 2 features, container item selection and cursor selection.

I think we need to separate this one interface into 2 interfaces for container item selection and cursor selection.

Moreover, container item selection can be separated into 2 features, selectable for item and selectable for container.

For container item selection, I think it should be as follows.

  1. Selectable for item
interface Efl.Ui.Selectable
├ (P) selected: bool
├ (M) toggle
  1. Selectable for container
interface Efl.Ui.Single_Selectable
├ (M) select
├ (M) unselect
├ (M) selected_get
interface Efl.Ui.Multiple_Selectable : Efl.Ui.Single_Selectable
├ (M) select_range
├ (M) unselect_range
├ (M) select_all
├ (M) unselect_all
├ (M) selected_range_get

Events are also required to be considered.

I am also in favor of this rfc, there is just one thing that i do not like:
toggle is a API which is not in the style of the rest of EFL, we usally only have the property, flipping this bit should be fairly easy. Additionally, (from a user pov) in most cases you do not want to flip the bit, but rather set a explicit value.

As a sidenote: Efl.Ui.Mutli_Selectable already exists and makes sense. I think we can just extend the existing interface with the given APIs here.

zmike added a subscriber: zmike.Jul 19 2019, 6:10 AM

I am also in favor of this rfc, there is just one thing that i do not like:
toggle is a API which is not in the style of the rest of EFL, we usally only have the property, flipping this bit should be fairly easy. Additionally, (from a user pov) in most cases you do not want to flip the bit, but rather set a explicit value.

I agree here; in EFL we favor API design where only one outcome is possible (e.g., "set the value to X", "change the state to Y") in order to avoid cases where a function may be called repeatedly with different results each time.

I imagine the property in this case would be called selected; won't this have some naming conflicts with properties from other classes? We may need to do a pass on that...

bu5hm4n added a comment.EditedJul 31 2019, 3:53 AM

@Jaehyun_Cho Are you currently working on that ? I am currently working on items, and this seems like something i need now, so I would just go with this interface and define it myself ?

@bu5hm4n I haven't started yet. It's ok if you go with this :) Are you thinking of implementing all (Selectable for item + Selectable for container) ?

I think we also want to have here some indicator that tells if we are running in CTRL+select mode, or "touch-style" select mode, for selecting and unselecting more items?

My revisions are bringing in the first draft of those interfaces. However, we should think about the following:

  1. How do we want to organize ranges, start and end with objects or ids ? (ids like the id they have in the parent container)
  2. How do we want to organize things like multiselect behaviour (touch vs. ctrl-select) in elm_config or different per widget?
  3. select_all / unselect_all / select_range / unselect_range should this be on the interface or in some helper class ? It could be implemented based on the rest of the API, would lower the barrier of entry for people implementing this interface
  4. Events: in multi just a "range,changed" event which brings the first and last selectable / id (depends on #1), in single its just "last_selected,changed" not that bad IMO.
cedric added a subscriber: cedric.Aug 7 2019, 9:20 AM

I would like to point out an issue with the current API. The more content we get, the more problem it generate ? The caller of the API, could manually throttle the call to the iterator, but there is no guarantee that the object would not change (I can see implementing a custom iterator that create item and destroy them as it walk over the collection view, that part isn't problematic).
I kind of have two idea here.

My preferred one being to enforce that as long as the iterator is live, the object will not allow any change to the selection. It's easy to do, and allow for throttling call to the iterator safely. It's a bit of a hack, but simple to do. We could provide some helper/example on how to call the API during idle or something like that.

The other possibility is to completely move away from the iterator and implement it with a callback that will pass a group of selected item and a future that is triggered once all the selected content are returned. It might be a slightly more efficient solution, but quite more complex for the user in the simpler case.

I personally vote for solution one. If anyone else has any other idea?

@cedric i am not sure if i like bringing in another type or way of doing that. Can we solve that in the iterator ? That we invalidate it by the time the object is invalidated (that is quite easy, just set all callbacks to NULL).

As an addition to my previous comment:

  1. Do we want a way to have always at least 1 item selected?
  1. you mean the index id? I think for the range, integer id would be much easier to understand and use for the user, but mostly user have object handle not the integer id... but they can get the index of item easily. if we do not make our index_get poorly like O(n)... I think using integer id is more intuitive.
  2. I think we need to give common experience on that usage, like mobile multi select by click, desktop multi select by ctrl or drag, so it is profile dependencies then widgets.. I vote config.
  3. the case of how implement it will be different per widgets I think, like collection widget, they have all the items list, they could manually update the selection on items, but in the view, data should be updated on the model... I don't think we need helper class for this selection. but let's see how many widget want to selection and how many widget have common implements.
  4. seems good to me.
  5. I think that is select_always. mostly if I clicked selected item, it should be unselected, but in select_always mode, it remained selected state. but in multiselect, selected item should be unselected, so I think select_always is only for single selection..

1: Okay, lets go with ID for now, switching to objects should be easy i think
2: Ok - config it will be
3: Ok
4: ... Ok :)
5: According to the implementations and the documentation of efl_ui_multiselectalbe.eo.h, it seems that the always is meant to define the way when the event is emitted, always or only on state changes. This has no relation to have always at least one item selected.

I have the same opinions with @SanghyeonLee for 1, 2, 3.

About 4, I have a question.
Does "range,changed" event contain all the selectable IDs? Or first and last selectable IDs which are 2 IDs?
Does range should have continuous IDs only? How about not continuous IDs? (e.g. 1, 3, 5, ...)

About 5, I think "having always at least 1 item selected" is related to implementation. So I am not sure we need a method or property for that.

bu5hm4n added a comment.EditedAug 20 2019, 7:07 AM

I just implemented it with IDs and it seems just super complex and unnecessary (the caller first has to get the id from the object, just for the container to get the object again from the id), it is now implemented with objects.

Additional to this i would tend to implement two more APIs and a single event:

  • selection_reaction_mode: Can either be touch or desktop (it defines the behavior how to add multiple items) but i am not sure on where this property will be, on the config, on the class, or on the object.
  • fallback_object : You can set here a selectable object, when all items are unselected, this object is selected. I think this comes handy in the widget itself, implementing this outside the widget is quite hard, inside the widget is easy ... :)
  • "selection,changed" event: This is emitted every time a change to the state of selection in the container happens, this can be used in single or multiselect to get the new set of selected items, (via the property in single_selection or the iterator in multi_selection) (Lets forget about this for now)

This is then enough i think, and every usecase that i can think of from the here discussed features is possible.

zmike moved this task from Backlog to Evaluating on the efl: api board.Sep 2 2019, 5:50 AM

How is this progressing?

basically done ... @SanghyeonLee @Jaehyun_Cho happy with what we have ?

bu5hm4n closed this task as Resolved.Sep 6 2019, 2:26 AM
bu5hm4n claimed this task.

Lets reopen this when something else comes up.