diff --git a/src/Eto/Forms/Container.cs b/src/Eto/Forms/Container.cs old mode 100644 new mode 100755 index 366881ce23..a64b2135f8 --- a/src/Eto/Forms/Container.cs +++ b/src/Eto/Forms/Container.cs @@ -295,9 +295,10 @@ protected void RemoveParent(Control child) child.TriggerUnLoad(EventArgs.Empty); } child.VisualParent = null; - if (ReferenceEquals(child.InternalLogicalParent, this)) - child.InternalLogicalParent = null; } + + if (ReferenceEquals(child.InternalLogicalParent, this)) + child.InternalLogicalParent = null; } /// @@ -397,7 +398,7 @@ protected void SetParent(Control child, Action assign = null, Control previousCh // 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(); + child.Detach(); //.VisualParent?.Remove(child); if (ReferenceEquals(child.InternalLogicalParent, null)) SetLogicalParent(child); diff --git a/src/Eto/Forms/Layout/TableCell.cs b/src/Eto/Forms/Layout/TableCell.cs index 2c966da979..71f313ca9a 100755 --- a/src/Eto/Forms/Layout/TableCell.cs +++ b/src/Eto/Forms/Layout/TableCell.cs @@ -7,8 +7,10 @@ namespace Eto.Forms; [sc.TypeConverter(typeof(TableCellConverter))] public class TableCell { - Control control; - TableLayout layout; + Control _control; + TableCellCollection _cells; + + TableLayout ParentTable => _cells?._layout; /// /// Gets or sets a value indicating whether this will scale its width @@ -31,23 +33,24 @@ public class TableCell /// The control. public Control Control { - get { return control; } + get { return _control; } set { - if (control != value) + if (_control != value) { value?.Detach(); - control?.Detach(); - layout?.InternalRemoveLogicalParent(control); - control = value; - layout?.InternalSetLogicalParent(control); + _control?.Detach(); + var layout = ParentTable; + layout?.InternalRemoveLogicalParent(_control); + _control = value; + layout?.InternalSetLogicalParent(_control); } } } internal void SetControl(Control control) { - this.control = control; + _control = control; } /// @@ -119,20 +122,29 @@ public static implicit operator TableCell(Image image) return new TableCell(new ImageView { Image = image }); } - internal void SetLayout(TableLayout layout) + internal void SetLayout(TableCellCollection cells, bool shouldRemove, TableLayout oldLayout = null) { - if (!ReferenceEquals(this.layout, layout)) + if (!ReferenceEquals(_cells, cells)) + { + // remove from old cells collection + if (shouldRemove) + _cells?.RemoveItemWithoutLayoutUpdate(this); + + ParentTable?.InternalRemoveLogicalParent(_control); + _cells = cells; + ParentTable?.InternalSetLogicalParent(_control); + } + else if (!ReferenceEquals(oldLayout, ParentTable)) { - this.layout?.InternalRemoveLogicalParent(control); - this.layout = layout; - layout?.InternalSetLogicalParent(control); + oldLayout?.InternalRemoveLogicalParent(_control); + ParentTable?.InternalSetLogicalParent(_control); } } } class TableCellCollection : Collection, IList { - TableLayout layout; + internal TableLayout _layout; public TableCellCollection() { @@ -142,18 +154,37 @@ public TableCellCollection(IEnumerable list) : base(list.Select(r => r ?? new TableCell { ScaleWidth = true }).ToList()) { } - - internal void SetLayout(TableLayout layout) + + internal void SetLayout(TableRow row, TableLayout layout, bool shouldRemove) { - this.layout = layout; + if (ReferenceEquals(layout, _layout)) + return; + + var oldLayout = _layout; + if (shouldRemove && _layout?.Rows is TableRowCollection oldLayoutRows) + { + // switching layouts, remove from old layout without triggering a new SetLayout + // This really shouldn't be done and should throw an exception + oldLayoutRows.RemoveItemWithoutLayoutUpdate(row); + } + _layout = layout; + foreach (var cell in this) - cell.SetLayout(layout); + cell.SetLayout(this, true, oldLayout); } + + internal void RemoveItemWithoutLayoutUpdate(TableCell cell) + { + var index = IndexOf(cell); + if (index >= 0) + base.RemoveItem(index); + } + protected override void RemoveItem(int index) { var item = this[index]; - item?.SetLayout(null); + item?.SetLayout(null, false); base.RemoveItem(index); } @@ -161,7 +192,7 @@ protected override void ClearItems() { foreach (var item in this) { - item?.SetLayout(null); + item?.SetLayout(null, false); } base.ClearItems(); } @@ -170,17 +201,17 @@ protected override void InsertItem(int index, TableCell item) { if (item == null) item = new TableCell { ScaleWidth = true }; - item.SetLayout(layout); + item.SetLayout(this, true); base.InsertItem(index, item); } protected override void SetItem(int index, TableCell item) { var old = this[index]; - old?.SetLayout(null); + old?.SetLayout(null, false); if (item == null) item = new TableCell { ScaleWidth = true }; - item.SetLayout(layout); + item.SetLayout(this, true); base.SetItem(index, item); } diff --git a/src/Eto/Forms/Layout/TableRow.cs b/src/Eto/Forms/Layout/TableRow.cs index 72c6303af0..620c025e5f 100755 --- a/src/Eto/Forms/Layout/TableRow.cs +++ b/src/Eto/Forms/Layout/TableRow.cs @@ -155,39 +155,46 @@ public static implicit operator TableCell(TableRow row) return new TableCell(new TableLayout(row)); } - internal void SetLayout(TableLayout layout) + internal void SetLayout(TableLayout layout, bool shouldRemove) { - ((TableCellCollection)Cells).SetLayout(layout); + ((TableCellCollection)Cells).SetLayout(this, layout, shouldRemove); } } class TableRowCollection : Collection, IList { - TableLayout layout; + internal TableLayout _layout; public TableRowCollection(TableLayout layout) { - this.layout = layout; + _layout = layout; } public TableRowCollection(TableLayout layout, IEnumerable list) : base(list.Select(r => r ?? new TableRow { ScaleHeight = true }).ToList()) { - this.layout = layout; + _layout = layout; foreach (var item in this) - item?.SetLayout(layout); + item?.SetLayout(layout, true); + } + + internal void RemoveItemWithoutLayoutUpdate(TableRow row) + { + var index = IndexOf(row); + if (index >= 0) + base.RemoveItem(index); } protected override void RemoveItem(int index) { var item = this[index]; - item?.SetLayout(null); + item?.SetLayout(null, false); base.RemoveItem(index); } protected override void ClearItems() { foreach (var item in this) - item?.SetLayout(null); + item?.SetLayout(null, false); base.ClearItems(); } @@ -195,17 +202,17 @@ protected override void InsertItem(int index, TableRow item) { if (item == null) item = new TableRow { ScaleHeight = true }; - item?.SetLayout(layout); + item?.SetLayout(_layout, true); base.InsertItem(index, item); } protected override void SetItem(int index, TableRow item) { var old = this[index]; - old?.SetLayout(null); + old?.SetLayout(null, false); if (item == null) item = new TableRow { ScaleHeight = true }; - item?.SetLayout(layout); + item?.SetLayout(_layout, true); base.SetItem(index, item); } diff --git a/test/Eto.Test/UnitTests/Forms/Layout/TableLayoutTests.cs b/test/Eto.Test/UnitTests/Forms/Layout/TableLayoutTests.cs index 6f49cb5574..c1f58380e6 100644 --- a/test/Eto.Test/UnitTests/Forms/Layout/TableLayoutTests.cs +++ b/test/Eto.Test/UnitTests/Forms/Layout/TableLayoutTests.cs @@ -189,6 +189,88 @@ public void LabelInAutoSizedColumnShouldHaveCorrectWidth() Assert.Greater(label.Height, 0, "Label didn't get correct height!"); }); } + + [Test] + public void DestroyingChildShouldRemoveFromParent() + { + TableLayout layout = null; + Label child = null; + Shown(form => + { + child = new Label { Text = "I should not be shown" }; + layout = new TableLayout + { + Rows = { + new TableRow(child) + } + }; + child.Dispose(); + form.Content = layout; + }, () => + { + Assert.IsTrue(child.IsDisposed); + Assert.IsNull(child.Parent); + CollectionAssert.DoesNotContain(layout.Children, child); + }); + } + + [Test, ManualTest] + public void CopyingRowsShouldReparentChildren() + { + ManualForm("Label above should show", form => + { + 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 the 2nd table"); + Assert.AreEqual(child.LogicalParent, layout1, "#1.2 Child's logical parent should now be the 2nd table"); + + // copy rows to a new layout + var layout2 = new TableLayout { ID = "layout2" }; + foreach (var row in layout1.Rows.ToList()) + layout2.Rows.Add(row); + + Assert.AreEqual(0, layout1.Rows.Count, "#2.1 All rows should now be in the 2nd table"); + Assert.AreEqual(child.Parent, layout2, "#2.2 Child's parent should now be the 2nd table"); + Assert.AreEqual(child.LogicalParent, layout2, "#2.3 Child's logical parent should now be the 2nd table"); + return layout2; + }); + } + + [Test, ManualTest] + public void CopyingCellsShouldReparentChildren() + { + ManualForm("Label above should show", form => + { + var child = new Label { Text = "I should be shown!" }; + + var layout1 = new TableLayout { ID = "layout1" }; + var cell1 = new TableCell(child); + var row1 = new TableRow(child); + layout1.Rows.Add(row1); + Assert.AreEqual(child.Parent, layout1, "#1.1 Child's parent should now be the 2nd table"); + Assert.AreEqual(child.LogicalParent, layout1, "#1.2 Child's logical parent should now be the 2nd table"); + + // copy rows to a new layout + var layout2 = new TableLayout { ID = "layout2" }; + var row2 = new TableRow(); + foreach (var row in layout1.Rows.ToList()) + { + foreach (var cell in row.Cells.ToList()) + row2.Cells.Add(cell); + } + layout2.Rows.Add(row2); + + Assert.AreEqual(1, layout1.Rows.Count, "#2.1 Should still have a single row"); + Assert.AreEqual(0, layout1.Rows[0].Cells.Count, "#2.2 Cells should be removed from old table"); + Assert.AreEqual(1, layout2.Rows.Count, "#2.3 2nd table should have a single row"); + Assert.AreEqual(1, layout2.Rows[0].Cells.Count, "#2.4 All cells should now be in the 2nd table"); + Assert.AreEqual(child.Parent, layout2, "#2.5 Child's parent should now be the 2nd table"); + Assert.AreEqual(child.LogicalParent, layout2, "#2.6 Child's logical parent should now be the 2nd table"); + return layout2; + }); + } } }