diff --git a/Docs/Development.md b/Docs/Development.md index 51d0ec2..6761d14 100644 --- a/Docs/Development.md +++ b/Docs/Development.md @@ -152,11 +152,15 @@ Sources/Workspace/ Support/Shell.swift runs git/gh/bkt through the login shell Support/GitDirectoryWatcher.swift git run outside the app: watches `.git` for a checkout Support/WorkingTreeWatcher.swift files written outside the app: FSEvents over the whole tree + Support/GitChangeColors.swift what added, changed and gone look like — tree, gutter and change list Support/BrandMark.swift real logos (TypeScript, Docker, GitHub…) from SVG Support/BrandPath.swift the path data behind them — generated, don't edit Support/AuthorAvatar.swift author pictures: where to find one, cache, disc view Support/WindowKeyMonitor.swift keys the terminal would otherwise eat (⌃⇥, ⎋) Support/EscapeKey.swift ⎋ = close, and who keeps ⎋ for themselves + Support/InitialFocus.swift a window that opens with the keyboard in nobody's field + Support/TerminalFocus.swift the keys into a conversation you click, and out of one you leave + Support/ViewerMotion.swift how long a screen takes to arrive, in one place Support/PullRequestReference.swift finding a `#123` written in a commit message Support/FileOperations.swift what the Files tab does to disk: create, copy, move, rename, trash Support/MarkdownPDF.swift the preview as a PDF: measure, render, cut into sheets @@ -178,7 +182,7 @@ Sources/Workspace/ PullRequestReviewer.swift who is reviewing, their verdicts, and asking more people PullRequestBuild.swift the CI runs on the head commit, normalised across hosts GitHubAccounts.swift per-repo gh account choice + GH_TOKEN injection - GitStatus.swift porcelain status + per-file diff + GitStatus.swift porcelain status + per-file diff, and the verdict a row is coloured by DefaultBranch.swift which branch the host calls default: gh/bkt, then origin/HEAD RepositoryCommit.swift the branch's own git log, grouped by day DiffModel.swift unified diff → side-by-side rows diff --git a/README.md b/README.md index fcf4d09..36351b0 100644 --- a/README.md +++ b/README.md @@ -89,12 +89,14 @@ missing ones, so you never have to work out which command it wanted. | --- | --- | | The board | Open PRs as a table: title, age, activity, reviewers, CI — one row each. | | Read it all | Details, Diff and Commits tabs. Markdown, images and mentions rendered properly. | -| Threads | Replies nested under what they answer, every author in a colour of their own. | -| Write back | Comment, reply, edit the description, approve, request changes, merge, reject. | +| Threads | Replies nested under what they answer, every author in a colour of their own. A resolved one folds to a single row — author, file and first line, under a green **Resolved** chip — in the conversation and in the diff alike; click it to read the rest. | +| Write back | Comment, reply, resolve a thread or open it again, edit the description, approve, request changes, merge, reject. | | Reviewers | See who approved, add more from a searchable list of the people who can be asked. | | `@` anyone | Type `@` in any comment box and pick a name from the repo. | -| Builds beside you | CI runs for the head commit in a side panel, failures first. | +| Builds beside you | CI runs for the head commit in a side panel, failures first, kept current on their own while the panel is in front of you. | | Update from main | One button when the branch has fallen behind. | +| Read it again | One button in the summary bar re-reads the whole request from the host — title, state, diff, conversation, reviewers and builds. | +| Off to the host | The button beside the title opens the pull request in your browser. | ### Code and diffs @@ -112,11 +114,12 @@ missing ones, so you never have to work out which command it wanted. | Feature | What it does | | --- | --- | | Real editing | Tree-sitter colours, folding, multiple cursors, find and replace (⌘F), auto-indent. | -| Change markers | What has changed since HEAD, down the edge of the gutter — green where lines were added, blue where they were changed, and a wedge where they were deleted. | +| Change markers | What has changed since HEAD, down the edge of the gutter — green where lines were added, yellow where they were changed, and a red wedge where they were deleted. | | Language servers | Diagnostics, completions (⌃Space) and ⌘-click to definition for 20+ languages. | | Go to file (⌘P) | `item controller` finds `app/controllers/item_controller.rb`. Punctuation optional. | | Search in files | ripgrep, grouped by file, every hit marked in the file you open. | | A file tree you can use | Drag in and out, copy and paste with Finder, rename in place, duplicate, multi-select, new file or folder. | +| Changes in the tree | A new file is green, a changed one yellow, a conflict red — same colours as the gutter beside it. Folders wear a dot for whatever is inside them. | ### Previews diff --git a/Sources/Workspace/Editor/GutterDiffMarkers.swift b/Sources/Workspace/Editor/GutterDiffMarkers.swift index 2ae9a13..5032007 100644 --- a/Sources/Workspace/Editor/GutterDiffMarkers.swift +++ b/Sources/Workspace/Editor/GutterDiffMarkers.swift @@ -1,10 +1,9 @@ import AppKit import CodeEditSourceEditor import CodeEditTextView -import SwiftUI /// VS Code's coloured stripe down the line numbers: green where lines were added, -/// blue where they were changed, and a wedge where lines were deleted. +/// yellow where they were changed, and a red wedge where lines were deleted. /// /// **The gutter is the package's, and it offers nothing to decorate it with.** /// `TextViewController.gutterView` is internal, `GutterViewDelegate` has one @@ -17,9 +16,10 @@ import SwiftUI /// What makes it hold together rather than drift: /// /// - **The gutter is findable without the private property.** `GutterView` is a -/// `public` class and `loadView` attaches it with `addFloatingSubview`, so it is -/// a direct subview of the `public` scroll view and can be picked out by type. -/// That is a type check, not a name or an index, and it fails to nothing. +/// `public` class hanging somewhere under the `public` scroll view, so it can be +/// picked out by type — see ``gutterView(in:)`` for where AppKit actually keeps +/// it, which is not where `loadView` appears to put it. That is a type check, +/// not a name or an index, and it fails to nothing. /// - **Its coordinates are the document's.** The gutter is as tall as the whole /// file and the package keeps `origin.y` at `textView.frame.origin.y`, so a /// y inside the gutter *is* a y in the text. Which is why `GutterView` fills its @@ -113,7 +113,7 @@ final class GutterDiffMarkers: TextViewCoordinator, @unchecked Sendable { @MainActor private func install(in controller: TextViewController) { guard let scrollView = controller.scrollView, - let gutter = scrollView.subviews.lazy.compactMap({ $0 as? GutterView }).first + let gutter = Self.gutterView(in: scrollView) else { return } self.gutter = gutter @@ -149,6 +149,36 @@ final class GutterDiffMarkers: TextViewCoordinator, @unchecked Sendable { } } + /// The gutter, wherever AppKit has put it. + /// + /// **Not `scrollView.subviews`, which is where it looks like it should be.** + /// `loadView` attaches it with `addFloatingSubview`, and that does not add + /// the view to the scroll view: AppKit lifts every floating subview into a + /// private `_NSScrollViewFloatingSubviewsContainerView` of its own and adds + /// *that*. So a scan of the direct subviews finds an `NSClipView` and a + /// container, no `GutterView`, and this whole coordinator quietly installed + /// nothing — which is exactly how the markers came to be written, shipped, + /// and never once drawn. + /// + /// Searching breadth-first from the scroll view instead, which is immune to + /// where the container is in the order and to it being renamed or dropped in + /// a later macOS. The clip view is skipped because the document is the text, + /// and the gutter is never inside it — that is the one subtree worth being + /// large. + @MainActor + private static func gutterView(in scrollView: NSScrollView) -> GutterView? { + var queue = scrollView.subviews + var index = 0 + while index < queue.count { + let view = queue[index] + index += 1 + if let gutter = view as? GutterView { return gutter } + if view is NSClipView { continue } + queue.append(contentsOf: view.subviews) + } + return nil + } + /// Set outright rather than left to an autoresizing mask. The gutter's frame /// is assigned in several places for several reasons — width for the line /// count, height for the document, origin for the content insets — and taking @@ -245,46 +275,15 @@ private final class GutterDiffMarkerView: NSView { } } + /// ``GitChangeColors``, the same three the file tree paints a row in. A + /// stripe beside line 40 and the colour on that file's name in the tree are + /// one claim about one repository, so they are one colour — read the tree, + /// open the file, and the marks carry on saying the same thing. private func color(for change: GitLineChange) -> NSColor { switch change { - case .added: GutterDiffColors.added - case .modified: GutterDiffColors.modified - case .deleted: GutterDiffColors.deleted + case .added: GitChangeColors.addedNS + case .modified: GitChangeColors.modifiedNS + case .deleted: GitChangeColors.deletedNS } } } - -/// The three stripe colours. -/// -/// Added and deleted are the diff viewer's own hues — ``DiffColors`` — rather than -/// a second green and a second red, so the two places in the app that say "this -/// line changed" say it in the same colour. They are lifted out of the dark end of -/// their range on the way, because those values are backgrounds for a whole row of -/// text and a three-point stripe on a dark gutter needs to be the thing that is -/// seen rather than the thing behind it. -/// -/// Modified has no such ancestor: the diff viewer never draws a changed line as -/// one thing, it draws the removal beside the addition, so there is no third -/// colour to borrow. Blue is named outright here, as it is in every editor with -/// this feature. -private enum GutterDiffColors { - static let added = stripe(from: DiffColors.addedWord) - static let deleted = stripe(from: DiffColors.removedWord) - static let modified = NSColor(rgb: 0x2E_7A_C6) - - /// Same hue, taken up to the saturation and brightness a thin mark needs. - private static func stripe(from color: Color) -> NSColor { - guard let source = NSColor(color).usingColorSpace(.sRGB) else { return NSColor(color) } - var hue: CGFloat = 0 - var saturation: CGFloat = 0 - var brightness: CGFloat = 0 - var alpha: CGFloat = 0 - source.getHue(&hue, saturation: &saturation, brightness: &brightness, alpha: &alpha) - return NSColor( - hue: hue, - saturation: max(saturation, 0.6), - brightness: max(brightness, 0.62), - alpha: 1 - ) - } -} diff --git a/Sources/Workspace/Models/GitStatus.swift b/Sources/Workspace/Models/GitStatus.swift index e19072e..2d181f0 100644 --- a/Sources/Workspace/Models/GitStatus.swift +++ b/Sources/Workspace/Models/GitStatus.swift @@ -1,5 +1,48 @@ import Foundation +/// What happened to one path, reduced to the handful of verdicts everything +/// outside the change list draws from: the file tree colours a row by this, and +/// the letter the row wears comes from it. +/// +/// **The order is the point.** A folder in the tree wears the strongest verdict +/// of anything inside it, and `Comparable` — synthesised from the order these +/// are declared in — is how "strongest" is decided. A folder holding one +/// conflict and twenty edits is a folder with a conflict in it. +enum GitChangeKind: Sendable, Hashable, Comparable { + /// Staged as new, or not in git at all yet. One idea at two stages, and the + /// tree has no room to say which — the change list beside it does. + case added + case renamed + case modified + case deleted + case conflicted + + /// The one letter the tree puts at the end of a file's row, the way every + /// editor with this feature does. A conflict is the exception: it is the + /// one that wants noticing rather than reading. + var letter: String { + switch self { + case .added: "A" + case .renamed: "R" + case .modified: "M" + case .deleted: "D" + case .conflicted: "!" + } + } + + /// What the row's tooltip says. Plain words, since the letter is the short + /// form and this is the long one. + var label: String { + switch self { + case .added: "Added" + case .renamed: "Renamed" + case .modified: "Modified" + case .deleted: "Deleted" + case .conflicted: "Conflicted" + } + } +} + /// A snapshot of `git status` for a repository. struct GitStatus: Sendable, Hashable { struct Change: Sendable, Hashable, Identifiable { @@ -67,6 +110,23 @@ struct GitStatus: Sendable, Hashable { } } + /// The same verdict as ``label``, in the form the tree and the gutter + /// colour from. Read off the porcelain letter rather than off `label`, + /// so a wording change up there cannot quietly repaint the tree. + /// + /// A copy counts as an addition: what the tree is showing is a file + /// that was not there before. A type change counts as a modification, + /// which is what it is — the path stayed, its contents did not. + var kind: GitChangeKind { + if isConflicted { return .conflicted } + switch statusLetter { + case "A", "?", "C": return .added + case "R": return .renamed + case "D": return .deleted + default: return .modified + } + } + var symbol: String { if isConflicted { return "exclamationmark.triangle.fill" } switch label { diff --git a/Sources/Workspace/Models/Project.swift b/Sources/Workspace/Models/Project.swift index 4ebb6db..0c0e2f4 100644 --- a/Sources/Workspace/Models/Project.swift +++ b/Sources/Workspace/Models/Project.swift @@ -154,6 +154,108 @@ final class Project: Identifiable { return false } + // MARK: - What git says about one row of the tree + + /// Where the repository actually starts, which is not always the folder that + /// was added — see ``GitStatus/root(of:)``. Every path `git status` reports + /// is relative to it, so it is what turns one of those back into a file on + /// disk. Read once and kept: a checkout does not move. + @ObservationIgnored private var gitRoot: URL? + + /// Absolute path → verdict, for every changed file **and every folder above + /// one**, so a change deep in a collapsed tree still shows on the folder the + /// user can see. Built from the whole status in one pass rather than + /// searched per row: the tree asks this once per visible row on every redraw, + /// and a repository mid-rebase has hundreds of changes. + @ObservationIgnored private var changeKindsCache: [String: GitChangeKind] = [:] + /// Which ``gitRevision`` the cache above was built from. `-1` is "never". + @ObservationIgnored private var changeKindsRevision = -1 + + /// What git says about one file or folder, or nil when it says nothing. + /// + /// Reads ``gitRevision`` first and on purpose: that is what makes a SwiftUI + /// row calling this observe the next status read, and it is also what tells + /// the cache it has gone stale. A commit moves every verdict in the tree + /// without touching a byte of any file, so watching the files is not enough. + func changeKind(for fileURL: URL) -> GitChangeKind? { + let revision = gitRevision + if changeKindsRevision != revision { + changeKindsCache = Self.changeKinds(in: gitStatus, folder: url, gitRoot: gitRoot) + changeKindsRevision = revision + } + return changeKindsCache[fileURL.standardizedFileURL.path] + } + + /// One status → the map above. + /// + /// Each change is written against its own path and then walked up, keeping + /// the strongest verdict at every level. The walk stops as soon as it meets + /// an ancestor that already holds something at least as strong, because + /// everything above that one does too — which is what keeps this linear in + /// practice rather than changes × depth. + /// + /// A rename contributes both of its names: the old one so a folder that only + /// lost a file still says so, the new one so the file now on disk does. + /// + /// **Keys are built from the added folder, not from the repository root**, + /// even though the paths git reports are relative to the root. The tree's + /// rows are URLs made by walking down from `folder`, and a dictionary + /// lookup is an exact string match — so a checkout reached through a + /// symlink (`/tmp/…` against git's `/private/tmp/…`) would agree about the + /// file and match on nothing at all. Resolving each row instead would be a + /// filesystem call per visible row per redraw. This resolves twice, here, + /// and only to work out how deep the opened folder sits. + private static func changeKinds( + in status: GitStatus?, + folder: URL, + gitRoot: URL? + ) -> [String: GitChangeKind] { + guard let status, !status.changes.isEmpty else { return [:] } + let base = folder.standardizedFileURL + let basePath = base.path + let prefix = subpath(of: base, under: gitRoot) + var result: [String: GitChangeKind] = [:] + + for change in status.changes { + let kind = change.kind + for path in change.gitPaths { + // A change elsewhere in the repository — a sibling package of + // the monorepo — has no row here to colour. + guard path.hasPrefix(prefix) else { continue } + let relative = String(path.dropFirst(prefix.count)) + guard !relative.isEmpty else { continue } + + var current = base.appendingPathComponent(relative).standardizedFileURL + // Strictly inside the folder: it is not a row in the tree + // itself, and this is also what stops the walk. + while current.path.count > basePath.count { + if let existing = result[current.path], existing >= kind { break } + result[current.path] = kind + current = current.deletingLastPathComponent() + } + } + } + return result + } + + /// How deep the added folder sits inside its repository, as the path prefix + /// git puts in front of everything under it — `"apps/web/"`, or `""` when + /// the folder *is* the repository, which is the ordinary case. + private static func subpath(of folder: URL, under gitRoot: URL?) -> String { + guard let gitRoot else { return "" } + let rootPath = gitRoot.standardizedFileURL.resolvingSymlinksInPath().path + let folderPath = folder.resolvingSymlinksInPath().path + guard folderPath.hasPrefix(rootPath), folderPath.count > rootPath.count else { return "" } + let suffix = folderPath.dropFirst(rootPath.count).drop { $0 == "/" } + return suffix.isEmpty ? "" : suffix + "/" + } + + /// Asks git for the top level, unless it already answered. + private func resolveGitRootIfNeeded() async { + guard gitRoot == nil else { return } + gitRoot = await GitStatus.root(of: url) + } + init(url: URL) { self.url = url self.root = FileNode(url: url, isDirectory: true) @@ -172,8 +274,13 @@ final class Project: Identifiable { async let remoteTask = RemoteInfo.load(for: url) async let statusTask = GitStatus.load(for: url) async let commitTask = RepositoryCommit.load(in: url, limit: commitLimit) + // Alongside the rest rather than before them: it is one process, it is + // only ever run once, and nothing here needs its answer until the + // revision below moves and the tree asks what changed. + async let rootTask: Void = resolveGitRootIfNeeded() remote = await remoteTask gitStatus = await statusTask + await rootTask gitRevision += 1 apply(await commitTask) startWatchingGitIfNeeded() @@ -184,7 +291,9 @@ final class Project: Identifiable { } func refreshGitStatus() async { + async let rootTask: Void = resolveGitRootIfNeeded() gitStatus = await GitStatus.load(for: url) + await rootTask gitRevision += 1 await refreshCommits() startWatchingGitIfNeeded() diff --git a/Sources/Workspace/Models/PullRequestComment.swift b/Sources/Workspace/Models/PullRequestComment.swift index 09931af..a5c1f4c 100644 --- a/Sources/Workspace/Models/PullRequestComment.swift +++ b/Sources/Workspace/Models/PullRequestComment.swift @@ -44,9 +44,26 @@ struct PullRequestComment: Identifiable, Sendable, Hashable { /// The host's own identifier for this comment, set only when the host can /// attach a reply to it. `nil` means "reply is not possible here". var replyToken: String? + /// Whether the thread this comment belongs to has been marked resolved. + /// + /// Resolution belongs to the thread rather than to any one comment, and + /// every host records it on the thread's first comment, so the flag is left + /// where the host put it and read back off the root — see + /// `PullRequestCommentNode.isResolved`. Copying it onto every reply would + /// mean the tree builder had to know what a thread is, which it does not. + var isResolved: Bool = false + /// Who resolved it, when the host says. + var resolvedBy: String? + /// The handle the host wants when the thread is resolved or opened again. + /// GitHub resolves a review *thread* by its GraphQL node id, Bitbucket the + /// comment the thread hangs off; `nil` means this thread cannot be resolved + /// from here and the button is not drawn. + var resolveToken: String? var canReply: Bool { replyToken != nil } + var canResolve: Bool { resolveToken != nil } + /// Where in the diff this comment belongs, when it belongs anywhere. var anchor: DiffLineAnchor? { guard let path, let line else { return nil } @@ -63,10 +80,37 @@ struct DiffLineAnchor: Sendable, Hashable { /// One comment with its replies, ready to render. struct PullRequestCommentNode: Identifiable, Sendable, Hashable { - var comment: PullRequestComment - var replies: [PullRequestCommentNode] = [] + let comment: PullRequestComment + let replies: [PullRequestCommentNode] + + /// Everything said under this comment, however deep it is nested. + /// + /// Counted as the tree is built rather than each time it is read: the + /// number rides in a bubble's header and in every resolved thread's row, + /// both of which are drawn again for reasons that have nothing to do with + /// the conversation, and walking the whole subtree on each of those passes + /// is a cost that grows with the review. + let totalReplies: Int + + init(comment: PullRequestComment, replies: [PullRequestCommentNode] = []) { + self.comment = comment + self.replies = replies + totalReplies = replies.reduce(replies.count) { $0 + $1.totalReplies } + } var id: String { comment.id } + + /// Whether the whole thread is settled. The root carries the host's answer, + /// so a reply never has to be consulted. + var isResolved: Bool { comment.isResolved } + + /// The first line with anything in it, for a thread shown as one row. + var preview: String { + comment.body + .split(separator: "\n", omittingEmptySubsequences: false) + .map { $0.trimmingCharacters(in: .whitespaces) } + .first { !$0.isEmpty } ?? "" + } } extension PullRequestComment { @@ -113,6 +157,20 @@ extension PullRequestComment { return nodes } + + /// The thread roots that hang off a line of the diff, keyed by that line. + /// A thread whose line is no longer in the diff simply does not appear; it + /// is still listed in the conversation, so nothing is lost. + static func inlineThreads( + in nodes: [PullRequestCommentNode] + ) -> [DiffLineAnchor: [PullRequestCommentNode]] { + var grouped: [DiffLineAnchor: [PullRequestCommentNode]] = [:] + for node in nodes { + guard let anchor = node.comment.anchor else { continue } + grouped[anchor, default: []].append(node) + } + return grouped + } } extension PullRequestService { @@ -202,11 +260,112 @@ extension PullRequestService { ) } - items.append(contentsOf: await gitHubReviewComments(for: pr, in: directory)) + var inline = await gitHubReviewComments(for: pr, in: directory) + await applyGitHubThreads(to: &inline, for: pr, in: directory) + items.append(contentsOf: inline) return items.sorted { ($0.createdAt ?? .distantPast) < ($1.createdAt ?? .distantPast) } } + /// Marks the review comments whose thread has been resolved, and records the + /// handle each thread is resolved by. + /// + /// The REST endpoint the comments themselves come from says nothing about + /// resolution — it exists only in GraphQL, and only per thread — so this is + /// a second call over the same comments. It is allowed to fail: a token + /// without the scope, an enterprise host on an older schema, or no `gh` at + /// all leaves every thread looking open, which is what the app showed + /// before and still reads correctly. + private static func applyGitHubThreads( + to comments: inout [PullRequestComment], + for pr: PullRequest, + in directory: URL + ) async { + guard !comments.isEmpty, + let repository = await gitHubRepository(for: pr, in: directory) + else { return } + + // `gh api graphql` does not expand {owner}/{repo} the way `gh api` does, + // so the names are passed as variables. + let query = "query($owner:String!,$repo:String!,$number:Int!)" + + "{repository(owner:$owner,name:$repo)" + + "{pullRequest(number:$number){reviewThreads(first:100){nodes" + + "{id isResolved comments(first:100){nodes{databaseId}}}}}}}" + let result = await GitHubCLI.run( + ["api", "graphql", + "-f", "query=\(query)", + "-f", "owner=\(repository.owner)", + "-f", "repo=\(repository.name)", + "-F", "number=\(pr.number)"], + in: directory, + timeout: 60 + ) + guard result.isSuccess else { return } + + struct Response: Decodable { + struct Root: Decodable { let repository: Repository? } + struct Repository: Decodable { let pullRequest: PullRequestNode? } + struct PullRequestNode: Decodable { let reviewThreads: Threads? } + struct Threads: Decodable { let nodes: [Thread]? } + struct Thread: Decodable { + struct Comments: Decodable { let nodes: [Comment]? } + struct Comment: Decodable { let databaseId: Int? } + let id: String? + let isResolved: Bool? + let comments: Comments? + } + let data: Root? + } + + guard let response = try? JSONDecoder().decode(Response.self, from: Data(result.stdout.utf8)), + let threads = response.data?.repository?.pullRequest?.reviewThreads?.nodes + else { return } + + var byCommentID: [String: (token: String, isResolved: Bool)] = [:] + for thread in threads { + guard let token = thread.id else { continue } + for comment in thread.comments?.nodes ?? [] { + guard let databaseId = comment.databaseId else { continue } + byCommentID["review-comment-\(databaseId)"] = (token, thread.isResolved ?? false) + } + } + + for index in comments.indices { + guard let thread = byCommentID[comments[index].id] else { continue } + comments[index].resolveToken = thread.token + comments[index].isResolved = thread.isResolved + } + } + + /// The owner and name GraphQL needs, from the pull request when the remote + /// named them and from `gh` itself otherwise. + private static func gitHubRepository( + for pr: PullRequest, + in directory: URL + ) async -> (owner: String, name: String)? { + if !pr.repositoryOwner.isEmpty, !pr.repositorySlug.isEmpty { + return (pr.repositoryOwner, pr.repositorySlug) + } + let result = await GitHubCLI.run( + ["repo", "view", "--json", "owner,name"], + in: directory, + timeout: 30 + ) + guard result.isSuccess else { return nil } + + struct Repository: Decodable { + struct Owner: Decodable { let login: String? } + let owner: Owner? + let name: String? + } + + guard let decoded = try? JSONDecoder().decode(Repository.self, from: Data(result.stdout.utf8)), + let owner = decoded.owner?.login, let name = decoded.name, + !owner.isEmpty, !name.isEmpty + else { return nil } + return (owner, name) + } + /// Inline review comments. `gh pr view` does not expose them, and they are /// the only GitHub comments that actually form threads (`in_reply_to_id`). private static func gitHubReviewComments( @@ -365,6 +524,11 @@ extension PullRequestService { let toLine = inline?["to"] as? Int let fromLine = inline?["from"] as? Int + // Cloud hangs a `resolution` object off the thread's first comment + // and leaves it out entirely while the thread is open, so its mere + // presence is the answer. + let resolution = item["resolution"] as? [String: Any] + return PullRequestComment( id: identifier, parentID: parent.map { "\($0)" }, @@ -376,7 +540,10 @@ extension PullRequestService { path: inline?["path"] as? String, line: toLine ?? fromLine, side: toLine != nil ? .new : .old, - replyToken: identifier + replyToken: identifier, + isResolved: resolution != nil, + resolvedBy: BitbucketUser.name(from: resolution?["user"] as? [String: Any]), + resolveToken: identifier ) } .sorted { ($0.createdAt ?? .distantPast) < ($1.createdAt ?? .distantPast) } @@ -432,6 +599,15 @@ extension PullRequestService { // Cloud-style parenting; Data Center nests instead, handled below. let parent = (dictionary["parent"] as? [String: Any])?["id"] + // Data Center names the thread's standing in `state`; Cloud, which + // can reach this decoder through `bkt pr view`, still says it with + // a `resolution` object. + let resolution = dictionary["resolution"] as? [String: Any] + let isResolved = (dictionary["state"] as? String)?.uppercased() == "RESOLVED" + || resolution != nil + let resolver = (dictionary["resolver"] as? [String: Any]) + ?? (resolution?["user"] as? [String: Any]) + return PullRequestComment( id: identifier, parentID: parent.map { "\($0)" }, @@ -443,7 +619,10 @@ extension PullRequestService { path: path, line: toLine ?? fromLine ?? anchorLine, side: isOldSide ? .old : .new, - replyToken: dictionary["id"].map { "\($0)" } + replyToken: dictionary["id"].map { "\($0)" }, + isResolved: isResolved, + resolvedBy: BitbucketUser.name(from: resolver), + resolveToken: dictionary["id"].map { "\($0)" } ) } @@ -702,6 +881,64 @@ extension PullRequestService { } } + /// Settles a thread, or opens it again. `comment` is the thread's root, the + /// only comment either host records resolution on. + static func setResolved( + _ resolved: Bool, + for comment: PullRequestComment, + on pr: PullRequest, + in directory: URL + ) async throws { + guard let token = comment.resolveToken else { + throw PullRequestError.commandFailed( + "This host does not let that thread be resolved from here." + ) + } + + switch pr.host { + case .github: + // Resolution is a GraphQL-only idea on GitHub, and it names the + // review thread rather than any comment in it — which is why the + // conversation is loaded with that node id already in hand. + let mutation = "mutation($id:ID!){" + + (resolved ? "resolveReviewThread" : "unresolveReviewThread") + + "(input:{threadId:$id}){thread{id isResolved}}}" + let result = await GitHubCLI.run( + ["api", "graphql", "-f", "query=\(mutation)", "-f", "id=\(token)"], + in: directory, + timeout: 60 + ) + guard result.isSuccess else { + throw PullRequestError.commandFailed(result.failureMessage) + } + + case .bitbucket: + // `bkt` has the verb itself, and it knows both flavours: Cloud's + // resolve endpoint and Data Center's comment state are one command + // here. Going to the REST path directly instead is what a 403 was + // answering — Cloud's `/resolve` refuses the credential `bkt api` + // presents, and `bkt` is the thing that knows how to ask properly. + // It wants the thread's own comment, which is the only one this is + // ever called with. + var arguments = ["bkt", "pr", "comments", + resolved ? "resolve" : "reopen", + "\(pr.number)", token] + // The checkout usually says which repository this is, but bkt reads + // that from its own context; naming it keeps a context pointing + // somewhere else from settling the wrong thread. + if !pr.repositorySlug.isEmpty { + arguments += ["--repo", pr.repositorySlug] + } + let result = await Shell.run(arguments, in: directory, timeout: 60) + guard result.isSuccess else { + throw PullRequestError.commandFailed(result.failureMessage) + } + + case .unknown: + throw PullRequestError.unsupportedHost + } + } + static func parseTimestamp(_ string: String) -> Date? { let withFraction = ISO8601DateFormatter() withFraction.formatOptions = [.withInternetDateTime, .withFractionalSeconds] diff --git a/Sources/Workspace/Models/ViewerItem.swift b/Sources/Workspace/Models/ViewerItem.swift index e4a8d65..158f6c6 100644 --- a/Sources/Workspace/Models/ViewerItem.swift +++ b/Sources/Workspace/Models/ViewerItem.swift @@ -89,8 +89,21 @@ final class ViewerItem: Identifiable { terminals.first { $0.id == selectedTerminalID } ?? terminals.last } - // Pull request conversation. - var comments: [PullRequestComment] = [] + // Pull request conversation. Assigned through `setComments(_:)`, which is + // what keeps the two shapes below in step with the flat list. + private(set) var comments: [PullRequestComment] = [] + /// The conversation as threads, nested and sorted. + /// + /// Built once, where the comments land, rather than in the views that draw + /// them: every one of those is a `body`, and a `body` runs again for + /// reasons that have nothing to do with the conversation — a key pressed in + /// the box at the foot of the page, the ten-second build tick, a drag on + /// the side panel's seam. Rebuilding the tree on each of those passes is + /// the same answer bought again, at a price that grows with the review. + private(set) var commentThreads: [PullRequestCommentNode] = [] + /// The same threads, keyed by the line of the diff they hang off — see + /// ``PullRequestComment/inlineThreads(in:)``. + private(set) var inlineCommentThreads: [DiffLineAnchor: [PullRequestCommentNode]] = [:] var isLoadingComments = false var isPostingComment = false var commentError: String? @@ -128,8 +141,12 @@ final class ViewerItem: Identifiable { var isLoadingBuilds = false var buildsError: String? /// Patches already fetched, keyed by hash: a commit does not change, so - /// stepping back and forth through the list costs one load each. - var commitDiffs: [String: Diff] = [:] + /// stepping back and forth through the list costs one load each. Written + /// through ``cacheCommitDiff(_:for:)``, which is what keeps the pile of + /// them from growing for as long as the pull request stays open. + private(set) var commitDiffs: [String: Diff] = [:] + /// The hashes in `commitDiffs`, oldest first — see `cacheCommitDiff`. + private var commitDiffOrder: [String] = [] var isLoadingCommitDiff = false var commitDiffError: String? @@ -138,6 +155,11 @@ final class ViewerItem: Identifiable { var syncState: PullRequestSyncState? var isCheckingSync = false var isRunningPullRequestAction = false + /// Whether everything about the pull request is being read again. It is not + /// `isRunningPullRequestAction`: that one means something is being *changed* + /// on the host and greys out the whole action bar, and a refresh changes + /// nothing. Two flags also keep two spinners from turning at once. + var isRefreshingPullRequest = false var isLoading = false var errorMessage: String? @@ -217,6 +239,36 @@ final class ViewerItem: Identifiable { /// session away. nonisolated var survivesClosing: Bool { isTerminal } + /// Keeps a commit's patch for the next time it is opened, and lets the + /// oldest ones go. + /// + /// A patch here is not a string: it is parsed *and* syntax-coloured, an + /// `AttributedString` for every side of every row. Reading down a hundred + /// commits of a long pull request would otherwise hold every one of them + /// for as long as the tab stays open. A dozen is enough for stepping back + /// and forth, which is what the cache is for. + func cacheCommitDiff(_ diff: Diff, for sha: String) { + if commitDiffs[sha] == nil { commitDiffOrder.append(sha) } + commitDiffs[sha] = diff + while commitDiffOrder.count > 12 { + let oldest = commitDiffOrder.removeFirst() + // Never the one being read: a reader who steps back to it would + // find it gone and pay for it again. + if oldest == selectedCommit?.sha { + commitDiffOrder.append(oldest) + continue + } + commitDiffs[oldest] = nil + } + } + + /// The conversation, and everything drawn from it, in one write. + func setComments(_ comments: [PullRequestComment]) { + self.comments = comments + commentThreads = PullRequestComment.tree(from: comments) + inlineCommentThreads = PullRequestComment.inlineThreads(in: commentThreads) + } + init(kind: Kind, title: String, subtitle: String? = nil) { self.kind = kind self.title = title diff --git a/Sources/Workspace/Models/WorkspaceStore.swift b/Sources/Workspace/Models/WorkspaceStore.swift index 4325c6d..72f6598 100644 --- a/Sources/Workspace/Models/WorkspaceStore.swift +++ b/Sources/Workspace/Models/WorkspaceStore.swift @@ -714,8 +714,20 @@ final class WorkspaceStore { /// Shows or folds the navigator by hand — the button in the viewer's header /// and the View menu. Nothing else moves the pane: where it is left is where /// it stays, whatever the viewer goes on to show. + /// + /// The fold is animated here rather than at the call sites, because there + /// are three of them and they used to disagree: the header button eased, + /// ⌥⌘0 snapped, and a dashboard tile snapped. A pane is one thing whichever + /// way it is asked for, so the transaction belongs with the property. func toggleNavigator() { - showsNavigator.toggle() + withAnimation(ViewerMotion.paneChange) { showsNavigator.toggle() } + } + + /// The same for the repositories pane, which the header button and ⌘0 both + /// reach for. It exists only to carry the animation — without it the two + /// call sites write the flag themselves and one of them forgets. + func toggleProjects() { + withAnimation(ViewerMotion.paneChange) { showsProjects.toggle() } } /// The document the viewer is actually showing. The open item survives a @@ -1109,6 +1121,78 @@ final class WorkspaceStore { } } + /// Reads everything this pull request is made of again, from the host. + /// + /// The same things ``openPullRequest(_:project:)`` fetches the first time + /// round, minus the shortcuts it takes: the request itself — its title, + /// state, draft flag, approvals and description all live on that one object + /// — the diff, the conversation, who is reviewing, the CI runs and how far + /// the branch has drifted. The commits come along only if the tab that + /// shows them has already been opened; a review that never went there + /// should not start paying for the call now. + /// + /// They are independent reads, so they go out together the way the + /// dashboard's do: the wait is the slowest host call rather than the sum of + /// them, and each writes what it owns as it lands. + func refreshPullRequest(_ item: ViewerItem, project: Project, pr: PullRequest) async { + // Not while a merge, a rejection or a sync is on its way: what comes + // back would be the state from before it landed. One refresh at a time, + // too — the button is easy to lean on. + guard !item.isRefreshingPullRequest, !item.isRunningPullRequestAction else { return } + item.isRefreshingPullRequest = true + defer { item.isRefreshingPullRequest = false } + + async let request: Void = reloadPullRequest(item, project: project, pr: pr) + async let diff: Void = loadPullRequestDiff(item, project: project, pr: pr) + async let comments: Void = loadComments(item, project: project, pr: pr) + async let builds: Void = loadBuilds(item, project: project, pr: pr) + async let commits: Void = reloadCommitsIfShown(item, project: project, pr: pr) + // Fetching first, because the branch may well have moved on the host + // since it was last counted here. + async let sync: Void = refreshSyncState(item, project: project, pr: pr, fetching: true) + _ = await (request, diff, comments, builds, commits, sync) + + showStatus("#\(pr.number) refreshed") + } + + /// The pull request object itself, re-read from the host. It is what the + /// summary bar and the header are drawn from, so a refresh that left it + /// alone would reload the diff under a title, a state and an approvals + /// count from whenever the request was opened. + private func reloadPullRequest(_ item: ViewerItem, project: Project, pr: PullRequest) async { + guard let remote = project.remote, remote.kind != .unknown else { return } + var latest = pr + do { + latest = try await PullRequestService.load( + number: pr.number, + for: remote, + in: project.url + ) + item.pullRequest = latest + // The header names the pull request by its title, and a title is + // one of the things that can have changed on the host. + item.title = latest.title + } catch { + // Whatever else lands is still worth showing, so this says what + // went wrong and leaves the rest of the refresh to finish. + showError(error.localizedDescription) + } + // Both hosts hand the reviewers over with the request, so this usually + // costs nothing beyond the call just made. + await loadReviewers(item, project: project, pr: latest) + if let named = await PullRequestService.namedMentions(in: latest, directory: project.url) { + item.pullRequest?.body = named + } + } + + /// The commits, but only for a Commits tab that has already been opened — + /// see ``loadCommits(_:project:pr:)`` for why they are not fetched with + /// everything else in the first place. + private func reloadCommitsIfShown(_ item: ViewerItem, project: Project, pr: PullRequest) async { + guard !item.commits.isEmpty else { return } + await loadCommits(item, project: project, pr: pr) + } + /// Opens a pull request known only by its number — a `#123` written in a /// commit message. The list the navigator holds is of open requests only, /// and a commit usually names one that has already merged, so anything not @@ -1162,9 +1246,9 @@ final class WorkspaceStore { item.isLoadingComments = true item.commentError = nil do { - item.comments = try await PullRequestService.comments(for: pr, in: project.url) + item.setComments(try await PullRequestService.comments(for: pr, in: project.url)) } catch { - item.comments = [] + item.setComments([]) item.commentError = error.localizedDescription } item.isLoadingComments = false @@ -1285,6 +1369,11 @@ final class WorkspaceStore { item.isLoadingCommits = false } + /// The CI runs, read out loud: the spinner turns while it is in flight, and + /// a refusal replaces the list with what the host said. This is the first + /// look at the panel and the reload button — both are somebody waiting for + /// an answer, so both are allowed to say they are working and to fail + /// visibly. func loadBuilds(_ item: ViewerItem, project: Project, pr: PullRequest) async { item.isLoadingBuilds = true item.buildsError = nil @@ -1297,6 +1386,23 @@ final class WorkspaceStore { item.isLoadingBuilds = false } + /// The same runs, read quietly, for the panel's ticker. + /// + /// Nobody asked for this one, so it says nothing: no spinner to flash every + /// ten seconds, and a `gh` or `bkt` that stumbles once leaves the last good + /// list where it is rather than emptying the panel under someone reading it. + /// The list is only assigned when it has actually changed, so a tick that + /// finds the same jobs redraws nothing. + func refreshBuilds(_ item: ViewerItem, project: Project, pr: PullRequest) async { + guard let builds = try? await PullRequestService.builds(for: pr, in: project.url) else { + return + } + if item.builds != builds { item.builds = builds } + // The host is answering again, so whatever the last failure left on + // screen is out of date. + if item.buildsError != nil { item.buildsError = nil } + } + /// Opens one commit inside the Commits tab and fetches its patch. func showCommit( _ commit: PullRequestCommit, @@ -1313,7 +1419,10 @@ final class WorkspaceStore { // The tab may have gone back to the list, or on to another commit, // while this was in flight — the patch is still worth keeping. if let text { - item.commitDiffs[commit.sha] = DiffHighlighter.highlight(await DiffParser.parseInBackground(text)) + item.cacheCommitDiff( + DiffHighlighter.highlight(await DiffParser.parseInBackground(text)), + for: commit.sha + ) } else if item.selectedCommit?.sha == commit.sha { item.commitDiffError = "Could not load the changes in \(commit.shortSHA)." } @@ -1504,6 +1613,36 @@ final class WorkspaceStore { item.isPostingComment = false } + /// Settles a comment thread, or opens it again. The conversation is read + /// back afterwards rather than edited in place: resolution is the host's + /// word, and a thread someone else has already touched should come back + /// saying so. + func setCommentResolved( + _ resolved: Bool, + for comment: PullRequestComment, + on item: ViewerItem, + project: Project, + pr: PullRequest + ) async { + item.isPostingComment = true + item.commentError = nil + do { + try await PullRequestService.setResolved( + resolved, + for: comment, + on: pr, + in: project.url + ) + showStatus(resolved + ? "Thread resolved on #\(pr.number)" + : "Thread reopened on #\(pr.number)") + await loadComments(item, project: project, pr: pr) + } catch { + item.commentError = error.localizedDescription + } + item.isPostingComment = false + } + /// A comment with its mentions in the form the host wants. /// /// The composer writes the **name**, because that is what the person typing @@ -1686,8 +1825,10 @@ final class WorkspaceStore { /// Everything that sends the user there goes through this: setting the tab /// alone does nothing visible while the pane is hidden. func showNavigator(_ tab: NavigatorTab) { + // The tab is set outside the transaction: which list is shown is the + // navigator's own business, and only the pane arriving is a fold. navigatorTab = tab - showsNavigator = true + withAnimation(ViewerMotion.paneChange) { showsNavigator = true } } /// Switching the navigator by hand — the tab bar, or the View menu. @@ -2281,6 +2422,10 @@ final class WorkspaceStore { /// Ends one panel's conversation — the ✕, and nothing else. Being replaced /// on screen is not this: see ``chats``. func closeChatPanel(_ panel: ChatPanel) { + // Same reason as folding, one step further: the surface is about to be + // torn down, and a first responder that has been closed is a window with + // nowhere for the keys to go. + TerminalFocus.relinquish(panel.session) panel.remember() panel.session.terminate() withAnimation(Self.chatPanelMotion) { @@ -2294,7 +2439,7 @@ final class WorkspaceStore { /// **Nothing happens at all when it is already in front**, and that guard /// has to be out here rather than inside ``raise(_:)``. /// - /// `ChatPanelClickMonitor` calls this on *every* left mouse-down in the + /// `WindowClickMonitor` calls this on *every* left mouse-down in the /// window, which is what lets a click anywhere in a panel bring it forward. /// With the check inside `raise`, the mutation was correctly skipped but /// `withAnimation` had already opened a transaction — so every click on the @@ -2315,6 +2460,15 @@ final class WorkspaceStore { /// The same, unfolded: what the Claude list, a banner and a resumed /// conversation all want, since each of them is a request to *read* it. + /// + /// And to type in it, which is why the keyboard comes with it. Every way + /// into this — the bar on the dock, the Claude list, a banner — is somebody + /// asking for a conversation by name, and landing them in it with the cursor + /// still somewhere else would make the next thing they do a click they + /// should not have had to make. + /// + /// The panel's terminal never left the window (see ``chats``), so there is + /// nothing to wait for: the surface is mounted and can take the keys now. func unfoldChatPanel(_ panel: ChatPanel) { withAnimation(Self.chatPanelMotion) { panel.isCollapsed = false @@ -2326,6 +2480,7 @@ final class WorkspaceStore { panel.remember() raise(panel) } + TerminalFocus.give(to: panel.session) } private func raise(_ panel: ChatPanel) { @@ -2365,6 +2520,13 @@ final class WorkspaceStore { /// buy by dropping the movement, and a panel that vanishes rather than /// folding gives the reader nothing to follow to the dock. func collapseChatPanel(_ panel: ChatPanel) { + // Before it goes: the chevron is inside the panel, so the mouse-down + // that reached it has just put the keyboard in this very terminal — and + // the mouse-up folds the terminal away. Left alone, the conversation + // would keep receiving everything typed at a window that no longer shows + // it. Only ever this panel's own focus, so folding one while reading + // another takes nothing from the one being read. + TerminalFocus.relinquish(panel.session) withAnimation(Self.chatPanelMotion) { panel.isCollapsed = true panel.remember() @@ -3120,8 +3282,15 @@ struct ChatPanelFrame: Equatable, Codable { /// The line the strip's bars sit on, in the overlay's coordinates. + /// + /// Flush with the bottom of the pane, and not held off it by ``margin`` the + /// way a floating panel is. A panel is an object sitting *in* the window and + /// wants air around it; the dock is a rail along the bottom edge, and a rail + /// with a gap under it reads as neither one thing nor the other. What the + /// gap cost was the only thing down there worth covering — the last line of + /// the pane — and covering it is what a dock does. static func dockLine(in region: CGRect) -> CGFloat { - max(region.maxY - margin - titleBarHeight, 0) + max(region.maxY - titleBarHeight, 0) } /// Where a panel goes as it folds: down to the right-hand end of the dock, diff --git a/Sources/Workspace/Support/AuthorAvatar.swift b/Sources/Workspace/Support/AuthorAvatar.swift index d64f22c..20b7223 100644 --- a/Sources/Workspace/Support/AuthorAvatar.swift +++ b/Sources/Workspace/Support/AuthorAvatar.swift @@ -194,14 +194,24 @@ struct AuthorAvatar: View { .resizable() .interpolation(.high) .aspectRatio(contentMode: .fill) + .transition(.opacity) } else { Text(initials) .font(.system(size: size * 0.5, weight: .semibold)) .foregroundStyle(tint) .frame(width: size, height: size) .background(tint.opacity(0.22)) + .transition(.opacity) } } + // The initials give way to the face rather than being swapped for it. + // A pull request opened for the first time asks for every picture on it + // at once and they land one at a time, so without this the whole page + // speckles. Opacity and nothing else: fifty discs growing into place is + // a page that cannot be read while it settles, and the two sides are the + // same disc at the same size anyway. It only ever runs on first sight — + // the initialiser primes a face the cache already has. + .animation(ViewerMotion.isReduced ? nil : .easeOut(duration: 0.2), value: image != nil) .frame(width: size, height: size) .clipShape(Circle()) .help(name) diff --git a/Sources/Workspace/Support/GitChangeColors.swift b/Sources/Workspace/Support/GitChangeColors.swift new file mode 100644 index 0000000..f855ead --- /dev/null +++ b/Sources/Workspace/Support/GitChangeColors.swift @@ -0,0 +1,46 @@ +import AppKit +import SwiftUI + +/// The one place the app decides what "added", "changed" and "gone" look like. +/// +/// Three places say it — the file tree colours a row, the editor's gutter draws a +/// stripe beside a line, and the sidebar's change list dots each entry — and they +/// are the same claim about the same repository, so they are the same colours. +/// Before this they were three separate literals and the tree agreed with neither +/// of the others. +/// +/// The values are the ones VS Code settled on rather than the system's `.green` +/// and `.red`, which are tuned for buttons: they are muted enough to sit under a +/// file name for a whole session without shouting, and light enough to carry a +/// three-point stripe on a dark gutter. Fixed rather than theme-derived, for the +/// same reason the syntax themes are checked in — a changed file should look the +/// same on every Mac. +enum GitChangeColors { + static let added = Color(nsColor: addedNS) + static let renamed = Color(nsColor: renamedNS) + static let modified = Color(nsColor: modifiedNS) + static let deleted = Color(nsColor: deletedNS) + static let conflicted = Color(nsColor: conflictedNS) + + /// The gutter is AppKit and draws into a `CGContext`, so it wants these + /// rather than the SwiftUI wrappers above. + static let addedNS = NSColor(rgb: 0x73_C9_91) + static let renamedNS = NSColor(rgb: 0x6C_9E_F8) + static let modifiedNS = NSColor(rgb: 0xE2_C0_8D) + static let deletedNS = NSColor(rgb: 0xC7_4E_39) + static let conflictedNS = NSColor(rgb: 0xE4_67_6B) + + static func color(for kind: GitChangeKind) -> Color { + switch kind { + case .added: added + case .renamed: renamed + case .modified: modified + case .deleted: deleted + case .conflicted: conflicted + } + } +} + +extension GitChangeKind { + var color: Color { GitChangeColors.color(for: self) } +} diff --git a/Sources/Workspace/Support/RemoteImage.swift b/Sources/Workspace/Support/RemoteImage.swift index 0a4d1ee..be96ee3 100644 --- a/Sources/Workspace/Support/RemoteImage.swift +++ b/Sources/Workspace/Support/RemoteImage.swift @@ -122,6 +122,15 @@ struct MarkdownImage: View { var body: some View { content + // The picture comes up through the placeholder it replaces rather + // than taking its place between two frames. A comment with several + // screenshots in it is otherwise read while it is still being hit + // by them. The line of text moves as the picture takes its real + // height, and that is not worth animating away: the reader is at + // the top of a comment that is still arriving, and a height easing + // open under them is worse than one that simply is. + .animation(ViewerMotion.isReduced ? nil : .easeOut(duration: 0.2), value: image != nil) + .animation(ViewerMotion.isReduced ? nil : .easeOut(duration: 0.2), value: isLoading) .task { await load() } } @@ -147,12 +156,14 @@ struct MarkdownImage: View { .onTapGesture { open() } .pointerCursor() .help(url.isFileURL ? "Open this file" : "Open in browser") + .transition(.opacity) } else if isLoading { placeholder { ProgressView().controlSize(.small) Text(caption) .foregroundStyle(.secondary) } + .transition(.opacity) } else { placeholder { Image(systemName: "photo") @@ -173,6 +184,7 @@ struct MarkdownImage: View { .buttonStyle(.link) .pointerCursor() } + .transition(.opacity) } } diff --git a/Sources/Workspace/Support/Skeleton.swift b/Sources/Workspace/Support/Skeleton.swift new file mode 100644 index 0000000..e880a16 --- /dev/null +++ b/Sources/Workspace/Support/Skeleton.swift @@ -0,0 +1,145 @@ +import AppKit +import SwiftUI + +/// The shape of something that has not arrived yet. +/// +/// A spinner says only that the app is busy. A skeleton says what is coming and +/// where it will be, so the page does not jump when it lands — the rows are +/// already the right height and the columns already the right width, and what +/// happens on arrival is text replacing grey rather than a layout being built. +/// +/// **It is for a first load only.** Anything being *re*-read already has its +/// last answer on screen, and that answer is better than a grey box: the builds +/// panel keeps its list through a failed tick for exactly this reason. A +/// skeleton where a reload happens is a screen that throws away what it knows +/// every few seconds. +/// +/// The pulse is one animation for a whole group rather than one per block — +/// see ``SkeletonGroup``. Twenty blocks each running their own repeating +/// animation is twenty timers, and a placeholder must not cost more than the +/// thing it stands in for. +struct Skeleton: View { + /// How wide, as a fraction of what is offered. Rows of text are ragged, and + /// a column of identical bars reads as a table rather than as prose. + var width: CGFloat? + var height: CGFloat = 12 + var cornerRadius: CGFloat = 4 + + @Environment(\.skeletonPulse) private var pulse + + var body: some View { + RoundedRectangle(cornerRadius: cornerRadius, style: .continuous) + .fill(.quaternary) + .opacity(pulse) + .frame(width: width, height: height) + .frame(maxWidth: width == nil ? .infinity : nil, alignment: .leading) + // It stands for content, and content is read. Nothing here is worth + // a stop on the way through with the keyboard. + .accessibilityHidden(true) + } +} + +/// A circle, for where a face goes. +struct SkeletonCircle: View { + var size: CGFloat = 20 + + @Environment(\.skeletonPulse) private var pulse + + var body: some View { + Circle() + .fill(.quaternary) + .opacity(pulse) + .frame(width: size, height: size) + .accessibilityHidden(true) + } +} + +/// Holds the breath every skeleton inside it takes. +/// +/// One repeating animation, published down the tree, rather than one per block. +/// Under Reduce Motion it does not breathe at all — the blocks are still the +/// shape of what is coming, which is the half of this that carries the meaning. +struct SkeletonGroup: View { + @ViewBuilder var content: Content + + /// SwiftUI's own reading of the setting rather than ``ViewerMotion``'s. + /// Both ask the same system for the same answer, but this one is watched: + /// turning Reduce Motion on with skeletons on screen has to stop a pulse + /// that is already running, and turning it off has to start one. + @Environment(\.accessibilityReduceMotion) private var isReduced + + @State private var isDim = false + + var body: some View { + content + .environment(\.skeletonPulse, isDim ? 0.45 : 0.85) + // The animation is chosen at the moment the value moves, so the + // reduced case is a plain assignment: nothing repeating is left + // running behind the blocks, which is what stopping it means here. + .animation(isReduced ? nil : .easeInOut(duration: 0.9).repeatForever(autoreverses: true), value: isDim) + .onAppear { isDim = !isReduced } + .onChange(of: isReduced) { isDim = !isReduced } + } +} + +private struct SkeletonPulseKey: EnvironmentKey { + /// What a block outside any group is drawn at: still there, still grey, + /// simply not breathing. + static let defaultValue: Double = 0.7 +} + +extension EnvironmentValues { + var skeletonPulse: Double { + get { self[SkeletonPulseKey.self] } + set { self[SkeletonPulseKey.self] = newValue } + } +} + +// MARK: - The shapes this app waits for + +/// A row of the kind the lists in the side panes are made of: a face, a line of +/// text, and a shorter line under it. +struct SkeletonRow: View { + var hasAvatar = true + /// Which of the ragged widths this row takes, so a column of them does not + /// come out as a rectangle. + var seed: Int = 0 + + private static let widths: [CGFloat] = [0.82, 0.64, 0.91, 0.73, 0.58] + + var body: some View { + HStack(spacing: 8) { + if hasAvatar { + SkeletonCircle(size: 20) + } + VStack(alignment: .leading, spacing: 5) { + GeometryReader { geometry in + VStack(alignment: .leading, spacing: 5) { + Skeleton(width: geometry.size.width * Self.widths[seed % Self.widths.count]) + Skeleton(width: geometry.size.width * 0.35, height: 9) + } + } + .frame(height: 26) + } + } + .padding(.horizontal, 8) + .padding(.vertical, 6) + } +} + +/// A column of them, for a list that has not come back yet. +struct SkeletonList: View { + var rows = 5 + var hasAvatar = true + + var body: some View { + SkeletonGroup { + VStack(alignment: .leading, spacing: 4) { + ForEach(0..