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

Only remove the child from parent if it is different #2657

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions src/Eto/Forms/Container.cs
Original file line number Diff line number Diff line change
Expand Up @@ -363,23 +363,23 @@ protected void SetParent(Control child, Action assign = null, Control previousCh
SuspendLayout();
if (Handler is IThemedControlHandler)
{
if (!ReferenceEquals(previousChild, null))
if (previousChild is not null)
RemoveLogicalParent(previousChild);

if (!ReferenceEquals(child, null) && ReferenceEquals(child.LogicalParent, null))
if (child is not null && child.LogicalParent is null)
SetLogicalParent(child);
assign();
ResumeLayout();
return;
}
if (!ReferenceEquals(previousChild, null) && !ReferenceEquals(previousChild, child))
if (previousChild is not null && !ReferenceEquals(previousChild, child))
{
#if DEBUG
if (!ReferenceEquals(previousChild.VisualParent, this))
throw new ArgumentException("The previous child control is not a child of this container. Ensure you only remove children that you own.");
#endif

if (!ReferenceEquals(previousChild.VisualParent, null))
if (previousChild.VisualParent is not null)
{
if (previousChild.Loaded)
{
Expand All @@ -393,15 +393,21 @@ protected void SetParent(Control child, Action assign = null, Control previousCh
previousChild.InternalLogicalParent = null;
}
}
if (!ReferenceEquals(child, null) && !ReferenceEquals(child.VisualParent, this))
if (child is not null && !ReferenceEquals(child.VisualParent, this))
{
// Detach so parent can remove from controls collection if necessary.
// prevents UnLoad from being called more than once when containers think a control is still a child
// no-op if there is no parent (handled in detach)
child.Detach(); //.VisualParent?.Remove(child);
child.VisualParent?.Remove(child);

if (ReferenceEquals(child.InternalLogicalParent, null))
// Remove from previous parent only if it differs
if (child.InternalLogicalParent is not null && !ReferenceEquals(child.InternalLogicalParent, this))
child.InternalLogicalParent?.Remove(child);

// Set new logical parent
if (child.InternalLogicalParent is null)
SetLogicalParent(child);

child.VisualParent = this;
if (Loaded && !child.Loaded)
{
Expand Down
26 changes: 26 additions & 0 deletions test/Eto.Test/UnitTests/Forms/Layout/TableLayoutTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,32 @@ public void CopyingCellsShouldReparentChildren()
return layout2;
});
}

[Test]
public void AddingChildToAnotherPanelShouldRemoveFromTableLayout()
{
Invoke(() =>
{
var child = new Label { Text = "I should be shown!" };

var layout1 = new TableLayout { ID = "layout1" };
layout1.Rows.Add(child);
Assert.AreEqual(child.Parent, layout1, "#1.1 Child's parent should now be layout1");
Assert.AreEqual(child.LogicalParent, layout1, "#1.2 Child's logical parent should now be layout1");

// use the child somewheres else
var panel = new Panel { ID = "panel" };
panel.Content = child;

Assert.AreEqual(1, layout1.Rows.Count, "#2.1 layout1 should still have a row");
CollectionAssert.DoesNotContain(layout1.Children, child, "#2.2 Child should no longer be in layout1");

Assert.AreEqual(child.Parent, panel, "#2.3 Child's parent should now be panel");
Assert.AreEqual(child.LogicalParent, panel, "#2.4 Child's logical parent should now be panel");
CollectionAssert.Contains(panel.Children, child, "#2.5 Child should be in panel");
});
}

}
}

Loading