From ca4ea9d76cd1bfde89cca4df74bf75e0f3ce32bc Mon Sep 17 00:00:00 2001 From: khoi Date: Thu, 29 Jan 2026 14:59:24 +0700 Subject: [PATCH] Add checks popover for status rings --- Makefile | 4 +- .../Github/GithubPullRequestCheckState.swift | 9 ++ .../Github/GithubPullRequestStatusCheck.swift | 140 ++++++++++++++---- .../Views/PullRequestCheckStatusStyle.swift | 32 ++++ .../PullRequestChecksPopoverButton.swift | 25 ++++ .../Views/PullRequestChecksPopoverView.swift | 90 +++++++++++ .../Views/PullRequestStatusButton.swift | 61 +++----- .../Repositories/Views/WorktreeRow.swift | 4 + .../PullRequestCheckBreakdownTests.swift | 18 +++ 9 files changed, 312 insertions(+), 71 deletions(-) create mode 100644 supacode/Clients/Github/GithubPullRequestCheckState.swift create mode 100644 supacode/Features/Repositories/Views/PullRequestCheckStatusStyle.swift create mode 100644 supacode/Features/Repositories/Views/PullRequestChecksPopoverButton.swift create mode 100644 supacode/Features/Repositories/Views/PullRequestChecksPopoverView.swift diff --git a/Makefile b/Makefile index 9e639020..4c502c41 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,7 @@ lint: # Run swiftlint exit 0; \ fi; \ while IFS= read -r -d '' file; do \ - mise exec -- swiftlint --quiet --path "$$file"; \ + mise exec -- swiftlint --quiet "$$file"; \ done < "$(FILES_FILE)"; \ else \ mise exec -- swiftlint --quiet; \ @@ -86,7 +86,7 @@ format: # Swift format fi; \ xargs -0 swift-format -p --in-place --configuration ./.swift-format.json -- < "$(FILES_FILE)"; \ while IFS= read -r -d '' file; do \ - mise exec -- swiftlint --fix --quiet --path "$$file"; \ + mise exec -- swiftlint --fix --quiet "$$file"; \ done < "$(FILES_FILE)"; \ else \ swift-format -p --in-place --recursive --configuration ./.swift-format.json supacode supacodeTests; \ diff --git a/supacode/Clients/Github/GithubPullRequestCheckState.swift b/supacode/Clients/Github/GithubPullRequestCheckState.swift new file mode 100644 index 00000000..dbd16191 --- /dev/null +++ b/supacode/Clients/Github/GithubPullRequestCheckState.swift @@ -0,0 +1,9 @@ +import Foundation + +nonisolated enum GithubPullRequestCheckState: Equatable { + case success + case failure + case inProgress + case expected + case skipped +} diff --git a/supacode/Clients/Github/GithubPullRequestStatusCheck.swift b/supacode/Clients/Github/GithubPullRequestStatusCheck.swift index cd10d0be..a0e11a4e 100644 --- a/supacode/Clients/Github/GithubPullRequestStatusCheck.swift +++ b/supacode/Clients/Github/GithubPullRequestStatusCheck.swift @@ -1,9 +1,85 @@ import Foundation nonisolated struct GithubPullRequestStatusCheck: Decodable, Equatable, Hashable { + let name: String? + let detailsUrl: String? let status: String? let conclusion: String? let state: String? + + init( + name: String? = nil, + detailsUrl: String? = nil, + status: String? = nil, + conclusion: String? = nil, + state: String? = nil + ) { + self.name = name + self.detailsUrl = detailsUrl + self.status = status + self.conclusion = conclusion + self.state = state + } + + enum CodingKeys: String, CodingKey { + case name + case context + case detailsUrl + case targetUrl + case status + case conclusion + case state + } + + nonisolated init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let name = try container.decodeIfPresent(String.self, forKey: .name) + let context = try container.decodeIfPresent(String.self, forKey: .context) + self.name = name ?? context + let detailsUrl = try container.decodeIfPresent(String.self, forKey: .detailsUrl) + let targetUrl = try container.decodeIfPresent(String.self, forKey: .targetUrl) + self.detailsUrl = detailsUrl ?? targetUrl + self.status = try container.decodeIfPresent(String.self, forKey: .status) + self.conclusion = try container.decodeIfPresent(String.self, forKey: .conclusion) + self.state = try container.decodeIfPresent(String.self, forKey: .state) + } + + var checkState: GithubPullRequestCheckState { + if let status, status.uppercased() != "COMPLETED" { + return .inProgress + } + if let state { + switch state.uppercased() { + case "SUCCESS": + return .success + case "FAILURE", "ERROR": + return .failure + case "EXPECTED": + return .expected + case "PENDING": + return .inProgress + default: + return .inProgress + } + } + if let conclusion { + switch conclusion.uppercased() { + case "SUCCESS", "NEUTRAL": + return .success + case "CANCELLED", "SKIPPED": + return .skipped + case "FAILURE", "TIMED_OUT", "ACTION_REQUIRED", "STARTUP_FAILURE", "STALE": + return .failure + default: + return .inProgress + } + } + return .inProgress + } + + var displayName: String { + name ?? "Check" + } } nonisolated struct GithubPullRequestStatusCheckRollup: Decodable, Equatable, Hashable { @@ -46,6 +122,29 @@ nonisolated struct PullRequestCheckBreakdown: Equatable { passed + failed + inProgress + expected + skipped } + var summaryText: String { + var parts: [String] = [] + if failed > 0 { + parts.append("\(failed) failed") + } + if inProgress > 0 { + parts.append("\(inProgress) in progress") + } + if skipped > 0 { + parts.append("\(skipped) skipped") + } + if expected > 0 { + parts.append("\(expected) expected") + } + if total > 0 { + parts.append("\(passed) successful") + } + if parts.isEmpty { + return "Checks unavailable" + } + return parts.joined(separator: ", ") + } + init(checks: [GithubPullRequestStatusCheck]) { var passed = 0 var failed = 0 @@ -53,39 +152,18 @@ nonisolated struct PullRequestCheckBreakdown: Equatable { var expected = 0 var skipped = 0 for check in checks { - if let status = check.status, status.uppercased() != "COMPLETED" { + switch check.checkState { + case .success: + passed += 1 + case .failure: + failed += 1 + case .inProgress: inProgress += 1 - continue - } - if let state = check.state { - switch state.uppercased() { - case "SUCCESS": - passed += 1 - case "FAILURE", "ERROR": - failed += 1 - case "EXPECTED": - expected += 1 - case "PENDING": - inProgress += 1 - default: - inProgress += 1 - } - continue - } - if let conclusion = check.conclusion { - switch conclusion.uppercased() { - case "SUCCESS", "NEUTRAL": - passed += 1 - case "CANCELLED", "SKIPPED": - skipped += 1 - case "FAILURE", "TIMED_OUT", "ACTION_REQUIRED", "STARTUP_FAILURE", "STALE": - failed += 1 - default: - inProgress += 1 - } - continue + case .expected: + expected += 1 + case .skipped: + skipped += 1 } - inProgress += 1 } self.passed = passed self.failed = failed diff --git a/supacode/Features/Repositories/Views/PullRequestCheckStatusStyle.swift b/supacode/Features/Repositories/Views/PullRequestCheckStatusStyle.swift new file mode 100644 index 00000000..49bd2ac1 --- /dev/null +++ b/supacode/Features/Repositories/Views/PullRequestCheckStatusStyle.swift @@ -0,0 +1,32 @@ +import SwiftUI + +struct PullRequestCheckStatusStyle { + let symbol: String + let color: Color + let label: String + + init(state: GithubPullRequestCheckState) { + switch state { + case .success: + self.symbol = "checkmark.circle.fill" + self.color = .green + self.label = "Success" + case .failure: + self.symbol = "xmark.circle.fill" + self.color = .red + self.label = "Failed" + case .inProgress: + self.symbol = "arrow.triangle.2.circlepath.circle.fill" + self.color = .yellow + self.label = "In progress" + case .expected: + self.symbol = "clock.circle.fill" + self.color = .yellow + self.label = "Expected" + case .skipped: + self.symbol = "minus.circle.fill" + self.color = .gray + self.label = "Skipped" + } + } +} diff --git a/supacode/Features/Repositories/Views/PullRequestChecksPopoverButton.swift b/supacode/Features/Repositories/Views/PullRequestChecksPopoverButton.swift new file mode 100644 index 00000000..a7ed9e12 --- /dev/null +++ b/supacode/Features/Repositories/Views/PullRequestChecksPopoverButton.swift @@ -0,0 +1,25 @@ +import SwiftUI + +struct PullRequestChecksPopoverButton: View { + let checks: [GithubPullRequestStatusCheck] + @State private var isPresented = false + + var body: some View { + if checks.isEmpty { + EmptyView() + } else { + let breakdown = PullRequestCheckBreakdown(checks: checks) + Button { + isPresented.toggle() + } label: { + PullRequestChecksRingView(breakdown: breakdown) + } + .buttonStyle(.plain) + .help("Show pull request checks") + .accessibilityLabel("Show pull request checks") + .popover(isPresented: $isPresented) { + PullRequestChecksPopoverView(checks: checks) + } + } + } +} diff --git a/supacode/Features/Repositories/Views/PullRequestChecksPopoverView.swift b/supacode/Features/Repositories/Views/PullRequestChecksPopoverView.swift new file mode 100644 index 00000000..47fa8733 --- /dev/null +++ b/supacode/Features/Repositories/Views/PullRequestChecksPopoverView.swift @@ -0,0 +1,90 @@ +import SwiftUI + +struct PullRequestChecksPopoverView: View { + let checks: [GithubPullRequestStatusCheck] + private let breakdown: PullRequestCheckBreakdown + private let sortedChecks: [GithubPullRequestStatusCheck] + @Environment(\.openURL) private var openURL + + init(checks: [GithubPullRequestStatusCheck]) { + self.checks = checks + self.breakdown = PullRequestCheckBreakdown(checks: checks) + self.sortedChecks = checks.sorted { + let left = Self.sortRank(for: $0.checkState) + let right = Self.sortRank(for: $1.checkState) + if left == right { + return $0.displayName.localizedStandardCompare($1.displayName) == .orderedAscending + } + return left < right + } + } + + var body: some View { + VStack(alignment: .leading) { + Text("Checks") + .font(.headline) + .monospaced() + + HStack { + PullRequestChecksRingView(breakdown: breakdown) + Text(breakdown.summaryText) + .foregroundStyle(.secondary) + } + .font(.caption) + + Divider() + + if sortedChecks.isEmpty { + Text("Checks unavailable") + .foregroundStyle(.secondary) + .font(.caption) + } else { + VStack(alignment: .leading) { + ForEach(sortedChecks, id: \.self) { check in + let style = PullRequestCheckStatusStyle(state: check.checkState) + HStack { + Image(systemName: style.symbol) + .foregroundStyle(style.color) + .accessibilityHidden(true) + if let url = check.detailsUrl.flatMap(URL.init(string:)) { + Button { + openURL(url) + } label: { + Text(check.displayName) + .lineLimit(1) + } + .buttonStyle(.plain) + .help("Open check details on GitHub") + } else { + Text(check.displayName) + .lineLimit(1) + } + Spacer() + Text(style.label) + .foregroundStyle(.secondary) + } + .font(.caption) + } + } + } + } + .padding() + .frame(minWidth: 260) + } + + private static func sortRank(for state: GithubPullRequestCheckState) -> Int { + switch state { + case .failure: + return 0 + case .inProgress: + return 1 + case .expected: + return 2 + case .skipped: + return 3 + case .success: + return 4 + } + } + +} diff --git a/supacode/Features/Repositories/Views/PullRequestStatusButton.swift b/supacode/Features/Repositories/Views/PullRequestStatusButton.swift index b0904a72..f6cc05fb 100644 --- a/supacode/Features/Repositories/Views/PullRequestStatusButton.swift +++ b/supacode/Features/Repositories/Views/PullRequestStatusButton.swift @@ -5,28 +5,30 @@ struct PullRequestStatusButton: View { @Environment(\.openURL) private var openURL var body: some View { - Button { - if let url = model.url { - openURL(url) + HStack(spacing: 6) { + if !model.statusChecks.isEmpty { + PullRequestChecksPopoverButton(checks: model.statusChecks) } - } label: { - HStack(spacing: 6) { - if let checkBreakdown = model.checkBreakdown { - PullRequestChecksRingView(breakdown: checkBreakdown) + Button { + if let url = model.url { + openURL(url) } - PullRequestBadgeView( - text: model.badgeText, - color: model.badgeColor - ) - if let detailText = model.detailText { - Text(detailText) + } label: { + HStack(spacing: 6) { + PullRequestBadgeView( + text: model.badgeText, + color: model.badgeColor + ) + if let detailText = model.detailText { + Text(detailText) + } } } - .font(.caption) - .monospaced() + .buttonStyle(.plain) + .help(model.helpText) } - .buttonStyle(.plain) - .help(model.helpText) + .font(.caption) + .monospaced() } } @@ -35,7 +37,7 @@ struct PullRequestStatusModel: Equatable { let number: Int let state: String? let url: URL? - let checkBreakdown: PullRequestCheckBreakdown? + let statusChecks: [GithubPullRequestStatusCheck] let detailText: String? init?(snapshot: WorktreeInfoSnapshot?) { @@ -52,37 +54,20 @@ struct PullRequestStatusModel: Equatable { self.url = snapshot.pullRequestURL.flatMap(URL.init(string:)) if state == "MERGED" { self.detailText = "Merged" - self.checkBreakdown = nil + self.statusChecks = [] return } let isDraft = snapshot.pullRequestIsDraft let prefix = "\(isDraft ? "(Drafted) " : "")↗ - " let checks = snapshot.pullRequestStatusChecks + self.statusChecks = checks if checks.isEmpty { self.detailText = prefix + "Checks unavailable" - self.checkBreakdown = nil return } let breakdown = PullRequestCheckBreakdown(checks: checks) let checksLabel = breakdown.total == 1 ? "check" : "checks" - var parts: [String] = [] - if breakdown.failed > 0 { - parts.append("\(breakdown.failed) failed") - } - if breakdown.inProgress > 0 { - parts.append("\(breakdown.inProgress) in progress") - } - if breakdown.skipped > 0 { - parts.append("\(breakdown.skipped) skipped") - } - if breakdown.expected > 0 { - parts.append("\(breakdown.expected) expected") - } - if breakdown.total > 0 { - parts.append("\(breakdown.passed) successful") - } - self.detailText = prefix + parts.joined(separator: ", ") + " \(checksLabel)" - self.checkBreakdown = breakdown + self.detailText = prefix + breakdown.summaryText + " \(checksLabel)" } var badgeText: String { diff --git a/supacode/Features/Repositories/Views/WorktreeRow.swift b/supacode/Features/Repositories/Views/WorktreeRow.swift index 520435ee..3d23f211 100644 --- a/supacode/Features/Repositories/Views/WorktreeRow.swift +++ b/supacode/Features/Repositories/Views/WorktreeRow.swift @@ -29,6 +29,7 @@ struct WorktreeRow: View { let pullRequestState = displayPullRequest?.state.uppercased() let pullRequestNumber = displayPullRequest?.number let pullRequestURL = displayPullRequest.flatMap { URL(string: $0.url) } + let pullRequestChecks = displayPullRequest?.statusCheckRollup?.checks ?? [] let pullRequestBadgeStyle = PullRequestBadgeStyle.style( state: pullRequestState, number: pullRequestNumber @@ -77,6 +78,9 @@ struct WorktreeRow: View { .help("Run script active") .accessibilityLabel("Run script active") } + if !pullRequestChecks.isEmpty { + PullRequestChecksPopoverButton(checks: pullRequestChecks) + } if let pullRequestBadgeStyle { pullRequestBadge( text: pullRequestBadgeStyle.text, diff --git a/supacodeTests/PullRequestCheckBreakdownTests.swift b/supacodeTests/PullRequestCheckBreakdownTests.swift index f3e2af8b..fa897b86 100644 --- a/supacodeTests/PullRequestCheckBreakdownTests.swift +++ b/supacodeTests/PullRequestCheckBreakdownTests.swift @@ -36,4 +36,22 @@ struct PullRequestCheckBreakdownTests { #expect(breakdown.inProgress == 3) #expect(breakdown.total == 3) } + + @Test func breakdownSummaryTextIncludesAllStatuses() { + let checks = [ + GithubPullRequestStatusCheck(status: "IN_PROGRESS", conclusion: "SUCCESS", state: "SUCCESS"), + GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: nil, state: "EXPECTED"), + GithubPullRequestStatusCheck(status: "COMPLETED", conclusion: nil, state: "PENDING"), + GithubPullRequestStatusCheck(status: nil, conclusion: "SKIPPED", state: nil), + GithubPullRequestStatusCheck(status: nil, conclusion: "SUCCESS", state: nil), + GithubPullRequestStatusCheck(status: nil, conclusion: "FAILURE", state: nil), + ] + + let breakdown = PullRequestCheckBreakdown(checks: checks) + + #expect( + breakdown.summaryText + == "1 failed, 2 in progress, 1 skipped, 1 expected, 1 successful" + ) + } } -- 2.51.2