Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 649387a

Browse files
shanashana
authored
Merge pull request #393 from github/fixes/390-none-added-twice-to-filters
Don't display [None] twice.
2 parents 245d9f0 + a9f4fb0 commit 649387a

File tree

11 files changed

+373
-186
lines changed

11 files changed

+373
-186
lines changed

‎src/GitHub.App/SampleData/PullRequestListViewModelDesigner.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public PullRequestListViewModelDesigner()
4444
SelectedAuthor = Authors.ElementAt(1);
4545
}
4646

47-
public ObservableCollection<IPullRequestModel> PullRequests { get; set; }
47+
public ITrackingCollection<IPullRequestModel> PullRequests { get; set; }
4848
public IPullRequestModel SelectedPullRequest { get; set; }
4949
public ICommand OpenPullRequest { get; set; }
5050

‎src/GitHub.App/ViewModels/PullRequestListViewModel.cs

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
1-
using GitHub.Exports;
2-
using System;
1+
using System;
32
using System.Collections.Generic;
4-
using System.Linq;
5-
using System.Text;
6-
using System.Threading.Tasks;
7-
using GitHub.Models;
83
using System.Collections.ObjectModel;
9-
using ReactiveUI;
10-
using NullGuard;
114
using System.ComponentModel.Composition;
12-
using GitHub.Services;
5+
using System.Linq;
136
using System.Reactive.Linq;
14-
using GitHub.Extensions.Reactive;
15-
using System.Windows.Data;
16-
using GitHub.Collections;
177
using System.Windows.Input;
18-
using GitHub.UI;
198
using System.Windows.Media.Imaging;
9+
using GitHub.Collections;
10+
using GitHub.Exports;
11+
using GitHub.Models;
12+
using GitHub.Services;
13+
using GitHub.UI;
14+
using NullGuard;
15+
using ReactiveUI;
2016

2117
namespace GitHub.ViewModels
2218
{
@@ -59,11 +55,11 @@ public PullRequestListViewModel(IRepositoryHost repositoryHost, ISimpleRepositor
5955
.Subscribe(s => UpdateFilter(s, SelectedAssignee, SelectedAuthor));
6056

6157
this.WhenAny(x => x.SelectedAssignee, x => x.Value)
62-
.Where(x => PullRequests != null)
58+
.Where(x => PullRequests != null && x != EmptyUser)
6359
.Subscribe(a => UpdateFilter(SelectedState, a, SelectedAuthor));
6460

6561
this.WhenAny(x => x.SelectedAuthor, x => x.Value)
66-
.Where(x => PullRequests != null)
62+
.Where(x => PullRequests != null && x != EmptyUser)
6763
.Subscribe(a => UpdateFilter(SelectedState, SelectedAssignee, a));
6864

6965
trackingAuthors = new TrackingCollection<IAccount>(Observable.Empty<IAccount>(),
@@ -73,8 +69,8 @@ public PullRequestListViewModel(IRepositoryHost repositoryHost, ISimpleRepositor
7369
trackingAuthors.Subscribe();
7470
trackingAssignees.Subscribe();
7571

76-
Authors = trackingAuthors.CreateListenerCollection(new List<IAccount> { EmptyUser });
77-
Assignees = trackingAssignees.CreateListenerCollection(new List<IAccount> { EmptyUser });
72+
Authors = trackingAuthors.CreateListenerCollection(EmptyUser, this.WhenAnyValue(x => x.SelectedAuthor));
73+
Assignees = trackingAssignees.CreateListenerCollection(EmptyUser, this.WhenAnyValue(x => x.SelectedAssignee));
7874

7975
PullRequests = new TrackingCollection<IPullRequestModel>();
8076
pullRequests.Comparer = OrderedComparer<IPullRequestModel>.OrderByDescending(x => x.UpdatedAt).Compare;
@@ -86,7 +82,7 @@ public override void Initialize([AllowNull] ViewWithData data)
8682
{
8783
base.Initialize(data);
8884

89-
PullRequests = repositoryHost.ModelService.GetPullRequests(repository, pullRequests) as TrackingCollection<IPullRequestModel>;
85+
PullRequests = repositoryHost.ModelService.GetPullRequests(repository, pullRequests);
9086
pullRequests.Subscribe(pr =>
9187
{
9288
trackingAssignees.AddItem(pr.Assignee);
@@ -104,12 +100,12 @@ void UpdateFilter(PullRequestState state, [AllowNull]IAccount ass, [AllowNull]IA
104100
(aut == null || aut.Equals(pr.Author));
105101
}
106102

107-
TrackingCollection<IPullRequestModel> pullRequests;
108-
public ObservableCollection<IPullRequestModel> PullRequests
103+
ITrackingCollection<IPullRequestModel> pullRequests;
104+
public ITrackingCollection<IPullRequestModel> PullRequests
109105
{
110106
[return: AllowNull]
111107
get { return pullRequests; }
112-
private set { this.RaiseAndSetIfChanged(ref pullRequests, (TrackingCollection<IPullRequestModel>)value); }
108+
private set { this.RaiseAndSetIfChanged(ref pullRequests, value); }
113109
}
114110

115111
IPullRequestModel selectedPullRequest;

‎src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs

Lines changed: 92 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using System.Reactive.Subjects;
1717
using System.Threading;
1818
using System.Linq;
19+
using System.Collections.Specialized;
1920

2021
namespace GitHub.Collections
2122
{
@@ -47,53 +48,107 @@ public static ObservableCollection<T> CreateListenerCollection<T>(this ITracking
4748
IList<T> stickieItemsOnTop = null)
4849
where T : class, ICopyable<T>
4950
{
50-
var col = new ObservableCollection<T>(stickieItemsOnTop ?? Enumerable.Empty<T>());
51-
tcol.CollectionChanged += (s, e) =>
51+
if (stickieItemsOnTop == null)
5252
{
53-
var offset = 0;
54-
if (stickieItemsOnTop != null)
55-
{
56-
foreach (var item in stickieItemsOnTop)
57-
{
58-
if (col.Contains(item))
59-
offset++;
60-
}
61-
}
53+
stickieItemsOnTop = new T[0];
54+
}
55+
56+
var col = new ObservableCollection<T>(stickieItemsOnTop.Concat(tcol));
57+
tcol.CollectionChanged += (_, e) => UpdateStickieItems(col, e, stickieItemsOnTop);
58+
return col;
59+
}
60+
61+
/// <summary>
62+
/// Creates an observable collection that tracks an <see cref="ITrackingCollection{T}"/>
63+
/// and adds a sticky item to the top of the collection when a related selection is null.
64+
/// </summary>
65+
/// <typeparam name="T">The type of items in the collection.</typeparam>
66+
/// <param name="tcol">The source tracking collection</param>
67+
/// <param name="stickieItemOnTop">The sticky item to add to the top of the collection.</param>
68+
/// <param name="selection">
69+
/// The current selection. If null or equal to the sticky item then the sticky item will be
70+
/// added to the collection.
71+
/// </param>
72+
/// <returns>An <see cref="ObservableCollection{T}"/>.</returns>
73+
public static ObservableCollection<T> CreateListenerCollection<T>(this ITrackingCollection<T> tcol,
74+
T stickieItemOnTop,
75+
IObservable<T> selection)
76+
where T : class, ICopyable<T>
77+
{
78+
Debug.Assert(stickieItemOnTop != null, "stickieItemOnTop may not be null in CreateListenerCollection");
79+
Debug.Assert(selection != null, "selection may not be null in CreateListenerCollection");
80+
81+
var stickieItems = new[] { stickieItemOnTop };
82+
var result = new ObservableCollection<T>(tcol);
83+
var hasSelection = false;
84+
85+
tcol.CollectionChanged += (_, e) =>
86+
{
87+
UpdateStickieItems(result, e, hasSelection ? stickieItems : null);
88+
};
6289

63-
if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Move)
90+
selection.Subscribe(x =>
91+
{
92+
hasSelection = x != null && !object.Equals(x, stickieItemOnTop);
93+
var hasStickie = result.FirstOrDefault() == stickieItemOnTop;
94+
95+
if (hasSelection && !hasStickie)
6496
{
65-
for (int i = 0, oldIdx = e.OldStartingIndex, newIdx = e.NewStartingIndex;
66-
i < e.OldItems.Count; i++, oldIdx++, newIdx++)
67-
{
68-
col.Move(oldIdx + offset, newIdx + offset);
69-
}
97+
result.Insert(0, stickieItemOnTop);
7098
}
71-
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Add)
99+
else if (hasStickie)
72100
{
73-
foreach (T item in e.NewItems)
74-
col.Add(item);
101+
result.Remove(stickieItemOnTop);
75102
}
76-
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Remove)
77-
{
78-
foreach (T item in e.OldItems)
79-
col.Remove(item);
80-
}
81-
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Replace)
103+
});
104+
105+
return result;
106+
}
107+
108+
static void UpdateStickieItems<T>(
109+
ObservableCollection<T> col,
110+
NotifyCollectionChangedEventArgs e,
111+
IList<T> stickieItemsOnTop)
112+
{
113+
var offset = 0;
114+
if (stickieItemsOnTop != null)
115+
{
116+
if (object.Equals(col.FirstOrDefault(), stickieItemsOnTop.FirstOrDefault()))
117+
offset = stickieItemsOnTop.Count;
118+
}
119+
120+
if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Move)
121+
{
122+
for (int i = 0, oldIdx = e.OldStartingIndex, newIdx = e.NewStartingIndex;
123+
i < e.OldItems.Count; i++, oldIdx++, newIdx++)
82124
{
83-
for (int i = 0, idx = e.OldStartingIndex; i < e.OldItems.Count; i++, idx++)
84-
col[idx + offset] = (T)e.NewItems[i];
125+
col.Move(oldIdx + offset, newIdx + offset);
85126
}
86-
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Reset)
127+
}
128+
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Add)
129+
{
130+
foreach (T item in e.NewItems)
131+
col.Add(item);
132+
}
133+
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Remove)
134+
{
135+
foreach (T item in e.OldItems)
136+
col.Remove(item);
137+
}
138+
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Replace)
139+
{
140+
for (int i = 0, idx = e.OldStartingIndex; i < e.OldItems.Count; i++, idx++)
141+
col[idx + offset] = (T)e.NewItems[i];
142+
}
143+
else if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Reset)
144+
{
145+
col.Clear();
146+
if (stickieItemsOnTop != null)
87147
{
88-
col.Clear();
89-
if (stickieItemsOnTop != null)
90-
{
91-
foreach (var item in stickieItemsOnTop)
92-
col.Add(item);
93-
}
148+
foreach (var item in stickieItemsOnTop)
149+
col.Add(item);
94150
}
95-
};
96-
return col;
151+
}
97152
}
98153
}
99154

‎src/GitHub.Exports.Reactive/ViewModels/IPullRequestListViewModel.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public override string ToString()
1919

2020
public interface IPullRequestListViewModel : IViewModel
2121
{
22-
ObservableCollection<IPullRequestModel> PullRequests { get; }
22+
ITrackingCollection<IPullRequestModel> PullRequests { get; }
2323
IPullRequestModel SelectedPullRequest { get; }
2424
ICommand OpenPullRequest { get; }
2525
IReadOnlyList<PullRequestState> States { get; set; }

‎src/GitHub.UI/Behaviours/AddEmptyItemToList.cs

Lines changed: 0 additions & 107 deletions
This file was deleted.

‎src/GitHub.UI/GitHub.UI.csproj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@
7777
<Reference Include="WindowsBase" />
7878
</ItemGroup>
7979
<ItemGroup>
80-
<Compile Include="Behaviours\AddEmptyItemToList.cs" />
8180
<Compile Include="Behaviours\ClosePopupAction.cs" />
8281
<Compile Include="Behaviours\OpenPopupAction.cs" />
8382
<Compile Include="Controls\Octicons\OcticonPaths.Designer.cs">

0 commit comments

Comments
 (0)