Skip to content
This repository was archived by the owner on Sep 25, 2024. It is now read-only.

Commit 0497704

Browse files
author
Eric Maupin
committed
[Core] Fix grouped type properties not swapping properly
Fixes #525 When we moved to the new panel group system, the grouped editors weren't accounted for and so when we try to remove their child property from the container group it's not found since they're inside the group editor VM. This also meant that even if removal had worked, we wouldn't properly add new shared view model types to the group.
1 parent d32b505 commit 0497704

File tree

3 files changed

+208
-9
lines changed

3 files changed

+208
-9
lines changed

‎Xamarin.PropertyEditing.Tests/PanelViewModelTests.cs‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,148 @@ public void GroupedTypesDoNotLeavePhantomCategory ()
442442
Assert.That (vm.ArrangedEditors.Count, Is.EqualTo (1));
443443
}
444444

445+
[Test]
446+
[Description ("https://github.com/xamarin/Xamarin.PropertyEditing/issues/525")]
447+
public void SwitchingFromObjectWithGroupedType ()
448+
{
449+
var targetWithProperties = new object ();
450+
var targetWithoutProperties = new object ();
451+
452+
var property = new Mock<IPropertyInfo> ();
453+
property.SetupGet (p => p.Type).Returns (typeof (string));
454+
property.SetupGet (p => p.Category).Returns ((string)null);
455+
property.SetupGet (p => p.Name).Returns ("name");
456+
457+
var property2 = new Mock<IPropertyInfo> ();
458+
property2.SetupGet (p => p.Type).Returns (typeof (string));
459+
property2.SetupGet (p => p.Category).Returns ((string)null);
460+
property2.SetupGet (p => p.Name).Returns ("name2");
461+
462+
var provider = new Mock<IEditorProvider> ();
463+
provider.Setup (ep => ep.GetObjectEditorAsync (targetWithProperties))
464+
.ReturnsAsync (new MockObjectEditor (property.Object, property2.Object) { Target = targetWithProperties });
465+
provider.Setup (ep => ep.GetObjectEditorAsync (targetWithoutProperties))
466+
.ReturnsAsync (new MockObjectEditor (new IPropertyInfo[0]) { Target = targetWithoutProperties });
467+
468+
var platform = new TargetPlatform (provider.Object) {
469+
GroupedTypes = new Dictionary<Type, string> {
470+
{ typeof(string), "strings" }
471+
}
472+
};
473+
474+
var vm = CreateVm (platform);
475+
vm.ArrangeMode = PropertyArrangeMode.Category;
476+
vm.AutoExpand = true;
477+
vm.SelectedObjects.Add (targetWithProperties);
478+
479+
Assume.That (vm.ArrangedEditors.Count, Is.EqualTo (1));
480+
481+
Assert.That (() => vm.SelectedObjects.ReplaceOrAdd (targetWithProperties, targetWithoutProperties),
482+
Throws.Nothing);
483+
Assert.That (vm.ArrangedEditors.Count, Is.EqualTo (0));
484+
}
485+
486+
[Test]
487+
public void SwitchingToObjectWithGroupedType ()
488+
{
489+
var targetWithProperties = new object ();
490+
var targetWithoutProperties = new object ();
491+
492+
var property = new Mock<IPropertyInfo> ();
493+
property.SetupGet (p => p.Type).Returns (typeof (string));
494+
property.SetupGet (p => p.Category).Returns ((string)null);
495+
property.SetupGet (p => p.Name).Returns ("name");
496+
497+
var property2 = new Mock<IPropertyInfo> ();
498+
property2.SetupGet (p => p.Type).Returns (typeof (string));
499+
property2.SetupGet (p => p.Category).Returns ((string)null);
500+
property2.SetupGet (p => p.Name).Returns ("name2");
501+
502+
var provider = new Mock<IEditorProvider> ();
503+
provider.Setup (ep => ep.GetObjectEditorAsync (targetWithProperties))
504+
.ReturnsAsync (new MockObjectEditor (property.Object, property2.Object) { Target = targetWithProperties });
505+
provider.Setup (ep => ep.GetObjectEditorAsync (targetWithoutProperties))
506+
.ReturnsAsync (new MockObjectEditor (new IPropertyInfo[0]) { Target = targetWithoutProperties });
507+
508+
var platform = new TargetPlatform (provider.Object) {
509+
GroupedTypes = new Dictionary<Type, string> {
510+
{ typeof(string), "strings" }
511+
}
512+
};
513+
514+
var vm = CreateVm (platform);
515+
vm.ArrangeMode = PropertyArrangeMode.Category;
516+
vm.AutoExpand = true;
517+
vm.SelectedObjects.Add (targetWithoutProperties);
518+
519+
Assume.That (vm.ArrangedEditors.Count, Is.EqualTo (0));
520+
521+
Assert.That (() => vm.SelectedObjects.ReplaceOrAdd (targetWithoutProperties, targetWithProperties),
522+
Throws.Nothing);
523+
Assert.That (vm.ArrangedEditors.Count, Is.EqualTo (1));
524+
525+
var group = vm.ArrangedEditors[0].Editors[0] as PropertyGroupViewModel;
526+
Assert.That (group, Is.Not.Null);
527+
Assert.That (group.Properties.Count, Is.EqualTo (2));
528+
}
529+
530+
[Test]
531+
public void GroupedTypeMultiselect ()
532+
{
533+
var outer = new object ();
534+
var inner = new object ();
535+
536+
var property = new Mock<IPropertyInfo> ();
537+
property.SetupGet (p => p.Type).Returns (typeof (string));
538+
property.SetupGet (p => p.Category).Returns ((string)null);
539+
property.SetupGet (p => p.Name).Returns ("name");
540+
541+
var property2 = new Mock<IPropertyInfo> ();
542+
property2.SetupGet (p => p.Type).Returns (typeof (string));
543+
property2.SetupGet (p => p.Category).Returns ((string)null);
544+
property2.SetupGet (p => p.Name).Returns ("name2");
545+
546+
var provider = new Mock<IEditorProvider> ();
547+
provider.Setup (ep => ep.GetObjectEditorAsync (outer))
548+
.ReturnsAsync (new MockObjectEditor (property.Object, property2.Object) { Target = outer });
549+
provider.Setup (ep => ep.GetObjectEditorAsync (inner))
550+
.ReturnsAsync (new MockObjectEditor (property.Object) { Target = inner });
551+
552+
var platform = new TargetPlatform (provider.Object) {
553+
GroupedTypes = new Dictionary<Type, string> {
554+
{ typeof(string), "strings" }
555+
}
556+
};
557+
558+
var vm = CreateVm (platform);
559+
vm.ArrangeMode = PropertyArrangeMode.Category;
560+
vm.AutoExpand = true;
561+
vm.SelectedObjects.Add (outer);
562+
563+
Assume.That (vm.ArrangedEditors.Count, Is.EqualTo (1));
564+
565+
var group = vm.ArrangedEditors[0].Editors[0] as PropertyGroupViewModel;
566+
Assume.That (group, Is.Not.Null);
567+
Assume.That (group.Properties.Count, Is.EqualTo (2));
568+
569+
bool shouldChange = false, changed = false;
570+
if (group.Properties is INotifyCollectionChanged incc) {
571+
shouldChange = true;
572+
incc.CollectionChanged += (o, e) => changed = true;
573+
}
574+
575+
vm.SelectedObjects.Add (inner);
576+
Assert.That (vm.ArrangedEditors[0].Editors[0] as PropertyGroupViewModel, Is.SameAs (group));
577+
Assert.That (group.Properties.Count, Is.EqualTo (1), "Number of remaining properties isn't correct");
578+
Assert.That (group.Properties[0].Property, Is.EqualTo (property.Object), "Wrong property found in the group");
579+
Assert.That (changed, Is.EqualTo (shouldChange), "Changed status didn't match expected");
580+
581+
vm.SelectedObjects.Remove (inner);
582+
group = vm.ArrangedEditors[0].Editors[0] as PropertyGroupViewModel;
583+
Assert.That (group, Is.Not.Null);
584+
Assert.That (group.Properties.Count, Is.EqualTo (2), "Outer properties didn't restore");
585+
}
586+
445587
internal override PanelViewModel CreateVm (TargetPlatform platform)
446588
{
447589
return new PanelViewModel (platform);

‎Xamarin.PropertyEditing/ViewModels/PanelViewModel.cs‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@ namespace Xamarin.PropertyEditing.ViewModels
1010
internal class PanelGroupViewModel
1111
: NotifyingObject
1212
{
13-
public PanelGroupViewModel (string category, IEnumerable<EditorViewModel> editors, bool separateUncommon = true)
13+
public PanelGroupViewModel (TargetPlatform targetPlatform, string category, IEnumerable<EditorViewModel> editors, bool separateUncommon = true)
1414
{
15+
if (targetPlatform == null)
16+
throw new ArgumentNullException (nameof(targetPlatform));
1517
if (editors == null)
1618
throw new ArgumentNullException (nameof(editors));
1719

20+
this.targetPlatform = targetPlatform;
1821
Category = category;
1922
AddCore (editors, separateUncommon);
2023
}
@@ -53,7 +56,19 @@ public bool Remove (EditorViewModel editor)
5356
if (editor == null)
5457
throw new ArgumentNullException (nameof(editor));
5558

56-
return GetList (editor, separate: true).Remove (editor);
59+
var list = GetList (editor, separate: true);
60+
if (editor is PropertyViewModel pvm && this.targetPlatform.GroupedTypes != null && this.targetPlatform.GroupedTypes.TryGetValue (pvm.Property.Type, out string groupName)) {
61+
var group = list.OfType<PropertyGroupViewModel> ().First (gvm => gvm.Category == groupName);
62+
if (group != null) {
63+
bool found = group.Remove (pvm);
64+
if (!group.HasChildElements)
65+
list.Remove (group);
66+
67+
return found;
68+
}
69+
}
70+
71+
return list.Remove (editor);
5772
}
5873

5974
public bool GetIsExpanded (PropertyArrangeMode mode)
@@ -80,6 +95,7 @@ public void SetIsExpanded (PropertyArrangeMode mode, bool expanded)
8095
private Dictionary<PropertyArrangeMode, bool> isExpanded;
8196
private readonly ObservableCollectionEx<EditorViewModel> editors = new ObservableCollectionEx<EditorViewModel> ();
8297
private readonly ObservableCollectionEx<EditorViewModel> uncommonEditors = new ObservableCollectionEx<EditorViewModel> ();
98+
private readonly TargetPlatform targetPlatform;
8399

84100
private void AddCore (IEnumerable<EditorViewModel> editors, bool separate)
85101
{
@@ -95,7 +111,16 @@ private void AddCore (EditorViewModel editor, bool separate)
95111
if (editor == null)
96112
throw new ArgumentNullException (nameof (editor));
97113

98-
GetList (editor, separate).Add (editor);
114+
var list = GetList (editor, separate);
115+
if (editor is PropertyViewModel pvm && this.targetPlatform.GroupedTypes != null && this.targetPlatform.GroupedTypes.TryGetValue (pvm.Property.Type, out string groupName)) {
116+
var group = list.OfType<PropertyGroupViewModel> ().First (gvm => gvm.Category == groupName);
117+
if (group != null)
118+
group.Add (pvm);
119+
else
120+
list.Add (editor);
121+
} else
122+
list.Add (editor);
123+
99124
OnPropertyChanged (nameof(HasChildElements));
100125
OnPropertyChanged (nameof(HasUncommonElements));
101126
}
@@ -249,17 +274,17 @@ protected override void OnAddEditors (IEnumerable<EditorViewModel> editors)
249274
string key = grouping.Key;
250275
if (remainingItems != null) {// TODO: pretty sure this was out of order before, add test
251276
if (remainingItems.Count > 0)
252-
this.arranged.Add (key, new PanelGroupViewModel (key, grouping.Where (evm => remainingItems.Contains (evm))));
277+
this.arranged.Add (key, new PanelGroupViewModel (TargetPlatform, key, grouping.Where (evm => remainingItems.Contains (evm))));
253278
} else
254-
this.arranged.Add (key, new PanelGroupViewModel (key, grouping, separateUncommon: !isFlat));
279+
this.arranged.Add (key, new PanelGroupViewModel (TargetPlatform, key, grouping, separateUncommon: !isFlat));
255280

256281
AutoExpandGroup (key);
257282
}
258283

