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

Commit e35b493

Browse files
ermaualanmcgovern
ermau
authored and
alanmcgovern
committed
Revert "Grouped type fixes" (#527)
PR526 introduced a crash in named arrange mode and exposed a hole in our tests. All tests pass, but no properties are shown in either mode.
1 parent 938859b commit e35b493

File tree

3 files changed

+13
-241
lines changed

3 files changed

+13
-241
lines changed

‎Xamarin.PropertyEditing.Tests/PanelViewModelTests.cs‎

Lines changed: 1 addition & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ public async Task PropertyListCategoryFiltered ()
209209
[Test]
210210
public async Task PropertyListCategoryGroupedWithNullCategory ()
211211
{
212-
// Purposefully a null category
212+
// Purposefully a null catgory
213213
var normalProp = new Mock<IPropertyInfo> ();
214214
normalProp.SetupGet (p => p.Type).Returns (typeof(string));
215215
normalProp.SetupGet (p => p.Name).Returns ("name");
@@ -414,176 +414,6 @@ 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-
587417
internal override PanelViewModel CreateVm (TargetPlatform platform)
588418
{
589419
return new PanelViewModel (platform);

‎Xamarin.PropertyEditing/ViewModels/PanelViewModel.cs‎

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

20-
this.targetPlatform = targetPlatform;
2118
Category = category;
2219
AddCore (editors, separateUncommon);
2320
}
@@ -56,19 +53,7 @@ public bool Remove (EditorViewModel editor)
5653
if (editor == null)
5754
throw new ArgumentNullException (nameof(editor));
5855

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);
56+
return GetList (editor, separate: true).Remove (editor);
7257
}
7358

7459
public bool GetIsExpanded (PropertyArrangeMode mode)
@@ -95,7 +80,6 @@ public void SetIsExpanded (PropertyArrangeMode mode, bool expanded)
9580
private Dictionary<PropertyArrangeMode, bool> isExpanded;
9681
private readonly ObservableCollectionEx<EditorViewModel> editors = new ObservableCollectionEx<EditorViewModel> ();
9782
private readonly ObservableCollectionEx<EditorViewModel> uncommonEditors = new ObservableCollectionEx<EditorViewModel> ();
98-
private readonly TargetPlatform targetPlatform;
9983

10084
private void AddCore (IEnumerable<EditorViewModel> editors, bool separate)
10185
{
@@ -111,16 +95,7 @@ private void AddCore (EditorViewModel editor, bool separate)
11195
if (editor == null)
11296
throw new ArgumentNullException (nameof (editor));
11397

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-
98+
GetList (editor, separate).Add (editor);
12499
OnPropertyChanged (nameof(HasChildElements));
125100
OnPropertyChanged (nameof(HasUncommonElements));
126101
}
@@ -271,20 +246,19 @@ protected override void OnAddEditors (IEnumerable<EditorViewModel> editors)
271246
}
272247
}
273248

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));
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));
280254

281255
AutoExpandGroup (key);
282256
}
283257

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

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

411385
private string GetGroup (EditorViewModel vm)
412386
{
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;
387+
return (ArrangeMode == PropertyArrangeMode.Name) ? "0" : (vm.Category ?? String.Empty);
421388
}
422389

423390
private bool MatchesFilter (EditorViewModel vm)

‎Xamarin.PropertyEditing/ViewModels/PropertyGroupViewModel.cs‎

Lines changed: 2 additions & 27 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.ToList ();
23+
this.properties = properties.ToArray ();
2424
foreach (var vm in this.properties) {
2525
if (vm.IsAvailable)
2626
this.propertiesView.Add (vm);
@@ -55,35 +55,10 @@ public string FilterText
5555
}
5656
}
5757

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;
58+
private readonly PropertyViewModel[] properties;
7959
private readonly ObservableCollectionEx<PropertyViewModel> propertiesView = new ObservableCollectionEx<PropertyViewModel> ();
8060
private string filterText;
8161

82-
private void Reload ()
83-
{
84-
Filter (null);
85-
}
86-
8762
private void Filter (string oldFilter)
8863
{
8964
bool hadChildren = HasChildElements;

0 commit comments

Comments
 (0)