Page MenuHomePhabricator

elm_widget: add signal for parent changes
AbandonedPublic

Authored by bu5hm4n on May 30 2016, 1:40 AM.

Details

Reviewers
tasn
raster
Summary

this signal is emitted when a new subobject is added by calling
elm_widget_sub_object_add.

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D3992
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2223
Build 2288: arc lint + arc unit
bu5hm4n updated this revision to Diff 9189.May 30 2016, 1:40 AM
bu5hm4n retitled this revision from to elm_widget: add signal for parent changes.
bu5hm4n updated this object.
bu5hm4n edited the test plan for this revision. (Show Details)
bu5hm4n added a reviewer: raster.

If you ask "WHY?", take a look at T3670 :)

cedric added a reviewer: tasn.May 31 2016, 4:43 PM

Just to bring your opinion on this. Maybe you have a better idea on how to do this.

raster edited edge metadata.May 31 2016, 6:48 PM

seem my comments in T3670

so this is so a child can know when its parent changed so it can change its registration info in the focus manager? the idea is that this event is triggered on the child whenever it changes its parent?

just fyi it looks like it doesn't handle getting a NULL parent being removed from the tree and then floating about). :)

raster requested changes to this revision.Jun 19 2016, 11:37 PM
raster edited edge metadata.

as above - missed sub object del's :) _elm_widget_sub_object_del() :)

This revision now requires changes to proceed.Jun 19 2016, 11:37 PM

Actually, this was done on purpose.
If we add it on del we would have two events on add one for the del one for the add.

what i meant is you have code here to call the callback on sub object add, but what if an object is deleted and NOT added to a new parent? the obj can't know and can't fix the focus manager...

i.e. there is a sub object del and no add. normally there is a del then an add. sometimes just an add and no del.

Yes thats a problem. But this cannot be fixed. since sub_object_add calls sub_object_del, so emit the event on del will result in 2 emitted events. Which is a problem.

If the api would be a parent set on the elm.widget class we could have it very simple, and sub_object_add should only be used on none elm.widget objects.

it's still a case that needs handling though. maybe having 2 events is not bad. one will have obj remove itself from the focus manager - the other add puts itself back in. it's simpler imho.

I dont think its a good idea, this will mean that each time a object is reparent´ed we are having 2 events, setting it to NULL and setting it to the new parent, I dont like this idea at all... Its adding more and more events for no reason.

Other idea: The parent change event only makes sense on elm.widgets.
So in sub_object_add() / sub_object_del() i am testing of the child to add is a elm.widget, if it is, i am just setting the parent on it, if not old code path happens. So in case of a elm.widget we are not having a unneccessary del call. What do you think?

well as for the 2 events... regardless if its "more events than you want"... it's necessary to handle addition as well as removal. if you skip one you just have incorrect code. it cannot handle an actual real possible situation - where a widget loses a parent but does not gain another. they are separate things in efl. it's necessary to handle all cases.

:) now in elm in almost all cases you will always get an ad after a del, but right now, if you do content_set() on a parent and there was content there before, the old content is orphaned and floats about. it's not deleted. from memory anyway... :)

In D3992#68166, @raster wrote:

well as for the 2 events... regardless if its "more events than you want"... it's necessary to handle addition as well as removal. if you skip one you just have incorrect code. it cannot handle an actual real possible situation - where a widget loses a parent but does not gain another. they are separate things in efl. it's necessary to handle all cases.

yes we need to handle both cases, but with your solution ONE SINGLE sub_object_add call results in a parent,change (with parent beeing NULL, this case cannot happen elm so this event has not any sense) and after that the next parent,change event (this time with the new parent). So this is not about incorrect code, its about not emitting useless events which are indicating a incorrect state of the tree.

:) now in elm in almost all cases you will always get an ad after a del, but right now, if you do content_set() on a parent and there was content there before, the old content is orphaned and floats about. it's not deleted. from memory anyway... :)

I have no idea what you are talking about...

bu5hm4n updated this revision to Diff 9474.Jul 1 2016, 2:38 AM
bu5hm4n edited edge metadata.

just emit the events even if they are useless.

bu5hm4n updated this revision to Diff 9476.Jul 1 2016, 3:55 AM
bu5hm4n edited edge metadata.

update api b0rkes

raster requested changes to this revision.Jul 10 2016, 11:42 PM
raster edited edge metadata.

actually this is worse. ok you get a children changed on the old parent AND an children changed on the new parent... you still get as many events as if you had an add and del. the ADVANTAGE of an add and del is that you can PASS the old or new child in the event info. if the child was in the add or del info it'd make it very efficient to whoever is listening for these to find the child by its handle explicitly and do something sensible.

This revision now requires changes to proceed.Jul 10 2016, 11:42 PM

How can something be worse than before when i just added something ?!

I have absolutly no idea what you are talking about. Your add event and del event are basically the parent,changed event, and there you can also track the child which was changed since the event is propergated throuw the tree... SO your way of stuffing things into the eventinfo is in here, since the first day of the patch ...

you have change info on the CHILD but not the PARENT. see my inline comments.

src/lib/elementary/elm_widget.c
1224

no info here. what child changed? who was added or removed? i have no idea what to do here as someone getting the event. how about simply providing the child that was added or removed (in this case added) in the event info. this means when i get this event i have USEFUL information - like what widget was removed from the children.

1356

same here like above - but this is a remove from the parent. the even gives me no info on what child object was removed. without this info anyone listening has to keep copies of all children before, then look atr current list and find who is missing.

there is NO cost to providing the child here as event info, so why not do it?

and better split the above into a child add event and this one here into a child del so the event type is information on if the child is being added or removed.

So in the end its not worse than before, you just miss a few features ;)

src/lib/elementary/elm_widget.c
1224

I will add the feature that you know which child got added or deleted. But i am not so sure how usefull it is, You can know which child got removed but most of the time the position of the subchildren is important, so you cannot get arround recomputing the complete subchildren.

1356

I would like to keep one event and add a event struct with a field which indicates if its a add or del. (Why ? To keep the api look the same)

bu5hm4n abandoned this revision.Feb 15 2018, 5:55 AM