Page MenuHomePhabricator

efl.ui.radio
Closed, ResolvedPublic

Description

class Efl.Ui.Radio @beta
├ (P) state_value

Related Objects

StatusAssignedTask
ResolvedNone
ResolvedNone
OpenNone
bu5hm4n created this task.May 3 2019, 11:24 AM
bu5hm4n triaged this task as TODO priority.
bu5hm4n added a subtask: T7865: efl.ui.check.

I think there should be another widget called Efl.Ui.Radio_Group, where you can add radio buttons. The whole semantic with with the group itself looks confusing to me.

This here is a try to come up with a design for a group class. I go here with the assertion that any set of radio buttons wants to be added in a group.

A new class Efl.Ui.Radio_Group. This class inherits from efl.ui.box. You can only add new Efl.Ui.Radio class instances to it. This means the placing and logical connection of radio buttons are now the same. Which is quite sane IMO since radio button groups do come with the same ui placement.

This will result in efl.ui.radio.eo only having one property state_value.
And another class Efl.Ui.Radio_Group that has value, and selected object.

Does that sound acceptable ?
@zmike @segfaultxavi @cedric opinions ?

I like how this simplifies creating radio groups, since you now provide the Efl.Ui.Box automatically.

However, I fear we lose the flexibility to have radio buttons anywhere in the UI (we only provide the API, we should not judge if that makes sense or not).

How about we keep the current grouping mechanism as the "low level API" (fixing what's broken) and add your proposed Efl.Ui.Radio_Group as the "high level API". The Radio_Group class would take care of keeping each Radio's group pointers pointing to the right place, etc.

We could do it like this:

  • Efl.Ui.Radio.Logical_Group
  • Efl.Ui.Radio.Box

[...]

The logical group is not a graphical element but rather just the logical stuff, (ensuring one radio button is only enabled etc.)
The box then is just inheriting from Efl.Ui.Box, and additionally has an internal Logical_Group where all the content is added to.

Does that sound nice ?
I don't think we can fix the logical group in a sane manner in the radio group itself.

I like it. Simple case: you instantiate a radio box and add radio buttons. Complex case, you instantiate a radio logical_group and register radio buttons to it, remembering to also add the radio buttons to the ui wherever you want.

Current Elm implementation already has a Logical_Group equivalent in the Group structure. This approach only makes it explicit and therefore easier to understand.

zmike added a comment.May 28 2019, 8:48 AM
  • Efl.Ui.Radio.Logical_Group

I like Efl.Ui.Radio.Group instead of Logical_Group. We should be careful to avoid introducing new terms (e.g., Logical) which we don't plan to reuse.

zmike moved this task from Backlog to Evaluating on the efl: api board.Jun 12 2019, 7:36 AM
zmike added a comment.Jun 12 2019, 9:19 AM

@bu5hm4n can you update this with what it's (probably) going to look like after all your changes land?

bu5hm4n updated the task description. (Show Details)Jun 20 2019, 8:40 AM

Any comments here? I am quite happy with this.

Looks good to me.

bu5hm4n moved this task from Evaluating to Stabilized on the efl: api board.Jul 12 2019, 6:26 AM

Related to radio, we need to support setting radio into other containers. (e.g. Efl.Ui.List)

Now we have Efl.Ui.Radio_Group interface and Efl.Ui.Radio_Group_Impl class.

Although we have class Efl.Ui.Radio_Group_Impl and we can use it to support the above scenario but the name (Efl.Ui.Radio_Group_Impl) does not look good enough to be publicly opened to be used.

Is it OK if we discuss the new name of it?

Discussing a new name is surely okay, however, you are here in the ticket of radio, not in the ticket of the radio group implementation, nor in the ticket for Efl.Ui.Radio_Group :) (T8024,T8025)

Related to radio, we need to support setting radio into other containers. (e.g. Efl.Ui.List)

This is exactly the reason why @bu5hm4n separated Efl.Ui.Radio_Group into an interface and an implementation class. This separation allows attaching the Efl.Ui.Radio_Group_Impl implementation to any widget, turning the widget into a radio group manager.
This is currently in use in the Efl.Ui.Radio_Box widget, making it very easy to implement and to use.
I think you should do the same thing to include radio buttons inside a list: Create an Efl.Ui.Radio_List widget, that behaves like Efl.Ui.Radio_Box.

Regarding the name Efl.Ui.Radio_Group_Impl, it can be discussed in T8025. Remember that initially the idea was that these implementations were created by a class method in Efl.Ui.Radio_Group (something like create_group_manager), so the _Impl class name was never seen by the user. I think I still like that method better :)