Skip to content

Revert "🎨 Redesign PopoverMainView — unified scrollable actions list (issue #294)" #312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eonist merged 1 commit into from
May 8, 2026
Merged

Revert "🎨 Redesign PopoverMainView — unified scrollable actions list (issue #294)" #312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
298 changes: 205 additions & 93 deletions Sources/RunnerBar/PopoverMainView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,19 @@
// ⚠️ REGRESSION GUARD — frame + padding rules (ref #52 #54 #57)
//
// RULE 1: Root VStack MUST use .frame(idealWidth: 420, maxWidth: .infinity, alignment: .top)
// AppDelegate reads hc.view.fittingSize in openPopover() to size the popover.
// ❌ NEVER remove .frame(idealWidth: 420)
// ❌ NEVER use .frame(width: 420)
// ❌ NEVER remove maxWidth: .infinity
// ❌ NEVER add .frame(height:) to root VStack
// AppDelegate reads hc.view.fittingSize in openPopover() to size the popover.
// ❌ NEVER remove .frame(idealWidth: 420)
// ❌ NEVER use .frame(width: 420)
// ❌ NEVER remove maxWidth: .infinity (VStack must stretch to full popover width)
// ❌ NEVER add .frame(height:) to root VStack
//
// RULE 2: ALL rows use .padding(.horizontal, 12)
// RULE 3: Job row HStack Spacer() is LOAD-BEARING.
// RULE 4: NEVER use .fixedSize() on any container.
// RULE 5: RunnerStoreObservable.reload() uses withAnimation(nil).

