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

Commit 78f59f5

Browse files
ermauermau
authored
Merge pull request #528 from xamarin/ermau-collection-fixes
[Core] Fix grouped type properties not swapping properly
2 parents e35b493 + d3e884a commit 78f59f5

File tree

3 files changed

+272
-14
lines changed

3 files changed

+272
-14
lines changed

‎Xamarin.PropertyEditing.Tests/PanelViewModelTests.cs‎

Lines changed: 201 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,11 +414,211 @@ public void AutoExpandChosenGroups ()
414414
Assert.That (vm.GetIsExpanded (normalProp.Object.Category), Is.True);
415415
}
416416

417+
[Test]
418+
public void GroupedTypesDoNotLeavePhantomCategory ()
419+
{
420+
var target = new object ();
421+
422+
var property = new Mock<IPropertyInfo> ();
423+
property.SetupGet (p => p.Type).Returns (typeof (string));
424+
property.SetupGet (p => p.Category).Returns ((string)null);
425+
property.SetupGet (p => p.Name).Returns ("name");
426+
427+
var provider = new Mock<IEditorProvider> ();
428+
provider.Setup (ep => ep.GetObjectEditorAsync (target))
429+
.ReturnsAsync (new MockObjectEditor (property.Object) { Target = target });
430+
431+
var platform = new TargetPlatform (provider.Object) {
432+
GroupedTypes = new Dictionary<Type, string> {
433+
{ typeof(string), "strings" }
434+
}
435+
};
436+
437+
var vm = CreateVm (platform);
438+
vm.ArrangeMode = PropertyArrangeMode.Category;
439+
vm.AutoExpand = true;
440+
vm.SelectedObjects.Add (target);
441+
442+
Assert.That (vm.ArrangedEditors.Count, Is.EqualTo (1));
443+
}
444+
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+
587+
[Test]
588+
public void GroupedTypeWhileNamed ()
589+
{
590+
var outer = new object ();
591+
592+
var property = new Mock<IPropertyInfo> ();
593+
property.SetupGet (p => p.Type).Returns (typeof (string));
594+
property.SetupGet (p => p.Category).Returns ((string)null);
595+
property.SetupGet (p => p.Name).Returns ("name");
596+
597+
var provider = new Mock<IEditorProvider> ();
598+
provider.Setup (ep => ep.GetObjectEditorAsync (outer))
599+
.ReturnsAsync (new MockObjectEditor (property.Object) { Target = outer });
600+
601+
var platform = new TargetPlatform (provider.Object) {
602+
GroupedTypes = new Dictionary<Type, string> {
603+
{ typeof(string), "strings" }
604+
}
605+
};
606+
607+
var vm = CreateVm (platform);
608+
vm.ArrangeMode = PropertyArrangeMode.Name;
609+
vm.SelectedObjects.Add (outer);
610+
611+
Assume.That (vm.ArrangedEditors.Count, Is.EqualTo (1));
612+
613+
var group = vm.ArrangedEditors[0].Editors[0] as StringPropertyViewModel;
614+
Assert.That (group, Is.Not.Null);
615+
}
616+
417617
internal override PanelViewModel CreateVm (TargetPlatform platform)
418618
{
419619
return new PanelViewModel (platform);
420620
}
421621

422622
private TestContext context;
423623
}
424-
}
624+
}

‎Xamarin.PropertyEditing/ViewModels/PanelViewModel.cs‎

Lines changed: 44 additions & 11 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> ().FirstOrDefault (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
}
@@ -246,19 +271,20 @@ protected override void OnAddEditors (IEnumerable<EditorViewModel> editors)
246271
}
247272
}
248273

249-
string key = grouping.Key ?? String.Empty;
250-
if (remainingItems != null) // TODO: pretty sure this was out of order before, add test
251-
this.arranged.Add (key, new PanelGroupViewModel (key, grouping.Where (evm => remainingItems.Contains (evm))));
252-
else
253-
this.arranged.Add (key, new PanelGroupViewModel (key, grouping, separateUncommon: !isFlat));
274+
string key = grouping.Key;
275+
if (remainingItems != null) {// TODO: pretty sure this was out of order before, add test
276+
if (remainingItems.Count > 0)
277+
this.arranged.Add (key, new PanelGroupViewModel (TargetPlatform, key, grouping.Where (evm => remainingItems.Contains (evm))));
278+
} else
279+
this.arranged.Add (key, new PanelGroupViewModel (TargetPlatform, key, grouping, separateUncommon: !isFlat));
254280

255281
AutoExpandGroup (key);
256282
}
257283

258284
if (groupedTypeProperties != null) { // Insert type-grouped properties back in sorted.
259285
int i = 0;
260286
foreach (var kvp in groupedTypeProperties.OrderBy (kvp => kvp.Key, CategoryComparer.Instance)) {
261-
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) });
262288

263289
bool added = false;
264290
for (; i < this.arranged.Count; i++) {
@@ -384,7 +410,14 @@ private void Filter (string oldFilter)
384410

385411
private string GetGroup (EditorViewModel vm)
386412
{
387-
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;
388421
}
389422

390423
private bool MatchesFilter (EditorViewModel vm)
@@ -399,4 +432,4 @@ private bool MatchesFilter (EditorViewModel vm)
399432
return vm.Name.Contains (FilterText, StringComparison.OrdinalIgnoreCase);
400433
}
401434
}
402-
}
435+
}

‎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)