class Efl.Ui.Check @beta ├ (P) selected ├ (E) selected,changed
- Remove NState from the inheritance tree
- Rename property to check_state and event to check_state,changed
- Make it include the Efl.Ui.Clickable mixin, and use the signals in theme.
class Efl.Ui.Check @beta ├ (P) selected ├ (E) selected,changed
Status | Assigned | Task | ||
---|---|---|---|---|
Resolved | None | T7867 efl.ui.radio | ||
Resolved | None | T7865 efl.ui.check | ||
Open | None | T7586 efl.access.widget.action |
IMO this should not inherit from nstate. Check is not made for displaying n-states, everything else then 2 states is going to break the displaying (same for efl.ui.check).
The whole point of Efl.Ui.Nstate is that widgets like Efl.Ui.Check inherit from it. It provides them count (number of states) and value (current state) properties.
I agree this is a bit over-engineered, but I do not see anything wrong with it.
Efl.Ui.Check is not meant to display more than 2 states. It's a particularization of Efl.Ui.Nstate that only supports 2 states (just like Efl.Ui.Button).
These two widgets should override the count.set property and show an error if anybody tries to use it.
Then what is the point of having this property, it is an abstraction that does not work. It's useless API. Additionally, due to the fact of inheriting from button, we also get things like autorepeat implemented on check, which is something we definitely do not want ... I would really just make it inherit from layout, and continue from there
Sure, I have no objection.
Do we have any real N-state widget in EFL, though? Something like a checkbox that accepts a grayed-out third state, or something like that? Looks like most of the work on this class was done by @jpeg so we cannot ask him...
I'm not aware of any kind of tristate type widget, I guess this was some kind of future-proofing? Would be good to get @woohyun or @Jaehyun_Cho to reply here.
I don't think nstate would be used in the near future. That was the reason why I hoped to cut the relation between nstate and check :)
I think this needs some naming changes:
Yeah, definitely. selected is about choosing one option among many, and we only care about on or off here.
I could go with active but it looks too similar with enabled (meaning that the widget can be interacted with)... how about check, cross, mark, stroke or check_state?
In many other places, we have used the pattern property,changed for event names whenever they are emitted upon property being changed. We should be consistent.
PROPERTY | EVENT |
check_state | check_state,changed |
checked | checked,changed |
is_checked | checked,changed |
I also agree that "selected" is weird, but I'm sorry to not give good new name.
Let's bring here the discussion in T8025 regarding whether or not Efl.Ui.Check should implement Efl.Ui.Clickable.
I think using Efl.Ui.Clickable to reuse code is OK (although I do not see the point of Checkboxes and radio buttons emitting longpressed events).
I think inheriting from Efl.Ui.Button will be too confusing... they are conceptually different widgets. I know Gtk does that, and it has always confused me :)
If we use the *not yet existing* selectable interface on this, then check_state is not there anymore, but rather the name from the selectable interface. (T8057)
@zmike @Jaehyun_Cho do we now want to have selected removed here and move to the Efl.Ui.Selectable interface ? (D9503)
Which makes it possible to implement single_select in radio group.
So - i applied the change, the revision is linked above. If the revision is landed, things do looks fine ?