/// Root popover view — unified scrollable Actions list per issue #294.
/// Subviews are in PopoverMainViewSubviews.swift to satisfy SwiftLint
/// file_length (<400) and type_body_length (<200) limits.
// swiftlint:disable type_body_length
/// Root popover view. Shows system stats, action groups, active jobs, runners, and scope settings.
struct PopoverMainView: View {
/// The observable that bridges RunnerStore state into SwiftUI.
@ObservedObject var store: RunnerStoreObservable
Expand All @@ -29,126 +28,239 @@

@State private var isAuthenticated = (githubToken() != nil)
@StateObject private var systemStats = SystemStatsViewModel()
@State private var visibleCount: Int = 10
@State private var expandedGroupIDs: Set<String> = []

var body: some View {
VStack(alignment: .leading, spacing: 0) {
PopoverHeaderView(
stats: systemStats.stats,
isAuthenticated: isAuthenticated,
onSelectSettings: onSelectSettings,
onSignIn: signInWithGitHub
)
// NOTE: no unconditional Divider() here — PopoverLocalRunnerRow owns its
// own leading + trailing Dividers inside its @ViewBuilder guard (fix #2).
if store.isRateLimited { rateLimitBanner; Divider() }
PopoverLocalRunnerRow(runners: store.runners)
ScrollView {
VStack(alignment: .leading, spacing: 0) {
actionsSection
// ── Header
HStack {
Text("RunnerBar v0.34") // ⚠️ bump on every commit
.font(.headline).foregroundColor(.secondary)
Spacer()
Button(action: onSelectSettings) {
Image(systemName: "gearshape")
.font(.system(size: 14))
.foregroundColor(.secondary)
}
.buttonStyle(.plain)
.help("Settings")
.padding(.trailing, 4)
if isAuthenticated {
HStack(spacing: 4) {
Circle().fill(Color.green).frame(width: 8, height: 8)
Text("Authenticated").font(.caption).foregroundColor(.secondary)
}
} else {
Button(action: signInWithGitHub) {
HStack(spacing: 4) {
Circle().fill(Color.orange).frame(width: 8, height: 8)
Text("Sign in with GitHub").font(.caption).foregroundColor(.orange)
}
}.buttonStyle(.plain)
}
}
.padding(.horizontal, 12).padding(.top, 12).padding(.bottom, 8)
Divider()

if store.isRateLimited {
HStack(spacing: 6) {
Image(systemName: "exclamationmark.triangle.fill")
.foregroundColor(.yellow).font(.caption)
Text("GitHub rate limit reached — pausing polls")
.font(.caption).foregroundColor(.secondary)
}
.padding(.horizontal, 12).padding(.vertical, 4)
Divider()
}

// ── System
Text("System")
.font(.caption).foregroundColor(.secondary)
.padding(.horizontal, 12).padding(.top, 8).padding(.bottom, 2)
SystemStatsView(stats: systemStats.stats)
Divider()

// ── Actions
Text("Actions")
.font(.caption).foregroundColor(.secondary)
.padding(.horizontal, 12).padding(.top, 8).padding(.bottom, 2)
if store.actions.isEmpty {
Text("No recent actions")
.font(.caption).foregroundColor(.secondary)
.padding(.horizontal, 12).padding(.vertical, 4)
} else {
ForEach(store.actions.prefix(5)) { actionGroup in
Button(action: { onSelectAction(actionGroup) }, label: {
HStack(spacing: 8) {
actionDot(for: actionGroup)
Text(actionGroup.label)
.font(.caption.monospacedDigit())
.foregroundColor(.secondary)
.lineLimit(1)
.frame(width: 52, alignment: .leading)
Text(actionGroup.title)
.font(.system(size: 12))
.foregroundColor(actionGroup.isDimmed ? .secondary : .primary)
.lineLimit(1).truncationMode(.tail)
Spacer()
if actionGroup.groupStatus == .inProgress
|| actionGroup.groupStatus == .queued {
Text(actionGroup.currentJobName)
.font(.caption).foregroundColor(.secondary)
.lineLimit(1).truncationMode(.tail)
.frame(minWidth: 0, maxWidth: 80, alignment: .trailing)
}
Text(actionGroup.jobProgress)
.font(.caption.monospacedDigit()).foregroundColor(.secondary)
.frame(width: 30, alignment: .trailing)
Text(actionGroup.elapsed)
.font(.caption.monospacedDigit()).foregroundColor(.secondary)
.frame(width: 40, alignment: .trailing)
Image(systemName: "chevron.right")
.font(.caption2).foregroundColor(.secondary)
}
.padding(.horizontal, 12).padding(.vertical, 3)
})
.buttonStyle(.plain)
}
.padding(.bottom, 6)
}
Divider()

// ── Active Jobs
Text("Active Jobs")
.font(.caption).foregroundColor(.secondary)
.padding(.horizontal, 12).padding(.top, 8).padding(.bottom, 2)
if store.jobs.isEmpty {
Text("No active jobs")
.font(.caption).foregroundColor(.secondary)
.padding(.horizontal, 12).padding(.vertical, 4)
} else {
ForEach(store.jobs.prefix(3)) { job in
Button(action: { onSelectJob(job) }, label: {
HStack(spacing: 8) {
jobDot(for: job)
Text(job.name)
.font(.system(size: 12))
.foregroundColor(job.isDimmed ? .secondary : .primary)
.lineLimit(1).truncationMode(.tail)
Spacer()
Text(job.isDimmed ? conclusionLabel(for: job) : jobStatusLabel(for: job))
.font(.caption)
.foregroundColor(
job.isDimmed ? conclusionColor(for: job) : jobStatusColor(for: job)
)
.frame(width: 76, alignment: .trailing)
Text(job.elapsed)
.font(.caption.monospacedDigit()).foregroundColor(.secondary)
.frame(width: 40, alignment: .trailing)
Image(systemName: "chevron.right")
.font(.caption2).foregroundColor(.secondary)
}
.padding(.horizontal, 12).padding(.vertical, 3)
})
.buttonStyle(.plain)
}
.padding(.bottom, 6)
}
Divider()
quitButton
Button(action: { NSApplication.shared.terminate(nil) }, label: {
Text("Quit RunnerBar").font(.system(size: 12)).foregroundColor(.secondary)
})
.buttonStyle(.plain)
.padding(.horizontal, 12).padding(.vertical, 6)
}
.frame(idealWidth: 420, maxWidth: .infinity, alignment: .top)
.onAppear {
isAuthenticated = (githubToken() != nil)
systemStats.start()
}
Comment on lines 173 to 176
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Undefined systemstats.start() 🐞 Bug ≡ Correctness

PopoverMainView calls systemStats.start(), but SystemStatsViewModel does not define a start()
method, causing a compilation failure. This blocks the app from building.
Agent Prompt 
### Issue description
`PopoverMainView` calls `systemStats.start()`, but `SystemStatsViewModel` has no `start()` method, which will fail compilation.

### Issue Context
`SystemStatsViewModel` currently creates its repeating `Timer` in `init()`. You can either (a) add a `start()` API (and likely move timer creation out of `init()`), or (b) remove the call to `start()` and rely on `init()` (not recommended because it keeps polling even when popover is closed).

### Fix Focus Areas
- Sources/RunnerBar/PopoverMainView.swift[172-176]
- Sources/RunnerBar/SystemStats.swift[70-86]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

// Fix #4: tear down the repeating timer/publisher on dismiss to prevent
// SystemStatsViewModel leaking CPU cycles after the view is gone.
.onDisappear {
systemStats.stop()
}
}

// MARK: - Rate limit banner
// MARK: - Helpers

/// Yellow warning banner shown when GitHub rate-limit is hit.
private var rateLimitBanner: some View {
HStack(spacing: 6) {
Image(systemName: "exclamationmark.triangle.fill")
.foregroundColor(.yellow).font(.caption)
Text("GitHub rate limit reached — pausing polls")
.font(.caption).foregroundColor(.secondary)
}
.padding(.horizontal, 12).padding(.vertical, 4)
/// Dot color for an action group based on its status.
@ViewBuilder
private func actionDot(for group: ActionGroup) -> some View {
Circle()
.fill(actionDotColor(for: group))
.frame(width: 8, height: 8)
}

// MARK: - Actions section
/// Dot color for a job based on its status.
@ViewBuilder
private func jobDot(for job: ActiveJob) -> some View {
Circle()
.fill(jobDotColor(for: job))
.frame(width: 8, height: 8)
}

/// Unified scrollable actions list with inline job expansion and pagination.
private var actionsSection: some View {
VStack(alignment: .leading, spacing: 0) {
if store.actions.isEmpty {
Text("No recent actions")
.font(.caption).foregroundColor(.secondary)
.padding(.horizontal, 12).padding(.vertical, 8)
} else {
let visible = Array(store.actions.prefix(visibleCount))
ForEach(visible) { group in
ActionRowView(
group: group,
isExpanded: expandedGroupIDs.contains(group.id),
onSelect: { onSelectAction(group) },
onToggleExpand: { toggleExpand(group.id) }
)
if expandedGroupIDs.contains(group.id) {
InlineJobRowsView(group: group, onSelectJob: onSelectJob)
}
}
if store.actions.count > visibleCount { loadMoreButton }
}
/// Color for an action group's status dot.
private func actionDotColor(for group: ActionGroup) -> Color {
switch group.groupStatus {
case .inProgress: return .yellow
case .queued: return .blue
case .completed:
if group.isDimmed { return .gray }
return group.runs.allSatisfy({ $0.conclusion == "success" }) ? .green : .red
}
.padding(.vertical, 4)
}

/// "Load 10 more actions…" pagination button.
private var loadMoreButton: some View {
Button(
action: { visibleCount += 10 },
label: {
Text("Load \(min(10, store.actions.count - visibleCount)) more actions…")
.font(.caption).foregroundColor(.secondary)
}
)
.buttonStyle(.plain)
.padding(.horizontal, 12).padding(.vertical, 6)
/// Color for a job's status dot.
private func jobDotColor(for job: ActiveJob) -> Color {
switch job.status {
case "in_progress": return .yellow
case "queued": return .blue
default: return job.conclusion == "success" ? .green : (job.isDimmed ? .gray : .red)

Check warning on line 213 in Sources/RunnerBar/PopoverMainView.swift

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract the nested ternary operation into an independent statement. 


See more on https://sonarcloud.io/project/issues?id=eoncode_runner-bar&issues=AZ4HQNki-Q1TKFcsN1wN&open=AZ4HQNki-Q1TKFcsN1wN&pullRequest=312
}
}

// MARK: - Quit
/// Human-readable status label for a live job.
private func jobStatusLabel(for job: ActiveJob) -> String {
switch job.status {
case "in_progress": return "Running"
case "queued": return "Queued"
default: return job.status.capitalized
}
}

/// Quit RunnerBar button pinned to the bottom of the popover.
private var quitButton: some View {
Button(
action: { NSApplication.shared.terminate(nil) },
label: {
Text("Quit RunnerBar")
.font(.system(size: 12)).foregroundColor(.secondary)
}
)
.buttonStyle(.plain)
.padding(.horizontal, 12).padding(.vertical, 6)
/// Foreground color for a live job's status label.
private func jobStatusColor(for job: ActiveJob) -> Color {
switch job.status {
case "in_progress": return .yellow
case "queued": return .blue
default: return .secondary
}
}

// MARK: - Helpers
/// Human-readable conclusion label for a completed/dimmed job.
private func conclusionLabel(for job: ActiveJob) -> String {
switch job.conclusion {
case "success": return "Success"
case "failure": return "Failed"
case "cancelled": return "Cancelled"
case "skipped": return "Skipped"
default: return job.conclusion?.capitalized ?? "Done"
}
}

/// Toggles expand/collapse state for an action group.
private func toggleExpand(_ id: String) {
if expandedGroupIDs.contains(id) {
expandedGroupIDs.remove(id)
} else {
expandedGroupIDs.insert(id)
/// Foreground color for a completed job's conclusion label.
private func conclusionColor(for job: ActiveJob) -> Color {
switch job.conclusion {
case "success": return .green
case "failure": return .red
case "cancelled": return .orange
default: return .secondary
}
}

/// Opens the GitHub PAT setup docs in the default browser.
/// NSAppleScript/Terminal removed — device-flow requires a user_code the app never generates.
/// Auth.swift resolves token via: gh auth token → GH_TOKEN → GITHUB_TOKEN (ref #221 #246).
private func signInWithGitHub() {
let urlString = "https://docs.github.com/en/authentication/" +
"keeping-your-account-and-data-secure/managing-your-personal-access-tokens"
guard let url = URL(string: urlString) else { return }
NSWorkspace.shared.open(url)
}
}
// swiftlint:enable type_body_length
Loading
Loading