Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method removeChild():DisplayObject never returns null #777

Closed
b005t3r opened this issue Oct 23, 2015 · 9 comments
Closed

Method removeChild():DisplayObject never returns null #777

b005t3r opened this issue Oct 23, 2015 · 9 comments
Labels
Milestone

Comments

@b005t3r
Copy link

b005t3r commented Oct 23, 2015

I'd expect removeChild() to return null if the given child was not removed (because it's not a child of the given parent), so I could, for example, use it to check if the parent has changed as a result of this call.

Currently, it always returns the object passed as child, which is not really that useful.

@b005t3r b005t3r changed the title removeChild():DisplayObject never returns null Method removeChild():DisplayObject never returns null Oct 23, 2015
@neolit123
Copy link

hey, b005t3r.

i'm sure you know that what Starling has is compatible with the Flash API and changing it could break for some users. but i do agree, what you suggest makes sense, because the caller already has a reference to the DisplayObject passed to removeChild()...and i don't think there are many users that grab the result from removeChild() as it's a known instance. in the Flash API the removeChild() return is such so that it's similar to the removeChildAt().

possibly a viable change.

contains() also exists, which BTW i think can be simplified in the current source from:

/** Determines if a certain object is a child of the container (recursively). */
        public function contains(child:DisplayObject):Boolean
        {
            while (child)
            {
                if (child == this) return true;
                else child = child.parent;
            }
            return false;
        }

to:

public function contains(child:DisplayObject):Boolean
{
    return child && child.parent == this;
}

@PrimaryFeather PrimaryFeather added this to the Starling 2.0 milestone Oct 27, 2015
@PrimaryFeather
Copy link
Contributor

Makes sense! I agree that this return value doesn't have much worth currently; I just did it that way because the Flash API does it like that. So it makes sense to change it to something more useful. I'll think about it!

@neolit123: the contains method not only checks if the object is a child of the container, but also if it's a grand-child, etc. That way, you can quickly check if an object is "underneath" another in the display tree.
Thus the loop. 😉

@PrimaryFeather
Copy link
Contributor

BTW, any comments on the removeChild change are welcome! What do others think — good idea, bad idea? Thanks in advance!

@b005t3r
Copy link
Author

b005t3r commented Oct 27, 2015

I have no idea how old Flash API works, obviously, but what I suggested is based on how a very robust JAVA Collection framework works - all methods like add(), remove() there return a value which clearly indicates if a given collection has changed in the process, which is very useful for e.g. refreshing views for which the collection was a model.

So that's my vote for changing the way it works currently to returning null if the DisplayObjectContainer did not change :)

@neolit123
Copy link

the contains method not only checks if the object is a child of the container, but also if it's a grand-child, etc. That way, you can quickly check if an object is "underneath" another in the display tree.
Thus the loop.

i see, makes perfect sense.

@PrimaryFeather
Copy link
Contributor

Thanks, guys! I'm leaning towards making that change in the 2.0 branch. =)

@PrimaryFeather
Copy link
Contributor

I must admit I totally forgot about that ...

@joshtynjala What do you think about this? Would it help / be a problem for Feathers if I changed this?

@joshtynjala
Copy link
Contributor

I don't think I use the return value from removeChild() anywhere in Feathers. I don't think it would break anything.

@PrimaryFeather
Copy link
Contributor

Closed with 1f08c5a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants