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

Commit 08b0c43

Browse files
committed
Implement CloseRequested in PanePageViewModelBase
Rather than making derved classes implement it if necessary. This is to prevent them maybe forgetting to dispose subscriptions to it.
1 parent f2c07d1 commit 08b0c43

File tree

4 files changed

+14
-8
lines changed

4 files changed

+14
-8
lines changed

src/GitHub.App/ViewModels/GitHubPane/NavigationViewModel.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public int RemoveAll(IPanePageViewModel page)
119119

120120
void BeforeItemAdded(IPanePageViewModel page)
121121
{
122-
if (page.CloseRequested != null && !history.Contains(page))
122+
if (!history.Contains(page))
123123
{
124124
page.CloseRequested.Subscribe(_ => RemoveAll(page));
125125
}

src/GitHub.App/ViewModels/GitHubPane/PanePageViewModelBase.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ namespace GitHub.ViewModels.GitHubPane
1212
public abstract class PanePageViewModelBase : ViewModelBase, IPanePageViewModel, IDisposable
1313
{
1414
static readonly Uri paneUri = new Uri("github://pane");
15-
Subject<Uri> navigate = new Subject<Uri>();
15+
readonly Subject<Uri> navigate = new Subject<Uri>();
16+
readonly Subject<Unit> close = new Subject<Unit>();
1617
Exception error;
1718
bool isBusy;
1819
bool isLoading;
@@ -59,7 +60,7 @@ public string Title
5960
}
6061

6162
/// <inheritdoc/>
62-
public virtual IObservable<Unit> CloseRequested { get; }
63+
public IObservable<Unit> CloseRequested => close;
6364

6465
/// <inheritdoc/>
6566
public IObservable<Uri> NavigationRequested => navigate;
@@ -84,6 +85,11 @@ public void Dispose()
8485
/// <inheritdoc/>
8586
public virtual Task Refresh() => Task.CompletedTask;
8687

88+
/// <summary>
89+
/// Sends a request to close the page.
90+
/// </summary>
91+
protected void Close() => close.OnNext(Unit.Default);
92+
8793
/// <summary>
8894
/// Sends a request to navigate to a new page.
8995
/// </summary>
@@ -96,8 +102,8 @@ protected virtual void Dispose(bool disposing)
96102
{
97103
if (disposing)
98104
{
99-
navigate?.Dispose();
100-
navigate = null;
105+
close.Dispose();
106+
navigate.Dispose();
101107
}
102108
}
103109
}

src/GitHub.App/ViewModels/GitHubPane/PullRequestCreationViewModel.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ public PullRequestCreationViewModel(
9595
});
9696

9797
Cancel = ReactiveCommand.Create();
98+
Cancel.Subscribe(_ => Close());
9899

99100
isExecuting = CreatePullRequest.IsExecuting.ToProperty(this, x => x.IsExecuting);
100101

@@ -298,7 +299,5 @@ ReactivePropertyValidator BranchValidator
298299
get { return branchValidator; }
299300
set { this.RaiseAndSetIfChanged(ref branchValidator, value); }
300301
}
301-
302-
public override IObservable<Unit> CloseRequested => Cancel.SelectUnit();
303302
}
304303
}

test/UnitTests/GitHub.App/ViewModels/GitHubPane/NavigationViewModelTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Reactive;
3+
using System.Reactive.Linq;
34
using System.Reactive.Subjects;
45
using GitHub.ViewModels.GitHubPane;
56
using NSubstitute;
@@ -412,7 +413,7 @@ public void CallsDeactivatedAndThenDisposedOnPages()
412413
static IPanePageViewModel CreatePage()
413414
{
414415
var result = Substitute.For<IPanePageViewModel>();
415-
result.CloseRequested.Returns((IObservable<Unit>)null);
416+
result.CloseRequested.Returns(Observable.Never<Unit>());
416417
return result;
417418
}
418419
}

0 commit comments

Comments
 (0)