diff --git a/Inspector/DelayedLoadingIndicator.swift b/Inspector/DelayedLoadingIndicator.swift new file mode 100644 index 0000000..ee0e705 --- /dev/null +++ b/Inspector/DelayedLoadingIndicator.swift @@ -0,0 +1,109 @@ +import Foundation + +// Decides when a loading indicator is worth showing. Loading that finishes +// within the grace period shows nothing at all, so a fast connection doesn't +// flash a spinner for a few frames; loading that runs longer shows the +// indicator, which then stays for a minimum time so it doesn't blink off. +// +// It only keeps time. The owner reports whether it is loading on every render +// and draws what `update(isLoading:)` answers; `onChange` asks it to render +// again when the answer changes on its own. Main thread only. Foundation only, +// so `make harness` can drive it on macOS with a scheduler of its own. +final class DelayedLoadingIndicator { + /// Runs `action` after `delay` seconds, and returns a way to cancel it. + typealias Schedule = (_ delay: TimeInterval, _ action: @escaping () -> Void) -> () -> Void + + private enum Stage { + case idle + // Loading, and the indicator is not due yet. + case waiting + // The indicator is up, and has not been for its minimum time. + case showing + // The indicator is up, and may go whenever the loading ends. + case settled + } + + static let defaultGracePeriod: TimeInterval = 0.75 + static let defaultMinimumVisibleTime: TimeInterval = 0.5 + + /// Called when the indicator appears, and when its minimum time is over. + var onChange: () -> Void = {} + + private let gracePeriod: TimeInterval + private let minimumVisibleTime: TimeInterval + private let schedule: Schedule + private var stage = Stage.idle + private var cancelPending: (() -> Void)? + + init( + gracePeriod: TimeInterval = defaultGracePeriod, + minimumVisibleTime: TimeInterval = defaultMinimumVisibleTime, + schedule: @escaping Schedule = DelayedLoadingIndicator.onMainQueue + ) { + self.gracePeriod = gracePeriod + self.minimumVisibleTime = minimumVisibleTime + self.schedule = schedule + } + + deinit { + cancelPending?() + } + + /// Whether the indicator is on screen. + var isVisible: Bool { + stage == .showing || stage == .settled + } + + /// The indicator has only just appeared. Content that is ready waits for + /// the next `onChange` rather than replacing it at once; a message such + /// as a failure need not wait, and ends it with `update(isLoading: false)`. + var holdsContent: Bool { + stage == .showing + } + + /// Reports whether the screen is loading, and answers whether the + /// indicator belongs on it. Loading that ends ends the indicator at once. + @discardableResult + func update(isLoading: Bool) -> Bool { + if !isLoading { + cancel() + } else if stage == .idle { + stage = .waiting + after(gracePeriod) { [weak self] in self?.reveal() } + } + return isVisible + } + + private func reveal() { + guard stage == .waiting else { return } + stage = .showing + after(minimumVisibleTime) { [weak self] in self?.settle() } + onChange() + } + + private func settle() { + guard stage == .showing else { return } + stage = .settled + cancelPending = nil + onChange() + } + + private func after(_ delay: TimeInterval, _ action: @escaping () -> Void) { + cancelPending?() + cancelPending = schedule(delay, action) + } + + private func cancel() { + cancelPending?() + cancelPending = nil + stage = .idle + } + + // A dispatch deadline, unlike a default-mode timer, still fires while a + // scroll view is tracking a finger. + static let onMainQueue: Schedule = { delay, action in + let work = DispatchWorkItem(block: action) + DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: work) + return work.cancel + } +} diff --git a/Inspector/InspectorUnavailableView.swift b/Inspector/InspectorUnavailableView.swift index 2784ed3..10fa464 100644 --- a/Inspector/InspectorUnavailableView.swift +++ b/Inspector/InspectorUnavailableView.swift @@ -116,6 +116,13 @@ extension UITableView { action: @escaping () -> Void = {} ) { if (backgroundView as? InspectorUnavailableView)?.content == content { return } - backgroundView = content.map { InspectorUnavailableView($0, action: action) } + let view = content.map { InspectorUnavailableView($0, action: action) } + backgroundView = view + // A loading state that turns up on a screen already showing (see + // DelayedLoadingIndicator) changes nothing VoiceOver was told about, + // so it is pointed out; one there from the start is read with the rest. + if case .loading = content, window != nil { + UIAccessibility.post(notification: .layoutChanged, argument: view) + } } } diff --git a/Inspector/ProcessDetailListViewController.swift b/Inspector/ProcessDetailListViewController.swift index c7191ed..f870ecc 100644 --- a/Inspector/ProcessDetailListViewController.swift +++ b/Inspector/ProcessDetailListViewController.swift @@ -16,6 +16,9 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu private var rowsByID: [String: (cells: [String], inspection: DetailRowInspection)] = [:] private var dataSource: UITableViewDiffableDataSource! private var failure: String? + // Details that come back quickly show a blank screen, not a flash of a + // spinner; details that take a while show one long enough to be read. + private let loadingIndicator = DelayedLoadingIndicator() private var searchText = "" private var sort: ProcessDetailSort { didSet { @@ -123,6 +126,9 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu cell.accessibilityHint = String(localized: "Shows the full record") return cell } + loadingIndicator.onChange = { [weak self] in + self?.render() + } render() Task { await load() } } @@ -137,6 +143,10 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu // MARK: Rendering private func render() { + // Rows that arrive just after the spinner appeared wait out its + // minimum time rather than blinking it away; a message never waits. + if loadingIndicator.holdsContent, !rows.isEmpty { return } + let showsLoading = loadingIndicator.update(isLoading: failure == nil && detail == nil) shareButton.isEnabled = detail != nil // An overlay owns the whole screen, so nothing is listed under one. var snapshot = NSDiffableDataSourceSnapshot() @@ -189,7 +199,7 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu tableView.setUnavailableContent(nil) } } else { - tableView.setUnavailableContent(.loading(nil)) + tableView.setUnavailableContent(showsLoading ? .loading(nil) : nil) } } diff --git a/Inspector/ProcessListViewController.swift b/Inspector/ProcessListViewController.swift index 6a61345..370991d 100644 --- a/Inspector/ProcessListViewController.swift +++ b/Inspector/ProcessListViewController.swift @@ -20,6 +20,9 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd // across the reloads a live sample causes. private var selectedIdentity: ProcessIdentity? private let creditsView = ProcessListCreditsView() + // A connection that comes up quickly shows a blank list, not a flash of + // "Connecting…"; one that takes a while shows it long enough to be read. + private let connectingIndicator = DelayedLoadingIndicator() init(model: ProcessListModel) { self.model = model @@ -66,6 +69,9 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd observation = model.changes.sink { [weak self] in self?.render() } + connectingIndicator.onChange = { [weak self] in + self?.render() + } render() } @@ -128,6 +134,9 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd // MARK: Rendering private func render() { + // Rows that arrive just after "Connecting…" appeared wait out its + // minimum time rather than blinking it away; a message never waits. + if connectingIndicator.holdsContent, !model.visibleRows.isEmpty { return } let rows = model.visibleRows let identities = rows.map(\.id) if identities != shownIdentities { @@ -166,10 +175,14 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd } private func renderOverlay() { + // Until the first sample lands there is nothing to list, connected or + // not; the indicator decides whether that is worth saying yet. + let isConnecting = model.rows.isEmpty && (model.phase == .connecting || model.phase == .active) + let showsConnecting = connectingIndicator.update(isLoading: isConnecting) switch model.phase { - case .connecting where model.rows.isEmpty: + case .connecting where model.rows.isEmpty, .active where model.rows.isEmpty: tableView.setUnavailableContent( - .loading(String(localized: "Connecting to the inspector service…")) + showsConnecting ? .loading(String(localized: "Connecting to the inspector service…")) : nil ) case .failed(let message): tableView.setUnavailableContent( diff --git a/Makefile b/Makefile index 104bc58..0000ade 100644 --- a/Makefile +++ b/Makefile @@ -175,6 +175,10 @@ harness: trap 'rm -f "$$harness_bin"' EXIT; \ xcrun --sdk macosx swiftc -swift-version 5 "$(ROOT_DIR)/Inspector/SceneRestorationReset.swift" "$(ROOT_DIR)/Tests/SceneRestorationHarness.swift" -o "$$harness_bin"; \ "$$harness_bin" + @harness_bin="$$(mktemp /tmp/inspector-loading-harness.XXXXXX)"; \ + trap 'rm -f "$$harness_bin"' EXIT; \ + xcrun --sdk macosx swiftc -swift-version 5 "$(ROOT_DIR)/Inspector/DelayedLoadingIndicator.swift" "$(ROOT_DIR)/Tests/DelayedLoadingHarness.swift" -o "$$harness_bin"; \ + "$$harness_bin" build: check harness bump-build XCBUILD_LABEL=build-ios $(XCODEBUILD) \ diff --git a/Tests/DelayedLoadingHarness.swift b/Tests/DelayedLoadingHarness.swift new file mode 100644 index 0000000..33e9c09 --- /dev/null +++ b/Tests/DelayedLoadingHarness.swift @@ -0,0 +1,147 @@ +import Foundation + +// A clock that only moves when told to, so every grace period and minimum +// time can be stepped through exactly, without sleeping. +final class ManualScheduler { + private struct Pending { + let id: Int + let deadline: TimeInterval + let action: () -> Void + } + + private(set) var now: TimeInterval = 0 + private var pending: [Pending] = [] + private var nextID = 0 + + var schedule: DelayedLoadingIndicator.Schedule { + { [unowned self] delay, action in + nextID += 1 + let id = nextID + pending.append(Pending(id: id, deadline: now + delay, action: action)) + return { [weak self] in self?.pending.removeAll { $0.id == id } } + } + } + + var pendingCount: Int { pending.count } + + func advance(by interval: TimeInterval) { + let end = now + interval + while let next = pending.filter({ $0.deadline <= end }).min(by: { $0.deadline < $1.deadline }) { + pending.removeAll { $0.id == next.id } + now = next.deadline + next.action() + } + now = end + } +} + +@main +enum DelayedLoadingHarness { + static func main() { + let grace: TimeInterval = 0.75 + let minimum: TimeInterval = 0.5 + + func make() -> (DelayedLoadingIndicator, ManualScheduler, () -> Int) { + let clock = ManualScheduler() + let indicator = DelayedLoadingIndicator( + gracePeriod: grace, + minimumVisibleTime: minimum, + schedule: clock.schedule + ) + var changes = 0 + indicator.onChange = { changes += 1 } + return (indicator, clock, { changes }) + } + + // Loading that ends inside the grace period never shows anything, + // and leaves nothing scheduled behind it. + do { + let (indicator, clock, changes) = make() + precondition(!indicator.update(isLoading: true)) + clock.advance(by: grace - 0.1) + precondition(!indicator.update(isLoading: true)) + precondition(!indicator.update(isLoading: false)) + precondition(clock.pendingCount == 0) + clock.advance(by: 5) + precondition(changes() == 0) + precondition(!indicator.isVisible && !indicator.holdsContent) + } + + // Reporting loading again doesn't restart the grace period: the + // indicator is due a grace period after loading began. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace / 2) + indicator.update(isLoading: true) + clock.advance(by: grace / 2) + precondition(changes() == 1) + precondition(indicator.update(isLoading: true)) + } + + // Loading that outlasts the grace period shows the indicator, which + // holds content back for its minimum time and then lets it through. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace) + precondition(changes() == 1) + precondition(indicator.isVisible && indicator.holdsContent) + clock.advance(by: minimum - 0.1) + precondition(indicator.holdsContent) + clock.advance(by: 0.1) + precondition(changes() == 2) + precondition(indicator.isVisible && !indicator.holdsContent) + // Loading that runs on keeps it, without another change. + clock.advance(by: 10) + precondition(indicator.update(isLoading: true)) + precondition(changes() == 2) + precondition(!indicator.update(isLoading: false)) + precondition(clock.pendingCount == 0) + } + + // A failure doesn't wait: ending the loading while the indicator is + // still inside its minimum removes it at once, with no late change. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace + 0.1) + precondition(indicator.holdsContent) + precondition(!indicator.update(isLoading: false)) + precondition(!indicator.isVisible && !indicator.holdsContent) + precondition(clock.pendingCount == 0) + clock.advance(by: 5) + precondition(changes() == 1) + } + + // Trying again after that starts over from a fresh grace period. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace + minimum) + indicator.update(isLoading: false) + precondition(!indicator.update(isLoading: true)) + clock.advance(by: grace - 0.1) + precondition(!indicator.isVisible) + clock.advance(by: 0.1) + precondition(indicator.isVisible && indicator.holdsContent) + precondition(changes() == 3) + } + + // An indicator that goes away takes its pending work with it. + do { + let clock = ManualScheduler() + var indicator: DelayedLoadingIndicator? = DelayedLoadingIndicator( + gracePeriod: grace, + minimumVisibleTime: minimum, + schedule: clock.schedule + ) + indicator?.update(isLoading: true) + precondition(clock.pendingCount == 1) + indicator = nil + precondition(clock.pendingCount == 0) + } + + print("Inspector delayed-loading harness passed") + } +}