259284
if (groupedTypeProperties != null) { // Insert type-grouped properties back in sorted.
260285
int i = 0;
261286
foreach (var kvp in groupedTypeProperties.OrderBy (kvp => kvp.Key, CategoryComparer.Instance)) {
262-
var group = new PanelGroupViewModel (kvp.Key, new[] { new PropertyGroupViewModel (TargetPlatform, kvp.Key, kvp.Value, ObjectEditors) });
287+
var group = new PanelGroupViewModel (TargetPlatform, kvp.Key, new[] { new PropertyGroupViewModel (TargetPlatform, kvp.Key, kvp.Value, ObjectEditors) });
263288

264289
bool added = false;
265290
for (; i < this.arranged.Count; i++) {
@@ -385,7 +410,14 @@ private void Filter (string oldFilter)
385410

386411
private string GetGroup (EditorViewModel vm)
387412
{
388-
return (ArrangeMode == PropertyArrangeMode.Name) ? "0" : (vm.Category ?? String.Empty);
413+
if (ArrangeMode == PropertyArrangeMode.Name)
414+
return "0";
415+
if (vm is PropertyViewModel pvm) {
416+
if (TargetPlatform.GroupedTypes != null && TargetPlatform.GroupedTypes.TryGetValue (pvm.Property.Type, out string groupName))
417+
return groupName;
418+
}
419+
420+
return vm.Category ?? String.Empty;
389421
}
390422

391423
private bool MatchesFilter (EditorViewModel vm)

‎Xamarin.PropertyEditing/ViewModels/PropertyGroupViewModel.cs‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public PropertyGroupViewModel (TargetPlatform platform, string category, IEnumer
2020

2121
Category = category;
2222

23-
this.properties = properties.ToArray ();
23+
this.properties = properties.ToList ();
2424
foreach (var vm in this.properties) {
2525
if (vm.IsAvailable)
2626
this.propertiesView.Add (vm);
@@ -55,10 +55,35 @@ public string FilterText
5555
}
5656
}
5757

58-
private readonly PropertyViewModel[] properties;
58+
public void Add (PropertyViewModel property)
59+
{
60+
if (property == null)
61+
throw new ArgumentNullException (nameof(property));
62+
63+
this.properties.Add (property);
64+
Reload ();
65+
}
66+
67+
public bool Remove (PropertyViewModel property)
68+
{
69+
if (property == null)
70+
throw new ArgumentNullException (nameof(property));
71+
72+
if (this.properties.Remove (property))
73+
return this.propertiesView.Remove (property);
74+
75+
return false;
76+
}
77+
78+
private readonly List<PropertyViewModel> properties;
5979
private readonly ObservableCollectionEx<PropertyViewModel> propertiesView = new ObservableCollectionEx<PropertyViewModel> ();
6080
private string filterText;
6181

82+
private void Reload ()
83+
{
84+
Filter (null);
85+
}
86+
6287
private void Filter (string oldFilter)
6388
{
6489
bool hadChildren = HasChildElements;

0 commit comments

Comments
 (0)