From b8bd0216aa9be2295d9a0b0200ecfd968fbbc220 Mon Sep 17 00:00:00 2001 From: Norton Andreev Date: Fri, 25 Sep 2026 15:03:39 +0500 Subject: [PATCH 01/12] Simplify the desktop pull request reader (#1157) A requested reviewer with no review carries no state from GitHub, so it now reads as awaiting rather than unknown. Avatars load through MarkdownImages, the app's one credential-free https image fetch; a failed load leaves the initial in place. --- ...PullRequestContextViewModel.Projections.cs | 61 +++++++---- .../ViewModels/PullRequestRow.cs | 5 +- .../Views/PullRequestReader.axaml | 101 ++++++++++-------- .../Views/PullRequestStatusLabel.axaml | 2 +- .../Views/PullRequestStyles.axaml | 55 +++++++--- src/Capacitor.App/Views/RemoteAvatar.cs | 38 +++++++ .../FakePullRequestSource.cs | 3 +- .../PullRequestPresentationTests.cs | 64 ++++++++++- 8 files changed, 238 insertions(+), 91 deletions(-) create mode 100644 src/Capacitor.App/Views/RemoteAvatar.cs diff --git a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs index 1628de60ae..0ce3b7917e 100644 --- a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs +++ b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs @@ -8,7 +8,9 @@ namespace Capacitor.App.ViewModels; public sealed partial class PullRequestContextViewModel { public string Title => CanDisplay && _overview?.Title is { } title ? title : _selected is { IsAvailable: true } choice ? choice.Link.Title ?? choice.Label : _selected?.Label ?? ""; - public string Branches => CanDisplay ? (_overview?.HeadRef ?? "?") + " → " + (_overview?.BaseRef ?? "?") : ""; + public string Branches => CanDisplay && (_overview?.HeadRef is not null || _overview?.BaseRef is not null) + ? (_overview.HeadRef ?? "?") + " → " + (_overview.BaseRef ?? "?") : ""; + public bool HasBranches => Branches.Length > 0; public string FetchedLabel => CanDisplay && _overviewRead?.FetchedAt is { } at ? "Fetched " + at.ToLocalTime().ToString("HH:mm:ss", CultureInfo.CurrentCulture) : ""; public string AccessLabel => CanDisplay ? (_grace ? "Access refresh paused" : "Access checked for " + (_overview?.AccessCheckedFor ?? "linked GitHub account")) : ""; public string? Description => CanDisplayReader && _section == "overview" ? _overview?.Description : null; @@ -32,25 +34,30 @@ public string PageNote { if (CurrentSection is not { } state) return _pageRequests.Contains(SectionKey) ? "Loading…" : "Choose Refresh to load this section."; if (state.Error is { } error) return error; if (state.Coverage != "complete") return "Limited snapshot: an ordered subset. More may be available on GitHub."; - if (state.Pages.Count > 0 && state.Pages.Sum(page => page.Rows.Length) == 0) return _section switch { + return ""; + } + } + public string EmptyNote { + get { + if (_section == "overview" || !CanDisplayReader || CurrentSection is not { Error: null } state) return ""; + if (state.Pages.Count == 0 || state.Pages.Sum(page => page.Rows.Length) > 0) return ""; + return _section switch { "threads" when state.Excluded is { Kind: "exact", Value: > 0 } => $"No unresolved threads ({state.Excluded.Value} resolved).", - "threads" => IncludeResolved ? "No threads." : "No unresolved threads.", "checks" => "No checks reported.", "reviewers" => "No reviewers reported.", - "reviews" => "No published reviews.", _ => "No comments." + "threads" => IncludeResolved ? "No threads." : "No unresolved threads.", "checks" => "No checks reported.", "reviewers" => "No reviewers yet.", + "reviews" => "No submitted reviews.", _ => "No comments." }; - return ""; } } + public bool HasEmptyNote => EmptyNote.Length > 0; public string SnapshotLabel => CanDisplayReader && CurrentSection?.Completed is { } at ? "Snapshot " + at.ToLocalTime().ToString("HH:mm:ss", CultureInfo.CurrentCulture) + (CurrentSection.Head is { Length: >= 7 } head ? " · " + head[..7] : "") : ""; - public string SectionTitle => _section switch { "checks" => "Checks", "reviewers" => "Reviewers", "reviews" => "Published reviews", - "threads" => "Inline threads", "thread_comments" => "Thread replies", "conversation" => "Conversation", _ => "Description" }; static readonly string[] NotifiedProperties = [ nameof(Notice), nameof(IsReading), nameof(HasChoice), nameof(HasPullRequest), nameof(HasListed), nameof(IsLegacy), nameof(CanOpenReader), nameof(Section), nameof(CanReveal), nameof(CanDisplay), - nameof(Title), nameof(Branches), nameof(FetchedLabel), nameof(AccessLabel), + nameof(Title), nameof(Branches), nameof(HasBranches), nameof(EmptyNote), nameof(HasEmptyNote), nameof(FetchedLabel), nameof(AccessLabel), nameof(Description), nameof(DescriptionTruncated), nameof(DescriptionNote), nameof(IsOverview), nameof(IsThreads), nameof(IsThreadComments), nameof(IncludeResolved), nameof(HasNotice), nameof(ShowsSignIn), nameof(ShowsLinkGitHub), nameof(ShowReaderContent), nameof(Rows), nameof(HasMore), - nameof(CanReloadEarlier), nameof(PageNote), nameof(SnapshotLabel), nameof(SectionTitle), + nameof(CanReloadEarlier), nameof(PageNote), nameof(SnapshotLabel), nameof(ReaderNote), nameof(HasReaderNote), nameof(ShowsInstallTool), nameof(InstallToolLabel), ]; @@ -96,6 +103,11 @@ void ReconcileRows(PullRequestRow[] rows) { }; static string Author(PullRequestActorDto? actor) => actor is null ? "Unknown author" : actor.Kind == "team" ? actor.Name ?? actor.Login ?? "Team" : actor.Login ?? actor.Name ?? "Unknown author"; + static string Joined(params string?[] parts) => string.Join(" · ", parts.Where(part => !string.IsNullOrEmpty(part))); + /// GitHub serves a user's avatar at /{login}.png on github.com and Enterprise hosts alike; teams and bots keep their glyph. + static string? AvatarUrl(PullRequestActorDto? actor, PullRequestSubjectDto subject) => + subject.Provider == "github" && actor is { Kind: "user", Login: { Length: > 0 } login } && Uri.CheckHostName(subject.Host) == UriHostNameType.Dns + ? $"https://{subject.Host}/{Uri.EscapeDataString(login)}.png?size=64" : null; static string Dated(DateTime? at) => at?.ToLocalTime().ToString("g", CultureInfo.CurrentCulture) ?? ""; static string Outcome(string? value) => value switch { "success" => "Passed", "failure" => "Failed", "pending" => "Pending", "neutral" => "Neutral", "skipped" => "Skipped", @@ -109,28 +121,33 @@ PullRequestRow ToRow(PullRequestCommentDto item, PullRequestSubjectDto subject) ? new(item.Id, Author(item.Author), Dated(item.CreatedAt), item.Body, null, PrLink(item.Url, subject), "available", item.BodyTruncated, ActorKind: item.Author?.Kind) : Unavailable(item.Id, "Comment", PrLink(item.Url, subject), item.Availability); PullRequestRow ToRow(PullRequestReviewDto item, PullRequestSubjectDto subject) => item.Availability == "available" && item.State is not ("pending" or "PENDING") - ? new(item.Id, Author(item.Author), ReviewState(item.State) + " · " + Dated(item.SubmittedAt), item.Body, null, PrLink(item.Url, subject), "available", item.BodyTruncated) + ? new(item.Id, Author(item.Author), Joined(ReviewState(item.State), Dated(item.SubmittedAt)), item.Body, null, PrLink(item.Url, subject), "available", item.BodyTruncated) : Unavailable(item.Id, "Review", PrLink(item.Url, subject), item.Availability); - PullRequestRow ToRow(PullRequestReviewerDto item, PullRequestSubjectDto subject) => item.Availability == "available" - ? new(item.Id, Author(item.Actor), (item.Requested == true ? "Review requested · " : "") + ReviewState(item.ReviewState), - null, null, PrLink(item.Url, subject), "available", IsReviewer: true, ActorKind: item.Actor?.Kind, - Status: ReviewRowStatus(item.ReviewState), ReviewRequested: item.Requested == true) - : Unavailable(item.Id, "Reviewer", null, item.Availability) with { IsReviewer = true }; + PullRequestRow ToRow(PullRequestReviewerDto item, PullRequestSubjectDto subject) { + if (item.Availability != "available") return Unavailable(item.Id, "Reviewer", null, item.Availability) with { IsReviewer = true }; + var status = ReviewRowStatus(item.ReviewState, item.Requested == true); + return new(item.Id, Author(item.Actor), (item.Requested == true ? "Review requested · " : "") + status.Text, + null, null, PrLink(item.Url, subject), "available", IsReviewer: true, ActorKind: item.Actor?.Kind, AvatarUrl: AvatarUrl(item.Actor, subject), + Status: status, ReRequested: item.Requested == true && item.ReviewState is not null); + } PullRequestRow ToRow(PullRequestCheckDto item, PullRequestSubjectDto subject) => item.Availability == "available" - ? new(item.Id, item.Name ?? "Unnamed check", Outcome(item.Outcome) + " · " + (item.AppName ?? item.Source ?? "Unknown source") - + (PullRequestWire.CheckLink(item.Url) is { } url ? " · " + new Uri(url).Host : ""), + ? new(item.Id, item.Name ?? "Unnamed check", + Joined(item.AppName ?? item.Source, PullRequestWire.CheckLink(item.Url) is { } url ? new Uri(url).Host : null), null, null, PullRequestWire.CheckLink(item.Url), "available", IsCheck: true, Outcome: item.Outcome ?? "unknown", Status: new(Outcome(item.Outcome), item.Outcome switch { "success" => "success", "failure" or "timed_out" or "action_required" => "failure", "pending" => "pending", _ => "neutral" })) : Unavailable(item.Id, "Check", PullRequestWire.CheckLink(item.Url), item.Availability) with { IsCheck = true, Outcome = "unknown" }; - static PullRequestStatus ReviewRowStatus(string? state) => new(ReviewState(state), state switch { - "approved" => "success", "changes_requested" => "failure", "commented" => "commented", _ => "neutral" - }); + static PullRequestStatus ReviewRowStatus(string? state, bool requested) => state switch { + "approved" => new("Approved", "success"), "changes_requested" => new("Changes requested", "failure"), + "commented" => new("Commented", "commented"), "dismissed" => new("Dismissed"), + null when requested => new("Awaiting review", Detail: "Review requested; this reviewer has not submitted one yet."), + _ => new("Unknown", Detail: "GitHub did not report this reviewer's state."), + }; PullRequestRow ToRow(PullRequestThreadDto item, PullRequestSubjectDto subject) => item.Availability == "available" ? new(item.Id, (item.Path ?? "Unknown file") + (item.Line is { } line ? ":" + line.ToString(CultureInfo.InvariantCulture) : ""), - (item.IsResolved == true ? "Resolved" : item.IsResolved == false ? "Unresolved" : "Resolution unknown") - + (item.IsOutdated == true ? " · Outdated" : "") + " · " + Author(item.RootComment?.Availability == "available" ? item.RootComment.Author : null), + Joined(item.IsResolved switch { true => "Resolved", false => "Unresolved", _ => null }, item.IsOutdated == true ? "Outdated" : null, + item.RootComment is { Availability: "available", Author: { } author } ? Author(author) : null), item.RootComment?.Availability == "available" ? item.RootComment.Body : null, item.DiffHunk, PrLink(item.Url, subject), "available", item.HunkTruncated || item.RootComment?.BodyTruncated == true, IsThread: true) : Unavailable(item.Id, "Thread", PrLink(item.Url, subject), item.Availability); diff --git a/src/Capacitor.App/ViewModels/PullRequestRow.cs b/src/Capacitor.App/ViewModels/PullRequestRow.cs index 45394fd73d..74b16f2247 100644 --- a/src/Capacitor.App/ViewModels/PullRequestRow.cs +++ b/src/Capacitor.App/ViewModels/PullRequestRow.cs @@ -2,7 +2,8 @@ namespace Capacitor.App.ViewModels; public sealed record PullRequestRow(string Id, string Title, string Detail, string? Body, string? Hunk, string? Url, string Availability, bool Truncated = false, bool IsThread = false, bool IsCheck = false, string? Outcome = null, - bool IsReviewer = false, string? ActorKind = null, PullRequestStatus? Status = null, bool ReviewRequested = false) { + bool IsReviewer = false, string? ActorKind = null, PullRequestStatus? Status = null, bool ReRequested = false, + string? AvatarUrl = null) { public bool HasBody => Availability == "available" && Body is { Length: > 0 }; public bool HasHunk => Availability == "available" && Hunk is { Length: > 0 }; public bool HasLink => Url is not null; @@ -11,5 +12,5 @@ public sealed record PullRequestRow(string Id, string Title, string Detail, stri public string Initial => Title.Length > 0 ? WorkContextPersonViewModel.InitialOf(Title) : "?"; public string ItemNote => Availability switch { "available" => Truncated ? "Preview truncated — full text on GitHub." : "", "redacted" => "This item is hidden.", _ => "This item could not be read." }; - internal long Bytes => 256 + 2L * (Id.Length + Title.Length + Detail.Length + (Body?.Length ?? 0) + (Hunk?.Length ?? 0) + (Url?.Length ?? 0)); + internal long Bytes => 256 + 2L * (Id.Length + Title.Length + Detail.Length + (Body?.Length ?? 0) + (Hunk?.Length ?? 0) + (Url?.Length ?? 0) + (AvatarUrl?.Length ?? 0)); } diff --git a/src/Capacitor.App/Views/PullRequestReader.axaml b/src/Capacitor.App/Views/PullRequestReader.axaml index 9bb3d151bb..fa827d6d76 100644 --- a/src/Capacitor.App/Views/PullRequestReader.axaml +++ b/src/Capacitor.App/Views/PullRequestReader.axaml @@ -6,39 +6,26 @@ x:DataType="vm:PullRequestContextViewModel" Background="{StaticResource KcapCanvasBrush}"> - + - - - - - - - - - - - - - + + + @@ -50,55 +37,75 @@ ItemContainerTheme="{StaticResource PullRequestTabTheme}" AutomationProperties.Name="Pull request sections"> - - + + + + + + + + + + + + - - - - - - - + + + + + + + + + + - - - From 3d876f1a8697903c0c5c3372c54012707121ba1f Mon Sep 17 00:00:00 2001 From: Norton Andreev Date: Fri, 25 Sep 2026 16:07:30 +0500 Subject: [PATCH 06/12] Give the pull request reader one loading, empty and updated state (#1157) Refresh lives on the About this work pane alone, which already reloads the reader, so the footer only reports when the open tab was last updated. --- ...PullRequestContextViewModel.Projections.cs | 18 ++++++-- .../Views/PullRequestReader.axaml | 46 ++++++++++--------- .../Views/PullRequestStyles.axaml | 24 ++++++++++ .../FakePullRequestSource.cs | 7 ++- .../PullRequestPresentationTests.cs | 36 +++++++++++++++ 5 files changed, 103 insertions(+), 28 deletions(-) diff --git a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs index 0ce3b7917e..db9567f7af 100644 --- a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs +++ b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs @@ -11,7 +11,7 @@ public sealed partial class PullRequestContextViewModel { public string Branches => CanDisplay && (_overview?.HeadRef is not null || _overview?.BaseRef is not null) ? (_overview.HeadRef ?? "?") + " → " + (_overview.BaseRef ?? "?") : ""; public bool HasBranches => Branches.Length > 0; - public string FetchedLabel => CanDisplay && _overviewRead?.FetchedAt is { } at ? "Fetched " + at.ToLocalTime().ToString("HH:mm:ss", CultureInfo.CurrentCulture) : ""; + public string FetchedLabel => CanDisplay && _overviewRead?.FetchedAt is { } at ? "Updated " + at.ToLocalTime().ToString("HH:mm:ss", CultureInfo.CurrentCulture) : ""; public string AccessLabel => CanDisplay ? (_grace ? "Access refresh paused" : "Access checked for " + (_overview?.AccessCheckedFor ?? "linked GitHub account")) : ""; public string? Description => CanDisplayReader && _section == "overview" ? _overview?.Description : null; public bool DescriptionTruncated => CanDisplayReader && _overview?.DescriptionTruncated == true; @@ -31,7 +31,7 @@ public string PageNote { get { if (_section == "overview") return ""; if (!CanDisplayReader) return "Refresh access to open PR content."; - if (CurrentSection is not { } state) return _pageRequests.Contains(SectionKey) ? "Loading…" : "Choose Refresh to load this section."; + if (CurrentSection is not { } state) return _pageRequests.Contains(SectionKey) ? "" : "This section has not loaded yet."; if (state.Error is { } error) return error; if (state.Coverage != "complete") return "Limited snapshot: an ordered subset. More may be available on GitHub."; return ""; @@ -49,15 +49,23 @@ public string EmptyNote { } } public bool HasEmptyNote => EmptyNote.Length > 0; + /// A first load only: a reload keeps the rows it is replacing on screen. + public bool IsSectionLoading => _section != "overview" && CanDisplayReader && CurrentSection is null && _pageRequests.Contains(SectionKey); + public string LoadingNote => "Loading " + _section switch { + "checks" => "checks", "reviewers" => "reviewers", "reviews" => "reviews", "threads" => "threads", "thread_comments" => "replies", _ => "activity" + } + "…"; public string SnapshotLabel => CanDisplayReader && CurrentSection?.Completed is { } at - ? "Snapshot " + at.ToLocalTime().ToString("HH:mm:ss", CultureInfo.CurrentCulture) + (CurrentSection.Head is { Length: >= 7 } head ? " · " + head[..7] : "") : ""; + ? "Updated " + at.ToLocalTime().ToString("HH:mm:ss", CultureInfo.CurrentCulture) : ""; + /// The footer names one thing, the time; which commit the checks ran on rides its hover. + public string FreshnessDetail => !IsOverview && CanDisplayReader && CurrentSection?.Head is { Length: >= 7 } head + ? "Checks ran on commit " + head[..7] : "When GitHub last sent what this tab shows"; static readonly string[] NotifiedProperties = [ nameof(Notice), nameof(IsReading), nameof(HasChoice), nameof(HasPullRequest), nameof(HasListed), nameof(IsLegacy), nameof(CanOpenReader), nameof(Section), nameof(CanReveal), nameof(CanDisplay), - nameof(Title), nameof(Branches), nameof(HasBranches), nameof(EmptyNote), nameof(HasEmptyNote), nameof(FetchedLabel), nameof(AccessLabel), + nameof(Title), nameof(Branches), nameof(HasBranches), nameof(EmptyNote), nameof(HasEmptyNote), nameof(IsSectionLoading), nameof(LoadingNote), nameof(FetchedLabel), nameof(AccessLabel), nameof(Description), nameof(DescriptionTruncated), nameof(DescriptionNote), nameof(IsOverview), nameof(IsThreads), nameof(IsThreadComments), nameof(IncludeResolved), nameof(HasNotice), nameof(ShowsSignIn), nameof(ShowsLinkGitHub), nameof(ShowReaderContent), nameof(Rows), nameof(HasMore), - nameof(CanReloadEarlier), nameof(PageNote), nameof(SnapshotLabel), + nameof(CanReloadEarlier), nameof(PageNote), nameof(SnapshotLabel), nameof(FreshnessDetail), nameof(ReaderNote), nameof(HasReaderNote), nameof(ShowsInstallTool), nameof(InstallToolLabel), ]; diff --git a/src/Capacitor.App/Views/PullRequestReader.axaml b/src/Capacitor.App/Views/PullRequestReader.axaml index 78472a1e63..fd51f9d1a7 100644 --- a/src/Capacitor.App/Views/PullRequestReader.axaml +++ b/src/Capacitor.App/Views/PullRequestReader.axaml @@ -83,9 +83,13 @@ IsVisible="{Binding DescriptionTruncated}" Classes="prNote" /> - - + + + + + + + - - - + + + + + + + + + + + + + + + + - diff --git a/src/Capacitor.App/Views/PullRequestStyles.axaml b/src/Capacitor.App/Views/PullRequestStyles.axaml index 1688f02f96..3e32147caa 100644 --- a/src/Capacitor.App/Views/PullRequestStyles.axaml +++ b/src/Capacitor.App/Views/PullRequestStyles.axaml @@ -124,6 +124,30 @@ + + + + diff --git a/test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs b/test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs index 14892d7882..ccf9ee1780 100644 --- a/test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs +++ b/test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs @@ -11,6 +11,8 @@ internal sealed class FakePullRequestSource(FakeTimeProvider time) : IPullReques public int Pages; public int TotalPages = 3; public bool EmptyPages; + /// Held open, every page read waits on it: the section stays in its first load. + public TaskCompletionSource? PageGate; public string? Failure; public string OverviewTitle = "Private PR"; public Func? PageItem; @@ -48,11 +50,12 @@ public Task>> PageAsync(string sessionI _ => new PullRequestCommentDto { Id = id, Availability = "available", Body = "Private comment" } }); var next = page + 1 < TotalPages ? (page + 1).ToString("x64", CultureInfo.InvariantCulture) : null; - return Task.FromResult(new PullRequestRead>(PullRequestReadKind.Ready, new() { + var read = new PullRequestRead>(PullRequestReadKind.Ready, new() { SnapshotId = new string('a', 64), SnapshotStartedAt = time.GetUtcNow().UtcDateTime, SnapshotCompletedAt = time.GetUtcNow().UtcDateTime, Coverage = "complete", HeadSha = section == "checks" ? new string('a', 40) : null, Total = new() { Kind = "exact", Value = TotalPages }, ExcludedByFilter = new() { Kind = "exact", Value = 0 }, Items = EmptyPages ? [] : [(T)item], PageCursor = page.ToString("x64", CultureInfo.InvariantCulture), NextCursor = next, HasMore = next is not null - }, subject, time.GetUtcNow().UtcDateTime, AccessValidForSeconds: 30, RequestStarted: time.GetTimestamp())); + }, subject, time.GetUtcNow().UtcDateTime, AccessValidForSeconds: 30, RequestStarted: time.GetTimestamp()); + return PageGate is { } gate ? gate.Task.ContinueWith(_ => read, TaskScheduler.Default) : Task.FromResult(read); } public static PullRequestLinkDto Link(int number) => new() { Provider = "github", Host = "github.com", RepoHash = "hash", Owner = "example", RepoName = "repo", Number = number, Url = $"https://github.com/example/repo/pull/{number}", Title = "Linked PR", HeadRef = "feature" }; diff --git a/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs b/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs index a5952c540a..615e2e8308 100644 --- a/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs +++ b/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs @@ -223,6 +223,21 @@ public Task A_refresh_click_shows_progress_at_once_and_runs_within_seconds() => await WorkspaceFixtures.WaitUntilAsync(() => h.Source.Lists > lists && !h.Model.IsReading, what: "manual refresh ran"); }); + /// Every tab's footer reads the same way; the commit the checks ran on is hover detail only. + [Test] + public Task The_footer_says_updated_on_every_tab() => RunOnUiAsync(async () => { + await using var h = new PullRequestViewTestHost(); + await h.ShowAsync(); + foreach (var section in new[] { "overview", "checks", "reviewers", "conversation" }) { + await h.Model.ShowSectionCommand.Execute(section); + await h.SettleAsync(); + await Assert.That(h.Model.FreshnessLabel).Matches(@"^Updated \d\d:\d\d:\d\d$"); + } + await h.Model.ShowSectionCommand.Execute("checks"); + await h.SettleAsync(); + await Assert.That(h.Model.FreshnessDetail).IsEqualTo("Checks ran on commit aaaaaaa"); + }); + [Test] public Task Row_details_leave_out_what_the_source_did_not_report() => RunOnUiAsync(async () => { await using var h = new PullRequestViewTestHost(); @@ -235,6 +250,27 @@ public Task Row_details_leave_out_what_the_source_did_not_report() => RunOnUiAsy await Assert.That(h.Model.Rows.Single().Detail).IsEmpty(); }); + /// A first load and the empty result it may turn into occupy one slot, one at a time. + [Test] + public Task A_first_load_shows_the_loading_state_in_the_empty_states_place() => RunOnUiAsync(async () => { + await using var h = new PullRequestViewTestHost(); + h.Source.EmptyPages = true; + await h.ShowAsync(); + h.Source.PageGate = new(); + await h.Model.ShowSectionCommand.Execute("reviews"); + Dispatcher.UIThread.RunJobs(); + var loading = h.Reader.FindControl("LoadingState")!; + var empty = h.Reader.FindControl("EmptyState")!; + await Assert.That(h.Model.LoadingNote).IsEqualTo("Loading reviews…"); + await Assert.That(loading.IsEffectivelyVisible).IsTrue(); + await Assert.That(empty.IsEffectivelyVisible).IsFalse(); + await Assert.That(h.Model.PageNote).IsEmpty(); + h.Source.PageGate.SetResult(); + await h.SettleAsync(); + await Assert.That(loading.IsEffectivelyVisible).IsFalse(); + await Assert.That(empty.IsEffectivelyVisible).IsTrue(); + }); + [Test] public Task An_empty_section_shows_the_empty_state_instead_of_a_page_note() => RunOnUiAsync(async () => { await using var h = new PullRequestViewTestHost(); From 3d8a73ba3b7ad75265784f8cc8f063c1c9574514 Mon Sep 17 00:00:00 2001 From: Norton Andreev Date: Fri, 25 Sep 2026 16:14:46 +0500 Subject: [PATCH 07/12] Reload the open pull request tab on the poll (#1157) A tab paged past its first page is left alone, since a reload restarts at page one. Progress shows only for a refresh someone clicked; the poll reads silently. --- ...PullRequestContextViewModel.Projections.cs | 3 +- .../PullRequestContextViewModel.Reads.cs | 7 +++- .../ViewModels/PullRequestContextViewModel.cs | 5 ++- .../ViewModels/WorkContextViewModel.cs | 13 ++++++- .../Views/PullRequestReader.axaml | 6 +-- src/Capacitor.App/Views/WorkContextView.axaml | 2 +- .../PullRequestPresentationTests.cs | 38 ++++++++++++++++++- 7 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs index db9567f7af..8839813155 100644 --- a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs +++ b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs @@ -62,7 +62,7 @@ public string EmptyNote { static readonly string[] NotifiedProperties = [ nameof(Notice), nameof(IsReading), nameof(HasChoice), nameof(HasPullRequest), nameof(HasListed), nameof(IsLegacy), nameof(CanOpenReader), nameof(Section), nameof(CanReveal), nameof(CanDisplay), - nameof(Title), nameof(Branches), nameof(HasBranches), nameof(EmptyNote), nameof(HasEmptyNote), nameof(IsSectionLoading), nameof(LoadingNote), nameof(FetchedLabel), nameof(AccessLabel), + nameof(Title), nameof(Branches), nameof(HasBranches), nameof(EmptyNote), nameof(HasEmptyNote), nameof(IsSectionLoading), nameof(LoadingNote), nameof(ShowsRefreshing), nameof(FetchedLabel), nameof(AccessLabel), nameof(Description), nameof(DescriptionTruncated), nameof(DescriptionNote), nameof(IsOverview), nameof(IsThreads), nameof(IsThreadComments), nameof(IncludeResolved), nameof(HasNotice), nameof(ShowsSignIn), nameof(ShowsLinkGitHub), nameof(ShowReaderContent), nameof(Rows), nameof(HasMore), nameof(CanReloadEarlier), nameof(PageNote), nameof(SnapshotLabel), nameof(FreshnessDetail), @@ -70,6 +70,7 @@ public string EmptyNote { ]; void Notify() { + if (!IsReading) _userRefresh = false; _readerNote = _readers is null ? null : _selected?.Subject is { } subject ? _readers.NoteFor(subject.Provider, subject.Host) : _primaryRepo?.Invoke() is { } repository ? _readers.NoteFor(repository.Provider, repository.Host) : null; diff --git a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs index 4ab9379054..c431e2f07f 100644 --- a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs +++ b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs @@ -87,8 +87,11 @@ void RequestOverview() { // any is still running, or the summary goes green beside rows that still say pending. var checksLive = _section == "checks" && CurrentSection is { } checks && (rollupMoved || checks.Pages.Any(page => page.Rows.Any(row => row.Outcome == "pending"))); - if (_readerVisible && _section != "overview" && (CurrentSection is null || _refreshPage || checksLive)) - RequestPage(null, refresh: _refreshPage || checksLive); + // The open tab follows the poll too, unless the reader paged past the first page: + // a reload restarts at page one and would pull the rows they were reading away. + var onePage = CurrentSection is { Pages.Count: 1, Earlier.Count: 0 }; + var reload = _refreshPage || checksLive || onePage; + if (_readerVisible && _section != "overview" && (CurrentSection is null || reload)) RequestPage(null, refresh: reload); _refreshPage = false; } else Fail(read); }; diff --git a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs index a4e15670dd..e119b0ce08 100644 --- a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs +++ b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs @@ -79,6 +79,9 @@ public PullRequestChoice? Selected { public bool ShowsInstallTool => _readerNote?.InstallUrl is not null; public string InstallToolLabel => _readerNote is null ? "" : "Install " + _readerNote.ToolName; public bool IsReading => _refreshing || _queuedRefresh || _overviewPending || _pageRequests.Count > 0; + bool _userRefresh; + /// Progress for a refresh someone asked for; the background poll reads silently. + public bool ShowsRefreshing => _userRefresh && IsReading; public bool HasChoice => _selected is not null; public bool HasPullRequest => _choices.Any(choice => choice.IsAvailable); public IObservable HasPullRequestChanges => _hasPullRequest; @@ -191,7 +194,7 @@ public void Reconnected() { RequestRefresh(); } /// The user's own refresh: rediscovers support and reloads the list, overview and open section. - public void Refresh() => RequestRefresh(manual: true); + public void Refresh() { _userRefresh = true; RequestRefresh(manual: true); Notify(); } /// On a legacy or unsupported capability the reader would open onto a notice and nothing else, /// so a caller with a PR in hand opens its URL instead. public bool CanOpenReader => HasPullRequest && !_legacy; diff --git a/src/Capacitor.App/ViewModels/WorkContextViewModel.cs b/src/Capacitor.App/ViewModels/WorkContextViewModel.cs index 623a0ba9ca..cf389a929f 100644 --- a/src/Capacitor.App/ViewModels/WorkContextViewModel.cs +++ b/src/Capacitor.App/ViewModels/WorkContextViewModel.cs @@ -191,9 +191,16 @@ public bool IsReading { private set { if (_isReading == value) return; this.RaiseAndSetIfChanged(ref _isReading, value); + if (!value) IsRefreshing = false; this.RaisePropertyChanged(nameof(RefreshTip)); } } + bool _isRefreshing; + /// A refresh someone asked for, still reading; the 30s poll runs without it. + public bool IsRefreshing { + get => _isRefreshing; + private set { if (_isRefreshing != value) { this.RaiseAndSetIfChanged(ref _isRefreshing, value); this.RaisePropertyChanged(nameof(RefreshTip)); } } + } bool _hasSession; // Subject, not WhenAnyValue — same RxAppBuilder init trap as SessionRailViewModel.SelectedAgentId. readonly BehaviorSubject _hasSessionChanges = new(false); @@ -213,7 +220,7 @@ private set { /// Tip on the header refresh control — bound with ShowOnDisabled so a greyed icon still explains itself. public string RefreshTip => HasSession - ? IsReading ? "Refreshing…" : "Reloads the work item, its pull requests and the plan" + ? IsRefreshing ? "Refreshing…" : "Reloads the work item, its pull requests and the plan" : "Waiting for the session ID"; public ReactiveCommand RefreshCommand { get; } @@ -249,6 +256,7 @@ public WorkContextViewModel( PullRequests?.Refresh(); Plan.Refresh(); if (_current is null) return; + IsRefreshing = true; if (_current.IsReading) _current.RefreshPending = true; else StartRead(_current); }, @@ -347,8 +355,9 @@ void Settle(ReadLease lease, WorkContextRead? read) { } catch (Exception ex) { Console.Error.WriteLine($"kcap: work context: {ex.Message}"); } + // A queued refresh keeps the pane reading, so the click that queued it keeps its progress. + if (lease.RefreshPending) { StartRead(lease); return; } IsReading = false; - if (lease.RefreshPending) StartRead(lease); } void OnTick() { diff --git a/src/Capacitor.App/Views/PullRequestReader.axaml b/src/Capacitor.App/Views/PullRequestReader.axaml index fd51f9d1a7..b121c52bf9 100644 --- a/src/Capacitor.App/Views/PullRequestReader.axaml +++ b/src/Capacitor.App/Views/PullRequestReader.axaml @@ -184,7 +184,7 @@ - + @@ -192,9 +192,9 @@ - + + StrokeLineCap="Round" Data="M11,2 A9,9 0 1 1 2,11" VerticalAlignment="Center" Classes.kcapSpinning="{Binding ShowsRefreshing}" /> diff --git a/src/Capacitor.App/Views/WorkContextView.axaml b/src/Capacitor.App/Views/WorkContextView.axaml index 0c82188613..11387089bc 100644 --- a/src/Capacitor.App/Views/WorkContextView.axaml +++ b/src/Capacitor.App/Views/WorkContextView.axaml @@ -304,7 +304,7 @@ + Classes.kcapSpinning="{Binding IsRefreshing}" Data="M11.5,6.5 A5,5 0 1 1 10,3 M10,0.5 L10,3 L7.5,3" /> diff --git a/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs b/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs index 615e2e8308..bb77ef2547 100644 --- a/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs +++ b/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs @@ -218,9 +218,45 @@ public Task A_refresh_click_shows_progress_at_once_and_runs_within_seconds() => await h.ShowAsync(); var lists = h.Source.Lists; h.Model.Refresh(); - await Assert.That(h.Model.IsReading).IsTrue(); + await Assert.That(h.Model.ShowsRefreshing).IsTrue(); h.Time.Advance(TimeSpan.FromSeconds(4)); await WorkspaceFixtures.WaitUntilAsync(() => h.Source.Lists > lists && !h.Model.IsReading, what: "manual refresh ran"); + await Assert.That(h.Model.ShowsRefreshing).IsFalse(); + }); + + /// The open tab follows the poll, and the poll never shows the progress a click does. + [Test] + public Task The_poll_reloads_the_open_tab_without_showing_progress() => RunOnUiAsync(async () => { + await using var h = new PullRequestViewTestHost(); + var body = "First review"; + h.Source.PageItem = section => section == "reviews" + ? new PullRequestReviewDto { Id = "review", Availability = "available", Body = body, State = "commented" } : null; + await h.ShowAsync(); + await h.Model.ShowSectionCommand.Execute("reviews"); + await h.SettleAsync(); + body = "Edited review"; + var sawProgress = false; + h.Model.PropertyChanged += (_, e) => { if (e.PropertyName == nameof(h.Model.ShowsRefreshing) && h.Model.ShowsRefreshing) sawProgress = true; }; + h.Time.Advance(TimeSpan.FromSeconds(31)); + await WorkspaceFixtures.WaitUntilAsync(() => h.Model.Rows.SingleOrDefault()?.Body == "Edited review", what: "open tab reloaded by the poll"); + await Assert.That(sawProgress).IsFalse(); + }); + + /// A reader who paged on keeps their rows: a reload would restart at page one. + [Test] + public Task The_poll_leaves_a_tab_alone_once_more_pages_are_loaded() => RunOnUiAsync(async () => { + await using var h = new PullRequestViewTestHost(); + h.Source.TotalPages = 3; + await h.ShowAsync(); + await h.Model.ShowSectionCommand.Execute("conversation"); + await h.SettleAsync(); + await h.Model.LoadMoreCommand.Execute(); + await h.SettleAsync(); + var pages = h.Source.Pages; + h.Time.Advance(TimeSpan.FromSeconds(31)); + await WorkspaceFixtures.WaitUntilAsync(() => h.Source.Overviews >= 2 && !h.Model.IsReading, what: "poll ran"); + await Assert.That(h.Source.Pages).IsEqualTo(pages); + await Assert.That(h.Model.Rows.Count).IsEqualTo(2); }); /// Every tab's footer reads the same way; the commit the checks ran on is hover detail only. From 482260d503bb8b7649adbf154d0348e1fce41b40 Mon Sep 17 00:00:00 2001 From: Norton Andreev Date: Fri, 25 Sep 2026 16:40:43 +0500 Subject: [PATCH 08/12] Reload a stale pull request tab when it is opened again (#1157) The poll reloads only the open tab, and the checks summary distrusts rows older than 30s, so rows kept past that beside a moved-on summary read as out of sync. --- ...ullRequestContextViewModel.Presentation.cs | 2 +- .../ViewModels/PullRequestContextViewModel.cs | 11 +- .../PullRequestPresentationTests.cs | 116 ++++-------------- 3 files changed, 32 insertions(+), 97 deletions(-) diff --git a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Presentation.cs b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Presentation.cs index ab6cbc15f8..ec7593278d 100644 --- a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Presentation.cs +++ b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.Presentation.cs @@ -58,7 +58,7 @@ public PullRequestStatus ChecksStatus { if (!CanDisplay) return new(""); var checks = _sections.GetValueOrDefault("checks"); if (checks is { Coverage: "complete", Stopped: false, Total.Kind: "exact", Completed: { } completed } - && checks.Head == _overview?.HeadSha && _time.GetUtcNow().UtcDateTime - completed < TimeSpan.FromSeconds(30) + && checks.Head == _overview?.HeadSha && _time.GetUtcNow().UtcDateTime - completed < RowsFreshFor && completed >= _overview?.Checks?.Availability.FetchedAt && checks.Pages.Sum(page => page.Rows.Length) == checks.Total.Value) { var rows = checks.Pages.SelectMany(page => page.Rows).ToArray(); if (rows.Length == 0) return new("No checks reported", Detail: "No checks were reported for this commit."); diff --git a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs index e119b0ce08..669f9be6f8 100644 --- a/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs +++ b/src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs @@ -225,7 +225,7 @@ public void SetReaderVisible(bool visible) { _readerVisible = visible; if (!visible && _grace) _graceSection = null; Notify(); - if (visible && CanReveal && _section != "overview" && CurrentSection is null) RequestPage(null); + if (visible && CanReveal && _section != "overview") LoadOrRefreshSection(); } void Select(PullRequestChoice? choice, bool explicitSelection = false) { if (_disposed || choice?.Subject == _selected?.Subject) return; @@ -253,8 +253,15 @@ void ShowSection(string section) { _openReader(); _readerVisible = true; Notify(); - if (section != "overview" && CurrentSection is null) RequestPage(null); + if (section != "overview") LoadOrRefreshSection(); } + /// A tab the poll skipped while it was hidden comes back stale; the summary stops trusting rows + /// older than 30s, so rows kept past that would disagree with it. + void LoadOrRefreshSection() { + if (CurrentSection is not { } state) RequestPage(null); + else if (state.Completed is not { } at || _time.GetUtcNow().UtcDateTime - at >= RowsFreshFor) RequestPage(null, refresh: true); + } + internal static readonly TimeSpan RowsFreshFor = TimeSpan.FromSeconds(30); void Tick() { if (_disposed || !_foreground) return; if (!_masked && _accessSeconds > 0 && Remaining <= 0 && !_grace) EnterGrace(); diff --git a/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs b/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs index bb77ef2547..cc1e4a6c3b 100644 --- a/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs +++ b/test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs @@ -3,12 +3,9 @@ using Avalonia; using Avalonia.Automation; using Avalonia.Controls; -using Avalonia.Controls.Presenters; using Avalonia.Controls.Primitives; -using Path = Avalonia.Controls.Shapes.Path; using Avalonia.Headless; using Avalonia.Input; -using Avalonia.Media; using Avalonia.Threading; using Avalonia.VisualTree; using Capacitor.App.Views; @@ -35,7 +32,6 @@ public Task Sidebar_status_actions_open_the_matching_tabs_and_review_subsections await Assert.That(AutomationProperties.GetName(checks)).IsEqualTo("Open checks: 1 failed"); var checksTab = h.Reader.FindControl("ChecksTab")!; await Assert.That(AutomationProperties.GetName(checksTab)).IsEqualTo("Checks: 1 failed"); - await Assert.That(checksTab.GetVisualDescendants().OfType().Single().IsEffectivelyVisible).IsTrue(); var review = h.Card.FindControl