Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
67 changes: 49 additions & 18 deletions abgui/Sources/abgui/AppModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ final class AppModel {
/// fetch was still running. It is also called from inside a device detail sheet, so its
/// failures were landing on whatever list happened to be behind that sheet.
func loadOSReleases() async {
await run { self.osReleases = try await $0.osReleases() }
await run(.osReleases) { self.osReleases = try await $0.osReleases() }
}

// Apps & Books (VPP) — a separate service; the content token is held in-memory only.
Expand Down Expand Up @@ -121,7 +121,11 @@ final class AppModel {
}

// Per-screen UI state
var isLoading = false
//
// `loadError` belongs to the PLAN (DiffView renders it); every list screen owns its own via
// `failure(_:)`. The old shared `isLoading` is gone — the plan has `isPlanning`, the seed has
// `isSeeding`, and each list pane answers `loading(_:)` for itself, so no screen can spin or
// stop spinning because of work it did not start.
var loadError: String?
var progressLog: [String] = [] // live stderr narration from abctl during diff/seed
var applyProgressLog: [String] = [] // live sync/apply progress, separate from final outcomes
Expand Down Expand Up @@ -757,8 +761,8 @@ final class AppModel {

// MARK: loads (each spawns a fresh abctl; errors surface in loadError)

func loadConfigurations() async { await run { self.configurations = try await $0.configurations() } }
func loadBlueprints() async { await run { self.blueprints = try await $0.blueprints() } }
func loadConfigurations() async { await run(.configurations) { self.configurations = try await $0.configurations() } }
func loadBlueprints() async { await run(.blueprints) { self.blueprints = try await $0.blueprints() } }

/// Compute the 3-way plan. `resetLog: false` keeps whatever is already in `progressLog` and
/// appends this run's transcript underneath it — the seed → diff hand-off, where wiping the
Expand Down Expand Up @@ -970,7 +974,7 @@ final class AppModel {

/// Load a read-only resource (a live GET; never writes).
func loadReadOnly(_ kind: ReadOnlyKind) async {
await run { client in
await run(.readOnly(kind)) { client in
switch kind {
case .devices: self.devices = try await client.devices()
case .mdmDevices: self.mdmDevices = try await client.mdmDevices()
Expand Down Expand Up @@ -1267,31 +1271,58 @@ final class AppModel {
/// unrecoverable without relaunching. Two independent concerns need two independent tokens.
private var workGeneration = 0

/// Shared load wrapper: toggles isLoading, clears/sets loadError, runs `body`.
/// Cancellation (navigating away terminates the abctl child) is swallowed like
/// loadPlan's — never shown as an error — and only the LATEST run may write the
/// shared isLoading/loadError, so an overlapping older run can't clear a live
/// load's spinner or overwrite its error with a stale one.
private func run(_ body: (AbctlClient) async throws -> Void) async {
// MARK: per-pane load state
//
// One `isLoading`/`loadError` shared by every screen meant one screen's fetch was another
// screen's spinner and another screen's error. Concretely: a slow Devices fetch spun
// Configurations; an OS Releases failure — which can be raised from inside a device detail
// SHEET — rendered on whatever list was behind it; and a generation counter existed purely
// to stop two overlapping reads clobbering each other's message. A pane owns its own state
// instead, so "is this screen busy" is a question about that screen.

/// Which screen a load belongs to.
enum LoadPane: Hashable {
case configurations, blueprints, osReleases, contexts
case readOnly(ReadOnlyKind)
}

private var loadingPanes: Set<LoadPane> = []
private var paneErrors: [LoadPane: String] = [:]

/// Named `loading(_:)`/`failure(_:)` rather than overloading the existing `isLoading` and
/// `loadError` properties: those still belong to the Diff screen's plan, and two things with
/// one name is how this state got confusing in the first place.
func loading(_ pane: LoadPane) -> Bool { loadingPanes.contains(pane) }
func failure(_ pane: LoadPane) -> String? { paneErrors[pane] }

/// Per-pane load wrapper. Cancellation (navigating away terminates the abctl child) is
/// swallowed — never shown as an error — and a pane's state is only ever written by a load
/// of that pane, so two screens loading at once cannot interfere at all.
private func run(_ pane: LoadPane, _ body: (AbctlClient) async throws -> Void) async {
guard let client = makeClient() else {
loadError = "abctl was not found in the app bundle."
paneErrors[pane] = "abctl was not found in the app bundle."
return
}
// Still a generation, but scoped to THIS pane: re-entering a screen while its own
// previous fetch is in flight should not have the older one clear the newer's spinner.
loadGeneration += 1
let generation = loadGeneration
isLoading = true
loadError = nil
paneGeneration[pane] = generation
loadingPanes.insert(pane)
paneErrors[pane] = nil
do {
try await body(client)
} catch is CancellationError {
// user navigated away / cancelled — clear the in-flight state, no error shown
// user navigated away / cancelled — no error shown
} catch {
if !Task.isCancelled, generation == loadGeneration {
loadError = error.localizedDescription
if !Task.isCancelled, paneGeneration[pane] == generation {
paneErrors[pane] = error.localizedDescription
}
}
if generation == loadGeneration { isLoading = false }
if paneGeneration[pane] == generation { loadingPanes.remove(pane) }
}

private var paneGeneration: [LoadPane: Int] = [:]
}

/// The model owns the notice type; the banner that renders it spells it plainly.
Expand Down
37 changes: 31 additions & 6 deletions abgui/Sources/abgui/Views/ArchiveView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,15 @@ struct ArchiveFileView: View {
@Environment(\.dismiss) private var dismiss
let entry: ArchiveEntry

/// Read ONCE into state, not inside `body`.
///
/// The profile was previously loaded by a synchronous `String(contentsOf:)` in the view
/// body — up to Apple's 1 MiB cap, on the main actor, re-executed on every body evaluation
/// rather than once per file. The same class of mistake as rebuilding the whole transcript
/// per log line: work that looks free because it is written as an expression.
@State private var contents: String?
@State private var failed = false

var body: some View {
VStack(spacing: 0) {
HStack {
Expand All @@ -95,14 +104,30 @@ struct ArchiveFileView: View {
}
.padding()
Divider()
ScrollView([.horizontal, .vertical]) {
Text((try? String(contentsOf: entry.fileURL, encoding: .utf8)) ?? "(couldn't read this archived file)")
.font(.system(.footnote, design: .monospaced))
.textSelection(.enabled)
.padding()
.frame(maxWidth: .infinity, alignment: .leading)
if let contents {
ScrollView([.horizontal, .vertical]) {
Text(contents)
.font(.system(.footnote, design: .monospaced))
.textSelection(.enabled)
.padding()
.frame(maxWidth: .infinity, alignment: .leading)
}
} else if failed {
ContentUnavailableView("Couldn't read this archived file", systemImage: "exclamationmark.triangle",
description: Text(entry.fileURL.lastPathComponent))
.frame(maxWidth: .infinity, maxHeight: .infinity)
} else {
ProgressView().frame(maxWidth: .infinity, maxHeight: .infinity)
}
}
.frame(minWidth: 640, minHeight: 480)
.task(id: entry.fileURL) {
// Off the main actor: a large archived profile on a slow volume should not
// stall the window while the sheet is opening.
let url = entry.fileURL
let loaded = await Task.detached { try? String(contentsOf: url, encoding: .utf8) }.value
contents = loaded
failed = loaded == nil
}
}
}
7 changes: 4 additions & 3 deletions abgui/Sources/abgui/Views/AssignSheet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ struct AssignSheet: View {
@State private var activity: Resource? // the last polled activity status
@State private var statusBusy = false
@State private var statusError: String?
// The server-list fetch has its OWN busy/error state (like assign/poll above) —
// the shared model.isLoading/loadError belong to the list screens, and a swallowed
// failure here would masquerade as "No MDM servers found." on a write-gating picker.
// The server-list fetch has its OWN busy/error state (like assign/poll above): this is a
// sheet, not a pane, so it has no entry in the model's per-pane load state — and a
// swallowed failure here would masquerade as "No MDM servers found." on a write-gating
// picker.
@State private var serversBusy = false
@State private var serversError: String?

Expand Down
2 changes: 1 addition & 1 deletion abgui/Sources/abgui/Views/BlueprintsView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ struct BlueprintsView: View {
openDetail(ids.first)
}
.overlay {
ListStateOverlay(isLoading: model.isLoading, error: model.loadError,
ListStateOverlay(isLoading: model.loading(.blueprints), error: model.failure(.blueprints),
isEmpty: model.blueprints.isEmpty,
emptyTitle: "No blueprints", emptySymbol: "square.stack.3d.up")
}
Expand Down
49 changes: 38 additions & 11 deletions abgui/Sources/abgui/Views/Components.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,47 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import SwiftUI

/// How much room a toolbar control earns.
///
/// Showing the title on EVERY control was the first attempt, and it traded one problem for
/// another: an unlabelled toolbar is a row of anonymous glyphs with nothing to hover for, but a
/// fully labelled one outgrew the window — at ~1090px the last item was clipped off the right
/// edge with no overflow affordance. Width is finite, so it has to be spent where a word
/// actually resolves an ambiguity.
enum ToolbarWeight {
/// Title + icon, always. For the control whose consequence you must not misread: `Apply…`
/// writes the live tenant, and `Verify Configs` sits beside it wearing a near-identical
/// checkmark glyph. These two are exactly the pair a bare icon cannot distinguish.
case titled
/// Icon + tooltip. For controls whose glyph is universally understood — a refresh arrow, a
/// folder, a trash can — where the title costs width and adds nothing a hover does not.
case compact
}

extension View {
/// Make a toolbar control READ as what it does: show its title next to the icon, and give it
/// a tooltip saying what pressing it will actually do.
/// Make a toolbar control READ as what it does.
///
/// macOS renders a bare `Label` inside a `.toolbar` as its icon alone, and SwiftUI supplies
/// no tooltip of its own — so an unannotated toolbar is a row of anonymous glyphs with
/// nothing to hover for. That is not a cosmetic problem in this app: `checkmark.shield`
/// (validate local files) and `checkmark.circle` (write the live tenant) are near-identical
/// at toolbar size, and a `trash` means "delete from Apple Business" on one screen and
/// "clear this session's log" on another. The word is the affordance; `help` carries the
/// consequence, and should say plainly when a button reaches the tenant.
func toolbarLabel(_ help: String) -> some View {
labelStyle(.titleAndIcon).help(help)
/// `help` is supplied for BOTH weights and is never optional: macOS renders a bare `Label`
/// in a `.toolbar` as its icon alone and SwiftUI adds no tooltip of its own, so a control
/// without one is unidentifiable by any means at all. The tooltip should name the
/// consequence, and say plainly when the button reaches the tenant — a `trash` means
/// "delete from Apple Business" on one screen and "clear this session's log" on another.
func toolbarLabel(_ help: String, weight: ToolbarWeight = .compact) -> some View {
labelStyle(weight == .titled ? AnyLabelStyle(TitleAndIconLabelStyle())
: AnyLabelStyle(IconOnlyLabelStyle()))
.help(help)
}
}

/// Type-erasing wrapper so one modifier can choose between label styles at runtime.
/// `labelStyle` takes a concrete generic type, so a ternary over two different styles needs
/// this rather than being expressible inline.
struct AnyLabelStyle: LabelStyle {
private let make: (Configuration) -> AnyView
init<S: LabelStyle>(_ style: S) {
make = { AnyView(style.makeBody(configuration: $0)) }
}
func makeBody(configuration: Configuration) -> some View { make(configuration) }
}

/// A toolbar refresh button that runs an async action. `help` names WHAT is being re-fetched —
Expand Down
6 changes: 3 additions & 3 deletions abgui/Sources/abgui/Views/ConfigurationsView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ struct ConfigurationsView: View {
}
}
.overlay {
ListStateOverlay(isLoading: model.isLoading, error: model.loadError,
ListStateOverlay(isLoading: model.loading(.configurations), error: model.failure(.configurations),
isEmpty: model.configurations.isEmpty,
emptyTitle: "No configurations", emptySymbol: "doc.text")
}
.navigationTitle("Configurations")
.toolbar {
Button { editorTarget = .create } label: { Label("New", systemImage: "plus") }
.toolbarLabel("Write a new .mobileconfig and create it in Apple Business.")
.toolbarLabel("Write a new .mobileconfig and create it in Apple Business.", weight: .titled)
Button {
if let config = selectedResource { editorTarget = .edit(config) }
} label: { Label("Edit", systemImage: "pencil") }
Expand All @@ -71,7 +71,7 @@ struct ConfigurationsView: View {
.toolbarLabel("Attach or detach the selected configuration to a blueprint. Writes Apple Business and updates gitops/blueprints/.")
.disabled(selection == nil)
Button(role: .destructive) { confirmDelete = true } label: { Label("Delete", systemImage: "trash") }
.toolbarLabel("Delete the selected configuration from Apple Business (archived first).")
.toolbarLabel("Delete the selected configuration from Apple Business (archived first).", weight: .titled)
.disabled(selection == nil)
RefreshButton(help: "Re-fetch the configuration list from Apple Business.") {
await model.loadConfigurations()
Expand Down
19 changes: 13 additions & 6 deletions abgui/Sources/abgui/Views/DashboardView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,24 @@ struct DashboardView: View {
loaded.insert(item) // a cache its own screen already filled counts as loaded
continue
}
// Each load reports into its OWN pane, so the pass reads back the failure of the
// thing it just ran rather than whatever last touched a shared slot. That
// misattribution used to need a comment explaining why the error might not be ours.
var pane: AppModel.LoadPane
switch item {
case .configurations: await model.loadConfigurations()
case .blueprints: await model.loadBlueprints()
case .configurations:
pane = .configurations
await model.loadConfigurations()
case .blueprints:
pane = .blueprints
await model.loadBlueprints()
default:
guard let kind = item.readOnly else { continue }
pane = .readOnly(kind)
await model.loadReadOnly(kind)
}
// Cancelled mid-call (tile click / navigation): the shared loadError now
// belongs to the destination screen's load — don't read or misattribute it.
if Task.isCancelled { return }
if let error = model.loadError {
if Task.isCancelled { return } // navigated away mid-call
if let error = model.failure(pane) {
loadFailed = error // stop the pass on the first failure (see above)
return
}
Expand Down
19 changes: 10 additions & 9 deletions abgui/Sources/abgui/Views/DiffView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,19 @@ struct DiffView: View {
GitSourceOfTruthControl(layout: .toolbar,
isOn: model.gitSourceOfTruth,
pending: $pendingGitSourceOfTruth)
// These four SHOW THEIR TITLES. A macOS toolbar renders a bare `Label` as
// its icon alone, and with no `.help` there is not even a tooltip to fall
// back on — the row read as four anonymous glyphs, two of which
// (checkmark.shield / checkmark.circle) are near-identical at toolbar size
// while doing very different things: one only reads local files, the other
// writes the live tenant. The word is the affordance, exactly as it is on
// the Git-source-of-truth control sitting beside them.
// Titles go to the two controls a glyph cannot tell apart: `checkmark.shield`
// (reads local files) and `checkmark.circle` (writes the live tenant) are
// near-identical at toolbar size while doing very different things. Refresh
// and Workspace keep their universally-legible glyph plus a tooltip — giving
// every control a title is what made this toolbar outgrow the window and clip
// its last item at ~1090px.
Button { sheet = .validate } label: { Label("Verify Configs", systemImage: "checkmark.shield") }
.toolbarLabel("Check the profiles in gitops/lib against Apple's schema. Reads local files only — no tenant change.")
.toolbarLabel("Check the profiles in gitops/lib against Apple's schema. Reads local files only — no tenant change.",
weight: .titled)
.disabled(model.isSeeding)
Button { sheet = .apply } label: { Label("Apply…", systemImage: "checkmark.circle") }
.toolbarLabel("Reconcile Apple Business with this plan. Opens a sheet that previews the exact abctl command and asks you to confirm before anything is written.")
.toolbarLabel("Reconcile Apple Business with this plan. Opens a sheet that previews the exact abctl command and asks you to confirm before anything is written.",
weight: .titled)
.disabled((model.plan?.actionableChangeCount ?? 0) == 0)
Button { model.refreshPlan() } label: { Label("Refresh", systemImage: "arrow.clockwise") }
.toolbarLabel("Recompute the plan: re-read gitops/ and re-fetch the live tenant.")
Expand Down
4 changes: 2 additions & 2 deletions abgui/Sources/abgui/Views/OSReleasesView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ struct OSReleasesView: View {
TableColumn("Devices") { Text(String($0.supportedDevices?.count ?? 0)) }
}
.overlay {
if model.isLoading { ProgressView() }
else if let error = model.loadError { ContentUnavailableView("Couldn't load releases", systemImage: "exclamationmark.triangle", description: Text(error)) }
if model.loading(.osReleases) { ProgressView() }
else if let error = model.failure(.osReleases) { ContentUnavailableView("Couldn't load releases", systemImage: "exclamationmark.triangle", description: Text(error)) }
else if rows.isEmpty { ContentUnavailableView.search(text: search) }
}
}
Expand Down
Loading