Add error checking on parentNode

[mpickell]

Added error checking to block removing from parentNode if element was already removed

I was hitting errors where another library had already removed the element from parentNode.

Associated fix in metafizzy/outlayer#52

[desandro]

Thanks for this contribution! I see how this could help in your situation.

That said, I feel this is an edge case. If the element has already been removed, then this code should be run in the first place. You could resolve this by checking if the element is already removed before triggering item.remove().

I appreciate your effort here, but I'm not keen on merging it in to master. I haven't had others request this kind of feature, so I don't feel its a necessary at this point.

I hope you continue to get involved in open source. Sorry to say no to this one!

[mpickell]

Hey @desandro thanks for the feedback... unfortunately i'm not calling item.remove() myself.

• I'm using the packery-angular library for packery in an angular app.
• I'm also using ng-repeat to put the packery items into the page..

If the user performs a search or other UI interaction that affects the list, angular ng-repeat is indirectly triggering code in jQuery that is removing the elements from the parentNode to redraw the new list. After that, packery-angular is also triggering its own remove which goes on to eventually trigger item.remove() in packery. jQuery handles this but unfortunately it is the library called first.

I was going to place a PR against packery-angular directly, but after stepping down into the code felt that it made more sense handling it lower-down in the packery level to block it from happening anywhere else. The same change was already made in the hideDropPlaceholder function at line 123. It feels hacky to me to handle it at higher level libraries?

I hope you'll reconsider but i'll look into other solutions.

[desandro]

Good point about hideDropPlaceholder. It's a safe addition. I'll merge it on the Outlayer repo metafizzy/outlayer#52

[mpickell]

@desandro Thanks!

Will this PR get merged as well then? The code is in two different functions. The one here in item.js as well as over in Outlayer. The comments in the functions are different so I'm assuming the function from Outlayer will not fix the issue here as well?

[desandro]

Good call. I forgot Packery's Item overwrites Outlayer's. WOW its been a day. This PR will land in the next release of Packery. Thank you so much for your contributions!