From a7491daffc0312f8859811efc5ee8ba02109db5f Mon Sep 17 00:00:00 2001 From: Stefan Hausotte Date: Sun, 14 Jun 2026 21:38:28 +0200 Subject: [PATCH] feat: display and upload image attachments in issues, PRs, and comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `AttachmentGallery` to show image thumbnails and file rows on issue/PR detail views and inline in comment bodies. A `PhotosPicker` button in the markdown toolbar lets users attach images when composing or editing; the selected image is uploaded via the Forgejo assets API and a markdown reference is inserted into the editor. Closes #79 test(ui): add AttachmentUITests — upload image and verify display Adds a combined UI test that uploads a 1×1 PNG to an issue via the markdown toolbar, submits a comment, re-enters the issue, and asserts that the Attachments section and gallery are visible. - AttachmentGallery: add accessibilityIdentifier("attachment-gallery") - MarkdownComponents: add -dev_testImageUpload launch-arg branch that bypasses PhotosPicker and uploads a hardcoded PNG, keeping the test deterministic without requiring photos in the simulator's library test(attachment): fix gallery element query and image detection for Forgejo uploads Move the attachment-gallery accessibility identifier from the VStack container to the horizontal ScrollView, which XCTest reliably exposes as a scroll view element. Update the UI test to use app.scrollViews["attachment-gallery"]. test(attachment): verify multiple image uploads render as distinct thumbnails Upload two images from the comment sheet, assert two markdown references are inserted, and after reload assert both render as thumbnails in the gallery. Confirms the ForEach(imageAttachments) gallery handles multiple attachments. test: add unit tests for attachmentMarkdown image vs link syntax Cover the pure attachmentMarkdown(for:) function directly: images (by MIME type and by extension) produce embed syntax, non-image files (log, pdf) produce link syntax. Faster and more focused than the UI test, which only exercised this indirectly. feat: wrap markdown toolbar icons and add file attachment upload The markdown toolbar used a horizontal scroll view, so trailing icons (link, list, quote, task, attach) were hidden off-screen with no visual cue. Replace it with a wrapping flow layout (MarkdownToolbarFlow) so every icon is always visible, wrapping to a second row on narrow widths. Add general file-attachment upload: the attach button is now a menu offering "Photo Library" (images via PhotosPicker) and "Choose File" (any file via .fileImporter). Picked files are read through a security-scoped resource and uploaded with a MIME type derived from the extension; non-image files insert link markdown via the existing attachmentMarkdown(for:) path. --- Forji/Forji.xcodeproj/project.pbxproj | 21 +- .../xcshareddata/swiftpm/Package.resolved | 9 - Forji/Forji/Helpers/PreviewData.swift | 22 ++ Forji/Forji/Views/AttachmentGallery.swift | 122 ++++++++++ Forji/Forji/Views/CommentSheet.swift | 3 + Forji/Forji/Views/CommentView.swift | 5 + Forji/Forji/Views/IssueDetailView.swift | 20 +- Forji/Forji/Views/IssueEditView.swift | 11 +- Forji/Forji/Views/IssueListView.swift | 3 +- Forji/Forji/Views/MarkdownComponents.swift | 67 +----- Forji/Forji/Views/MarkdownToolbar.swift | 215 ++++++++++++++++++ Forji/Forji/Views/MarkdownToolbarFlow.swift | 47 ++++ .../Forji/Views/MentionableEditorField.swift | 2 + Forji/Forji/Views/MetadataPickers.swift | 5 +- Forji/Forji/Views/PullRequestDetailView.swift | 22 +- Forji/Forji/Views/PullRequestEditView.swift | 6 +- Forji/Forji/Views/PullRequestListView.swift | 3 +- .../ForjiTests/MarkdownComponentsTests.swift | 34 +++ Forji/ForjiUITests/AttachmentUITests.swift | 179 +++++++++++++++ 19 files changed, 707 insertions(+), 89 deletions(-) create mode 100644 Forji/Forji/Views/AttachmentGallery.swift create mode 100644 Forji/Forji/Views/MarkdownToolbar.swift create mode 100644 Forji/Forji/Views/MarkdownToolbarFlow.swift create mode 100644 Forji/ForjiUITests/AttachmentUITests.swift diff --git a/Forji/Forji.xcodeproj/project.pbxproj b/Forji/Forji.xcodeproj/project.pbxproj index c62d148..10be1f4 100644 --- a/Forji/Forji.xcodeproj/project.pbxproj +++ b/Forji/Forji.xcodeproj/project.pbxproj @@ -230,7 +230,7 @@ mainGroup = DEC49F182F3CE05200E7DD54; minimizedProjectReferenceProxies = 1; packageReferences = ( - DE00000000000005000000AA /* XCRemoteSwiftPackageReference "ForgejoKit" */, + DE00000000000005000000AA /* XCLocalSwiftPackageReference "../../ForgejoKit" */, DEC49F6B2F3D00C700E7DD54 /* XCRemoteSwiftPackageReference "textual" */, DEC49F812F3D173F00E7DD54 /* XCRemoteSwiftPackageReference "HighlightSwift" */, ); @@ -633,15 +633,14 @@ }; /* End XCConfigurationList section */ -/* Begin XCRemoteSwiftPackageReference section */ - DE00000000000005000000AA /* XCRemoteSwiftPackageReference "ForgejoKit" */ = { - isa = XCRemoteSwiftPackageReference; - repositoryURL = "https://codeberg.org/secana/ForgejoKit.git"; - requirement = { - kind = exactVersion; - version = 0.7.0; - }; +/* Begin XCLocalSwiftPackageReference section */ + DE00000000000005000000AA /* XCLocalSwiftPackageReference "../../ForgejoKit" */ = { + isa = XCLocalSwiftPackageReference; + relativePath = ../../ForgejoKit; }; +/* End XCLocalSwiftPackageReference section */ + +/* Begin XCRemoteSwiftPackageReference section */ DEC49F6B2F3D00C700E7DD54 /* XCRemoteSwiftPackageReference "textual" */ = { isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/gonzalezreal/textual"; @@ -663,12 +662,12 @@ /* Begin XCSwiftPackageProductDependency section */ DE00000000000002000000AA /* ForgejoKit */ = { isa = XCSwiftPackageProductDependency; - package = DE00000000000005000000AA /* XCRemoteSwiftPackageReference "ForgejoKit" */; + package = DE00000000000005000000AA /* XCLocalSwiftPackageReference "../../ForgejoKit" */; productName = ForgejoKit; }; DE00000000000004000000AA /* ForgejoKit */ = { isa = XCSwiftPackageProductDependency; - package = DE00000000000005000000AA /* XCRemoteSwiftPackageReference "ForgejoKit" */; + package = DE00000000000005000000AA /* XCLocalSwiftPackageReference "../../ForgejoKit" */; productName = ForgejoKit; }; DEC49F6D2F3D023400E7DD54 /* Textual */ = { diff --git a/Forji/Forji.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Forji/Forji.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 321df27..363434d 100644 --- a/Forji/Forji.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Forji/Forji.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,15 +1,6 @@ { "originHash" : "931ec0beeaf4e6a5eaa0afab6f815f97bc126bda7a2c9f001c10de58585e766f", "pins" : [ - { - "identity" : "forgejokit", - "kind" : "remoteSourceControl", - "location" : "https://codeberg.org/secana/ForgejoKit.git", - "state" : { - "revision" : "4d1dfc1305c0194fc74b09d15d29940a98cd9752", - "version" : "0.7.0" - } - }, { "identity" : "highlightswift", "kind" : "remoteSourceControl", diff --git a/Forji/Forji/Helpers/PreviewData.swift b/Forji/Forji/Helpers/PreviewData.swift index 3415ad2..8189091 100644 --- a/Forji/Forji/Helpers/PreviewData.swift +++ b/Forji/Forji/Helpers/PreviewData.swift @@ -115,6 +115,28 @@ import Foundation ) } + extension Attachment { + static let previewImage = Attachment( + id: 1, + uuid: "ee01801f-ffdb-4154-856b-77eab52aaad2", + name: "screenshot.png", + size: 222_208, + browserDownloadUrl: "https://codeberg.org/attachments/ee01801f-ffdb-4154-856b-77eab52aaad2", + type: "image/png", + created: Date(), + ) + + static let previewFile = Attachment( + id: 2, + uuid: "abc12345-0000-0000-0000-000000000000", + name: "report.pdf", + size: 51200, + browserDownloadUrl: "https://codeberg.org/attachments/abc12345-0000-0000-0000-000000000000", + type: "application/pdf", + created: Date(), + ) + } + extension IssueComment { static let preview = IssueComment( id: 1, diff --git a/Forji/Forji/Views/AttachmentGallery.swift b/Forji/Forji/Views/AttachmentGallery.swift new file mode 100644 index 0000000..7ee7121 --- /dev/null +++ b/Forji/Forji/Views/AttachmentGallery.swift @@ -0,0 +1,122 @@ +import ForgejoKit +import SwiftUI + +/// Markdown to insert when an attachment is uploaded. Images use embed syntax +/// (`![name](url)`) so they render inline; everything else (logs, PDFs, archives, +/// …) uses link syntax (`[name](url)`) so it appears as a clickable link rather +/// than a broken inline image. +func attachmentMarkdown(for attachment: Attachment) -> String { + if attachment.isImage { + "![\(attachment.name)](\(attachment.browserDownloadUrl))" + } else { + "[\(attachment.name)](\(attachment.browserDownloadUrl))" + } +} + +struct AttachmentGallery: View { + let attachments: [Attachment] + + private var imageAttachments: [Attachment] { + attachments.filter(\.isImage) + } + + private var fileAttachments: [Attachment] { + attachments.filter { !$0.isImage } + } + + var body: some View { + VStack(alignment: .leading, spacing: 8) { + if !imageAttachments.isEmpty { + ScrollView(.horizontal, showsIndicators: false) { + HStack(spacing: 8) { + ForEach(imageAttachments) { attachment in + imageThumb(attachment) + } + } + } + .accessibilityIdentifier("attachment-gallery") + } + + ForEach(fileAttachments) { attachment in + fileRow(attachment) + } + } + } + + private func imageThumb(_ attachment: Attachment) -> some View { + Group { + if let url = URL(string: attachment.browserDownloadUrl) { + Link(destination: url) { + AsyncImage(url: url) { phase in + switch phase { + case let .success(image): + image + .resizable() + .scaledToFill() + case .failure: + brokenImagePlaceholder + default: + ProgressView() + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + } + .frame(width: 120, height: 90) + .clipped() + .clipShape(RoundedRectangle(cornerRadius: 8)) + } + } + } + } + + private func fileRow(_ attachment: Attachment) -> some View { + Group { + if let url = URL(string: attachment.browserDownloadUrl) { + Link(destination: url) { + HStack(spacing: 8) { + Image(systemName: "paperclip") + .foregroundStyle(.secondary) + Text(attachment.name) + .font(.subheadline) + Spacer() + Text(formatFileSize(attachment.size)) + .font(.caption) + .foregroundStyle(.secondary) + } + } + .tint(.primary) + .accessibilityIdentifier("attachment-file-row") + } + } + } + + private var brokenImagePlaceholder: some View { + Image(systemName: "photo.badge.exclamationmark") + .font(.title2) + .foregroundStyle(.secondary) + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(Color(.tertiarySystemFill)) + } + + private func formatFileSize(_ bytes: Int) -> String { + let kilobytes = Double(bytes) / 1024 + if kilobytes < 1024 { + return String(format: "%.1f KB", kilobytes) + } + return String(format: "%.1f MB", kilobytes / 1024) + } +} + +#if DEBUG + #Preview("Images") { + List { + Section("Attachments") { + AttachmentGallery(attachments: [.previewImage, .previewFile]) + } + } + .listStyle(.insetGrouped) + } + + #Preview("Empty") { + AttachmentGallery(attachments: []) + } +#endif diff --git a/Forji/Forji/Views/CommentSheet.swift b/Forji/Forji/Views/CommentSheet.swift index 5c82f1e..a040a9d 100644 --- a/Forji/Forji/Views/CommentSheet.swift +++ b/Forji/Forji/Views/CommentSheet.swift @@ -3,6 +3,7 @@ import SwiftUI struct CommentSheet: View { let users: [User] + var onUploadImage: ((Data, String, String) async throws -> String)? var onSubmit: (String) async throws -> Void @State private var text = "" @@ -20,6 +21,7 @@ struct CommentSheet: View { text: $text, selectedTab: $selectedTab, showToolbar: true, + onUploadImage: onUploadImage, ) } else { MentionableEditorField( @@ -27,6 +29,7 @@ struct CommentSheet: View { selectedTab: $selectedTab, users: users, showToolbar: true, + onUploadImage: onUploadImage, ) } } diff --git a/Forji/Forji/Views/CommentView.swift b/Forji/Forji/Views/CommentView.swift index 48a3e24..27b818d 100644 --- a/Forji/Forji/Views/CommentView.swift +++ b/Forji/Forji/Views/CommentView.swift @@ -41,6 +41,11 @@ struct CommentView: View { } MarkdownPreview(text: displayBody) + + if let assets = comment.assets, !assets.isEmpty { + AttachmentGallery(attachments: assets) + .padding(.top, 4) + } } .padding(.vertical, 4) .onAppear { diff --git a/Forji/Forji/Views/IssueDetailView.swift b/Forji/Forji/Views/IssueDetailView.swift index 181a45e..b5c905a 100644 --- a/Forji/Forji/Views/IssueDetailView.swift +++ b/Forji/Forji/Views/IssueDetailView.swift @@ -117,6 +117,14 @@ struct IssueDetailView: View { } } + // Attachments + if let assets = issue.assets, !assets.isEmpty { + Section("Attachments") { + AttachmentGallery(attachments: assets) + .padding(.vertical, 4) + } + } + // Comments if !comments.isEmpty { Section("Comments (\(comments.count))") { @@ -187,13 +195,14 @@ struct IssueDetailView: View { repository: repository, issue: issue, authService: authService, + onUploadImage: uploadIssueImage, ) { updatedIssue in self.issue = updatedIssue } } } .sheet(isPresented: $showCommentSheet) { - CommentSheet(users: []) { body in + CommentSheet(users: [], onUploadImage: uploadIssueImage) { body in guard let issueService else { throw URLError(.userAuthenticationRequired) } let comment = try await issueService.createComment( owner: owner, @@ -210,6 +219,15 @@ struct IssueDetailView: View { .errorAlert(message: $errorMessage, isPresented: $showError) } + private func uploadIssueImage(data: Data, fileName: String, mimeType: String) async throws -> String { + guard let issueService else { throw URLError(.userAuthenticationRequired) } + let attachment = try await issueService.uploadIssueAttachment( + owner: owner, repo: repo, index: issueNumber, + fileData: data, fileName: fileName, mimeType: mimeType, + ) + return attachmentMarkdown(for: attachment) + } + private func loadData() async { guard let issueService else { return } isLoading = true diff --git a/Forji/Forji/Views/IssueEditView.swift b/Forji/Forji/Views/IssueEditView.swift index 68b4a19..96dbc22 100644 --- a/Forji/Forji/Views/IssueEditView.swift +++ b/Forji/Forji/Views/IssueEditView.swift @@ -21,14 +21,20 @@ struct IssueEditView: View { private let issueService: IssueService? private let repositoryService: RepositoryService? + private let onUploadImage: ((Data, String, String) async throws -> String)? private let onSaved: (Issue) -> Void - init(repository: Repository, issue: Issue, authService: AuthenticationService, onSaved: @escaping (Issue) -> Void) { + init( + repository: Repository, issue: Issue, authService: AuthenticationService, + onUploadImage: ((Data, String, String) async throws -> String)? = nil, + onSaved: @escaping (Issue) -> Void, + ) { self.repository = repository self.issue = issue self.authService = authService issueService = authService.client.map { IssueService(client: $0) } repositoryService = authService.client.map { RepositoryService(client: $0) } + self.onUploadImage = onUploadImage self.onSaved = onSaved _title = State(initialValue: issue.title) _bodyText = State(initialValue: issue.body ?? "") @@ -54,7 +60,7 @@ struct IssueEditView: View { .accessibilityIdentifier("issue-edit-title-field") } - DescriptionEditorSection(text: $bodyText) + DescriptionEditorSection(text: $bodyText, onUploadImage: onUploadImage) LabelPickerSection( availableLabels: availableLabels, @@ -151,6 +157,7 @@ struct IssueEditView: View { self.authService = authService issueService = nil repositoryService = nil + onUploadImage = nil onSaved = { _ in } _title = State(initialValue: issue.title) _bodyText = State(initialValue: issue.body ?? "") diff --git a/Forji/Forji/Views/IssueListView.swift b/Forji/Forji/Views/IssueListView.swift index de17f0c..c691996 100644 --- a/Forji/Forji/Views/IssueListView.swift +++ b/Forji/Forji/Views/IssueListView.swift @@ -152,7 +152,8 @@ struct IssueListView: View { #if DEBUG init(preview _: Void, repository: Repository, authService: AuthenticationService, issues: [Issue], - notFound: Bool = false) { + notFound: Bool = false) + { self.repository = repository self.authService = authService issueService = nil diff --git a/Forji/Forji/Views/MarkdownComponents.swift b/Forji/Forji/Views/MarkdownComponents.swift index bf61bed..a9bd6c5 100644 --- a/Forji/Forji/Views/MarkdownComponents.swift +++ b/Forji/Forji/Views/MarkdownComponents.swift @@ -1,6 +1,8 @@ import ForgejoKit +import PhotosUI import SwiftUI import Textual +import UniformTypeIdentifiers enum EditPreviewTab: String, CaseIterable { case edit = "Edit" @@ -12,6 +14,9 @@ struct MarkdownEditorField: View { @Binding var selectedTab: EditPreviewTab var minHeight: CGFloat = 150 var showToolbar: Bool = false + /// Called with (data, fileName, mimeType) when the user picks an image. + /// Return the markdown snippet to insert, e.g. `![name](url)`. + var onUploadImage: ((Data, String, String) async throws -> String)? var body: some View { VStack(spacing: 0) { @@ -26,7 +31,7 @@ struct MarkdownEditorField: View { if selectedTab == .edit { VStack(spacing: 0) { if showToolbar { - MarkdownToolbar(text: $text) + MarkdownToolbar(text: $text, onUploadImage: onUploadImage) } TextEditor(text: $text) .frame(minHeight: minHeight, maxHeight: .infinity) @@ -58,66 +63,6 @@ struct MarkdownEditorField: View { } } -private struct MarkdownToolbar: View { - @Binding var text: String - - var body: some View { - ScrollView(.horizontal, showsIndicators: false) { - HStack(spacing: 12) { - toolbarButton("bold", icon: "bold", label: "Bold") { wrap("**") } - toolbarButton("italic", icon: "italic", label: "Italic") { wrap("_") } - toolbarButton("heading", icon: "number", label: "Heading") { prefix("# ") } - toolbarButton("code", icon: "chevron.left.forwardslash.chevron.right", - label: "Inline code") { wrap("`") } - toolbarButton("codeblock", icon: "text.page", label: "Code block") { wrapBlock("```") } - toolbarButton("link", icon: "link", label: "Insert link") { insertLink() } - toolbarButton("list", icon: "list.bullet", label: "Bulleted list") { prefix("- ") } - toolbarButton("quote", icon: "text.quote", label: "Quote") { prefix("> ") } - toolbarButton("task", icon: "checklist", label: "Task list") { prefix("- [ ] ") } - } - .padding(.horizontal, 8) - .padding(.vertical, 6) - } - } - - private func toolbarButton(_ id: String, icon: String, label: String, action: @escaping () -> Void) -> some View { - Button(action: action) { - Image(systemName: icon) - .font(.subheadline) - .frame(width: 28, height: 28) - .contentShape(Rectangle()) - } - .buttonStyle(.plain) - .foregroundStyle(.primary) - .accessibilityLabel(label) - .accessibilityIdentifier("markdown-toolbar-\(id)") - } - - private func wrap(_ marker: String) { - text.append("\(marker)text\(marker)") - } - - private func prefix(_ marker: String) { - if text.isEmpty || text.hasSuffix("\n") { - text.append(marker) - } else { - text.append("\n\(marker)") - } - } - - private func wrapBlock(_ fence: String) { - if text.isEmpty || text.hasSuffix("\n") { - text.append("\(fence)\n\n\(fence)") - } else { - text.append("\n\(fence)\n\n\(fence)") - } - } - - private func insertLink() { - text.append("[title](url)") - } -} - struct MarkdownPreview: View { let text: String var baseURL: URL? diff --git a/Forji/Forji/Views/MarkdownToolbar.swift b/Forji/Forji/Views/MarkdownToolbar.swift new file mode 100644 index 0000000..1bb3e8d --- /dev/null +++ b/Forji/Forji/Views/MarkdownToolbar.swift @@ -0,0 +1,215 @@ +import Foundation +import PhotosUI +import SwiftUI +import UniformTypeIdentifiers + +struct MarkdownToolbar: View { + @Binding var text: String + var onUploadImage: ((Data, String, String) async throws -> String)? + + @State private var photosItem: PhotosPickerItem? + @State private var showPhotosPicker = false + @State private var showFileImporter = false + @State private var isUploading = false + @State private var uploadError: String? + @State private var showUploadError = false + + var body: some View { + // A wrapping flow layout keeps every icon visible: when they don't fit on + // one line they wrap to the next, instead of being hidden off-screen behind + // a horizontal scroll view. + MarkdownToolbarFlow(spacing: 10) { + toolbarButton("bold", icon: "bold", label: "Bold") { wrap("**") } + toolbarButton("italic", icon: "italic", label: "Italic") { wrap("_") } + toolbarButton("heading", icon: "number", label: "Heading") { prefix("# ") } + toolbarButton("code", icon: "chevron.left.forwardslash.chevron.right", + label: "Inline code") { wrap("`") } + toolbarButton("codeblock", icon: "text.page", label: "Code block") { wrapBlock("```") } + toolbarButton("link", icon: "link", label: "Insert link") { insertLink() } + toolbarButton("list", icon: "list.bullet", label: "Bulleted list") { prefix("- ") } + toolbarButton("quote", icon: "text.quote", label: "Quote") { prefix("> ") } + toolbarButton("task", icon: "checklist", label: "Task list") { prefix("- [ ] ") } + + if onUploadImage != nil { + uploadControl + } + } + .padding(.horizontal, 8) + .padding(.vertical, 6) + .frame(maxWidth: .infinity, alignment: .leading) + .alert("Upload Failed", isPresented: $showUploadError) { + Button("OK", role: .cancel) {} + } message: { + Text(uploadError ?? "") + } + } + + @ViewBuilder + private var uploadControl: some View { + if isUploading { + ProgressView() + .controlSize(.small) + .frame(width: 28, height: 28) + } else if ProcessInfo.processInfo.arguments.contains("-dev_testImageUpload") { + // In UI tests, bypass the pickers and upload hardcoded files directly. + Button { Task { await uploadTestImage() } } label: { + toolbarIcon("photo") + } + .buttonStyle(.plain) + .foregroundStyle(.primary) + .accessibilityLabel("Attach image") + .accessibilityIdentifier("markdown-toolbar-image") + + Button { Task { await uploadTestFile() } } label: { + toolbarIcon("doc") + } + .buttonStyle(.plain) + .foregroundStyle(.primary) + .accessibilityLabel("Attach file") + .accessibilityIdentifier("markdown-toolbar-file") + } else { + Menu { + Button { showPhotosPicker = true } label: { + Label("Photo Library", systemImage: "photo") + } + Button { showFileImporter = true } label: { + Label("Choose File", systemImage: "doc") + } + } label: { + toolbarIcon("paperclip") + } + .buttonStyle(.plain) + .foregroundStyle(.primary) + .accessibilityLabel("Attach") + .accessibilityIdentifier("markdown-toolbar-attach") + .photosPicker(isPresented: $showPhotosPicker, selection: $photosItem, matching: .images) + .onChange(of: photosItem) { _, item in + guard let item else { return } + Task { await uploadPickedImage(item) } + } + .fileImporter( + isPresented: $showFileImporter, + allowedContentTypes: [.item], + allowsMultipleSelection: false, + ) { result in + Task { await uploadPickedFile(result) } + } + } + } + + private func toolbarIcon(_ systemName: String) -> some View { + Image(systemName: systemName) + .font(.subheadline) + .frame(width: 28, height: 28) + .contentShape(Rectangle()) + } + + private func uploadTestImage() async { + guard let onUploadImage else { return } + // 1×1 transparent PNG — used in UI tests to skip the Photos picker. + let base64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==" + guard let pngData = Data(base64Encoded: base64) else { return } + await runUpload { try await onUploadImage(pngData, "test.png", "image/png") } + } + + private func uploadTestFile() async { + guard let onUploadImage else { return } + // Plain-text log file — used in UI tests to exercise the non-image + // attachment path (link markdown + file-row display). + let logData = Data("line1 ERROR boom\nline2 WARN stuff\nline3 done\n".utf8) + await runUpload { try await onUploadImage(logData, "server.log", "text/plain") } + } + + private func runUpload(_ upload: () async throws -> String) async { + isUploading = true + defer { isUploading = false } + do { + let markdown = try await upload() + if text.isEmpty || text.hasSuffix("\n") { + text.append(markdown) + } else { + text.append("\n\(markdown)") + } + } catch { + uploadError = error.localizedDescription + showUploadError = true + } + } + + private func uploadPickedFile(_ result: Result<[URL], Error>) async { + guard let onUploadImage else { return } + do { + let urls = try result.get() + guard let url = urls.first else { return } + // Files returned by the document picker live outside the app sandbox and + // must be accessed through a security-scoped resource. + let didAccess = url.startAccessingSecurityScopedResource() + defer { if didAccess { url.stopAccessingSecurityScopedResource() } } + let data = try Data(contentsOf: url) + let fileName = url.lastPathComponent + let mimeType = UTType(filenameExtension: url.pathExtension)?.preferredMIMEType + ?? "application/octet-stream" + await runUpload { try await onUploadImage(data, fileName, mimeType) } + } catch { + uploadError = error.localizedDescription + showUploadError = true + } + } + + private func uploadPickedImage(_ item: PhotosPickerItem) async { + guard let onUploadImage else { return } + isUploading = true + defer { + isUploading = false + photosItem = nil + } + do { + guard let data = try await item.loadTransferable(type: Data.self) else { return } + let mimeType = item.supportedContentTypes.first?.preferredMIMEType ?? "image/jpeg" + let ext = item.supportedContentTypes.first?.preferredFilenameExtension ?? "jpg" + let markdown = try await onUploadImage(data, "image.\(ext)", mimeType) + if text.isEmpty || text.hasSuffix("\n") { + text.append(markdown) + } else { + text.append("\n\(markdown)") + } + } catch { + uploadError = error.localizedDescription + showUploadError = true + } + } + + private func toolbarButton(_ id: String, icon: String, label: String, action: @escaping () -> Void) -> some View { + Button(action: action) { + toolbarIcon(icon) + } + .buttonStyle(.plain) + .foregroundStyle(.primary) + .accessibilityLabel(label) + .accessibilityIdentifier("markdown-toolbar-\(id)") + } + + private func wrap(_ marker: String) { + text.append("\(marker)text\(marker)") + } + + private func prefix(_ marker: String) { + if text.isEmpty || text.hasSuffix("\n") { + text.append(marker) + } else { + text.append("\n\(marker)") + } + } + + private func wrapBlock(_ fence: String) { + if text.isEmpty || text.hasSuffix("\n") { + text.append("\(fence)\n\n\(fence)") + } else { + text.append("\n\(fence)\n\n\(fence)") + } + } + + private func insertLink() { + text.append("[title](url)") + } +} diff --git a/Forji/Forji/Views/MarkdownToolbarFlow.swift b/Forji/Forji/Views/MarkdownToolbarFlow.swift new file mode 100644 index 0000000..3740625 --- /dev/null +++ b/Forji/Forji/Views/MarkdownToolbarFlow.swift @@ -0,0 +1,47 @@ +import SwiftUI + +/// A simple left-aligned flow layout: lays subviews out left to right, wrapping +/// to a new line when the next subview would overflow the available width. Used +/// by the markdown toolbar so all action icons stay visible (wrapping instead of +/// scrolling off-screen). +struct MarkdownToolbarFlow: Layout { + var spacing: CGFloat = 10 + + func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache _: inout Void) -> CGSize { + let maxWidth = proposal.width ?? .infinity + var cursorX: CGFloat = 0 + var cursorY: CGFloat = 0 + var rowHeight: CGFloat = 0 + var widestRow: CGFloat = 0 + for subview in subviews { + let size = subview.sizeThatFits(.unspecified) + if cursorX > 0, cursorX + size.width > maxWidth { + widestRow = max(widestRow, cursorX - spacing) + cursorX = 0 + cursorY += rowHeight + spacing + rowHeight = 0 + } + cursorX += size.width + spacing + rowHeight = max(rowHeight, size.height) + } + widestRow = max(widestRow, cursorX - spacing) + return CGSize(width: min(maxWidth, widestRow), height: cursorY + rowHeight) + } + + func placeSubviews(in bounds: CGRect, proposal _: ProposedViewSize, subviews: Subviews, cache _: inout Void) { + var cursorX = bounds.minX + var cursorY = bounds.minY + var rowHeight: CGFloat = 0 + for subview in subviews { + let size = subview.sizeThatFits(.unspecified) + if cursorX > bounds.minX, cursorX + size.width > bounds.maxX { + cursorX = bounds.minX + cursorY += rowHeight + spacing + rowHeight = 0 + } + subview.place(at: CGPoint(x: cursorX, y: cursorY), anchor: .topLeading, proposal: ProposedViewSize(size)) + cursorX += size.width + spacing + rowHeight = max(rowHeight, size.height) + } + } +} diff --git a/Forji/Forji/Views/MentionableEditorField.swift b/Forji/Forji/Views/MentionableEditorField.swift index 7f4afa4..e732521 100644 --- a/Forji/Forji/Views/MentionableEditorField.swift +++ b/Forji/Forji/Views/MentionableEditorField.swift @@ -7,6 +7,7 @@ struct MentionableEditorField: View { var users: [User] var minHeight: CGFloat = 150 var showToolbar: Bool = false + var onUploadImage: ((Data, String, String) async throws -> String)? @State private var mentionQuery: String? @@ -24,6 +25,7 @@ struct MentionableEditorField: View { selectedTab: $selectedTab, minHeight: minHeight, showToolbar: showToolbar, + onUploadImage: onUploadImage, ) if selectedTab == .edit, mentionQuery != nil, !matchingUsers.isEmpty { diff --git a/Forji/Forji/Views/MetadataPickers.swift b/Forji/Forji/Views/MetadataPickers.swift index 37ca6e4..56e1d3e 100644 --- a/Forji/Forji/Views/MetadataPickers.swift +++ b/Forji/Forji/Views/MetadataPickers.swift @@ -6,6 +6,7 @@ import SwiftUI struct DescriptionEditorSection: View { @Binding var text: String var title: String = "Description" + var onUploadImage: ((Data, String, String) async throws -> String)? @State private var showingEditor = false var body: some View { @@ -24,7 +25,7 @@ struct DescriptionEditorSection: View { } .tint(.primary) .sheet(isPresented: $showingEditor) { - DescriptionEditorSheet(text: $text, title: title) + DescriptionEditorSheet(text: $text, title: title, onUploadImage: onUploadImage) } } } @@ -33,6 +34,7 @@ struct DescriptionEditorSection: View { private struct DescriptionEditorSheet: View { @Binding var text: String let title: String + var onUploadImage: ((Data, String, String) async throws -> String)? @State private var selectedTab: EditPreviewTab = .edit @Environment(\.dismiss) private var dismiss @@ -42,6 +44,7 @@ private struct DescriptionEditorSheet: View { text: $text, selectedTab: $selectedTab, showToolbar: true, + onUploadImage: onUploadImage, ) .frame(maxHeight: .infinity) .padding() diff --git a/Forji/Forji/Views/PullRequestDetailView.swift b/Forji/Forji/Views/PullRequestDetailView.swift index 583dcb6..fa8150f 100644 --- a/Forji/Forji/Views/PullRequestDetailView.swift +++ b/Forji/Forji/Views/PullRequestDetailView.swift @@ -31,6 +31,7 @@ struct PullRequestDetailView: View { @State private var showActionsExpanded = false private let prService: PullRequestService? + private let issueService: IssueService? private let repositoryService: RepositoryService? init(repository: Repository, prNumber: Int, authService: AuthenticationService) { @@ -38,6 +39,7 @@ struct PullRequestDetailView: View { self.prNumber = prNumber self.authService = authService prService = authService.client.map { PullRequestService(client: $0) } + issueService = authService.client.map { IssueService(client: $0) } repositoryService = authService.client.map { RepositoryService(client: $0) } } @@ -108,6 +110,13 @@ struct PullRequestDetailView: View { } } + if let assets = activePR.assets, !assets.isEmpty { + Section("Attachments") { + AttachmentGallery(attachments: assets) + .padding(.vertical, 4) + } + } + reviewsSection changesSection conflictSection(activePR) @@ -133,6 +142,7 @@ struct PullRequestDetailView: View { repository: repository, pullRequest: editingPR, authService: authService, + onUploadImage: uploadPRImage, ) { _ in Task { await loadData() } } @@ -174,7 +184,7 @@ struct PullRequestDetailView: View { } } .sheet(isPresented: $showCommentSheet) { - CommentSheet(users: assignees) { body in + CommentSheet(users: assignees, onUploadImage: uploadPRImage) { body in guard let prService else { throw URLError(.userAuthenticationRequired) } let comment = try await prService.createComment( owner: owner, @@ -436,6 +446,15 @@ struct PullRequestDetailView: View { reviewComments.values.flatMap(\.self) } + private func uploadPRImage(data: Data, fileName: String, mimeType: String) async throws -> String { + guard let issueService else { throw URLError(.userAuthenticationRequired) } + let attachment = try await issueService.uploadIssueAttachment( + owner: owner, repo: repo, index: prNumber, + fileData: data, fileName: fileName, mimeType: mimeType, + ) + return attachmentMarkdown(for: attachment) + } + private func loadData() async { guard let prService, let repositoryService else { return } isLoading = true @@ -586,6 +605,7 @@ struct PullRequestDetailView: View { self.prNumber = prNumber self.authService = authService prService = nil + issueService = nil repositoryService = nil _pullRequest = State(initialValue: pullRequest) _comments = State(initialValue: comments) diff --git a/Forji/Forji/Views/PullRequestEditView.swift b/Forji/Forji/Views/PullRequestEditView.swift index b71e3f1..f514cc8 100644 --- a/Forji/Forji/Views/PullRequestEditView.swift +++ b/Forji/Forji/Views/PullRequestEditView.swift @@ -22,6 +22,7 @@ struct PullRequestEditView: View { private let prService: PullRequestService? private let repositoryService: RepositoryService? + private let onUploadImage: ((Data, String, String) async throws -> String)? private let onSaved: (PullRequest) -> Void private var isMerged: Bool { @@ -31,6 +32,7 @@ struct PullRequestEditView: View { init( repository: Repository, pullRequest: PullRequest, authService: AuthenticationService, + onUploadImage: ((Data, String, String) async throws -> String)? = nil, onSaved: @escaping (PullRequest) -> Void, ) { self.repository = repository @@ -38,6 +40,7 @@ struct PullRequestEditView: View { self.authService = authService prService = authService.client.map { PullRequestService(client: $0) } repositoryService = authService.client.map { RepositoryService(client: $0) } + self.onUploadImage = onUploadImage self.onSaved = onSaved _title = State(initialValue: pullRequest.title) _bodyText = State(initialValue: pullRequest.body ?? "") @@ -64,7 +67,7 @@ struct PullRequestEditView: View { .accessibilityIdentifier("pr-edit-title-field") } - DescriptionEditorSection(text: $bodyText) + DescriptionEditorSection(text: $bodyText, onUploadImage: onUploadImage) LabelPickerSection( availableLabels: availableLabels, @@ -199,6 +202,7 @@ struct PullRequestEditView: View { self.authService = authService prService = nil repositoryService = nil + onUploadImage = nil onSaved = { _ in } _title = State(initialValue: pullRequest.title) _bodyText = State(initialValue: pullRequest.body ?? "") diff --git a/Forji/Forji/Views/PullRequestListView.swift b/Forji/Forji/Views/PullRequestListView.swift index bdd5885..353be51 100644 --- a/Forji/Forji/Views/PullRequestListView.swift +++ b/Forji/Forji/Views/PullRequestListView.swift @@ -152,7 +152,8 @@ struct PullRequestListView: View { #if DEBUG init(preview _: Void, repository: Repository, authService: AuthenticationService, pullRequests: [PullRequest], - notFound: Bool = false) { + notFound: Bool = false) + { self.repository = repository self.authService = authService prService = nil diff --git a/Forji/ForjiTests/MarkdownComponentsTests.swift b/Forji/ForjiTests/MarkdownComponentsTests.swift index 46fedc2..046b682 100644 --- a/Forji/ForjiTests/MarkdownComponentsTests.swift +++ b/Forji/ForjiTests/MarkdownComponentsTests.swift @@ -1,9 +1,43 @@ import Foundation +import ForgejoKit import Testing @testable import Forji struct MarkdownComponentsTests { + private func attachment(name: String, type: String?) -> ForgejoKit.Attachment { + ForgejoKit.Attachment( + id: 1, uuid: "u", name: name, size: 0, + browserDownloadUrl: "https://example.com/a/\(name)", + type: type, created: Date(), + ) + } + + // MARK: - attachmentMarkdown + + @Test func imageAttachmentUsesEmbedSyntax() { + let md = attachmentMarkdown(for: attachment(name: "shot.png", type: "image/png")) + #expect(md == "![shot.png](https://example.com/a/shot.png)") + } + + @Test func imageDetectedByExtensionUsesEmbedSyntax() { + // Forgejo returns the generic type "attachment", so detection falls to the extension. + let md = attachmentMarkdown(for: attachment(name: "photo.jpg", type: "attachment")) + #expect(md == "![photo.jpg](https://example.com/a/photo.jpg)") + } + + @Test func nonImageFileUsesLinkSyntax() { + let md = attachmentMarkdown(for: attachment(name: "server.log", type: "text/plain")) + #expect(md == "[server.log](https://example.com/a/server.log)") + } + + @Test func nonImageBinaryUsesLinkSyntax() { + let md = attachmentMarkdown(for: attachment(name: "report.pdf", type: "attachment")) + #expect(md == "[report.pdf](https://example.com/a/report.pdf)") + } + + // MARK: - repoRelativePath + @Test func repoRelativePathExtractsFilePath() { let url = URL(string: "https://forgejo.example.com/owner/repo/src/branch/main/path/to/file.swift")! #expect(repoRelativePath(from: url) == "path/to/file.swift") diff --git a/Forji/ForjiUITests/AttachmentUITests.swift b/Forji/ForjiUITests/AttachmentUITests.swift new file mode 100644 index 0000000..594892c --- /dev/null +++ b/Forji/ForjiUITests/AttachmentUITests.swift @@ -0,0 +1,179 @@ +import XCTest + +final class AttachmentUITests: ForgejoUITestBase { + + // Tests that multiple images can be uploaded from the markdown toolbar comment + // sheet, and that both uploaded images appear in the issue's Attachments section + // on reload. The -dev_testImageUpload launch argument bypasses the PhotosPicker + // and uploads a hardcoded 1×1 PNG directly, keeping the test deterministic without + // requiring photos in the simulator's library. + @MainActor + func testUploadMultipleImagesInCommentAndVerifyDisplayed() throws { + app.launchArguments += ["-dev_testImageUpload"] + loginAndWaitForHome() + + // Navigate to an existing issue + navigateToIssueTab() + let issueCell = app.staticTexts["Test issue 1 from integration tests"].firstMatch + XCTAssertTrue(issueCell.waitForExistence(timeout: 10)) + issueCell.tap() + XCTAssertTrue(app.staticTexts["issue-detail-title"].waitForExistence(timeout: 10)) + + // Open the comment sheet + expandActionMenu() + let commentButton = app.buttons["issue-comment-button"] + XCTAssertTrue(commentButton.waitForExistence(timeout: 5)) + commentButton.tap() + + // Tap the image upload button twice — in test mode each tap uploads a + // hardcoded PNG instead of presenting the Photos picker. Two taps produce + // two distinct attachments and two markdown references. + let imageButton = app.buttons["markdown-toolbar-image"] + XCTAssertTrue(imageButton.waitForExistence(timeout: 5)) + let textEditor = app.textViews["markdown-text-editor"].firstMatch + XCTAssertTrue(textEditor.waitForExistence(timeout: 5)) + + // First upload — wait until one markdown reference is inserted before tapping + // again, so the second upload appends rather than racing the first. + imageButton.tap() + expectation(for: NSPredicate(format: "value MATCHES '.*!\\\\[.*'"), evaluatedWith: textEditor) + waitForExpectations(timeout: 15, handler: nil) + + // Second upload — wait until two markdown references are present. + imageButton.tap() + let twoRefs = NSPredicate(format: "value MATCHES '(.*!\\\\[.*){2,}'") + expectation(for: twoRefs, evaluatedWith: textEditor) + waitForExpectations(timeout: 15, handler: nil) + let editorValue = textEditor.value as? String ?? "" + XCTAssertEqual( + editorValue.components(separatedBy: "![").count - 1, 2, + "Expected two markdown image references in the editor after two uploads" + ) + + // Submit the comment + app.buttons["Submit"].tap() + + // Wait for the comment sheet (NavigationStack title "New Comment") to dismiss + let commentSheetNavBar = app.navigationBars["New Comment"] + expectation(for: NSPredicate(format: "exists == false"), evaluatedWith: commentSheetNavBar) + waitForExpectations(timeout: 10, handler: nil) + + // Navigate back to the issue list + app.navigationBars.buttons.firstMatch.tap() + XCTAssertTrue( + app.staticTexts["Test issue 1 from integration tests"].waitForExistence(timeout: 10), + "Should return to the issue list after navigating back" + ) + + // Re-enter the issue: loadData() fetches the issue fresh, + // now including the uploaded attachment in issue.assets. + app.staticTexts["Test issue 1 from integration tests"].firstMatch.tap() + XCTAssertTrue(app.staticTexts["issue-detail-title"].waitForExistence(timeout: 10)) + + // Scroll past the issue header, milestone, assignees, and description + // to reach the Attachments section that appears below them. + app.swipeUp() + app.swipeUp() + + // The Attachments section must now be visible with the gallery rendered. + XCTAssertTrue( + app.staticTexts["Attachments"].waitForExistence(timeout: 10), + "Attachments section header should appear after uploading images to the issue" + ) + let gallery = app.scrollViews["attachment-gallery"] + XCTAssertTrue( + gallery.waitForExistence(timeout: 5), + "AttachmentGallery image scroll view should be rendered inside the Attachments section" + ) + + // Both uploaded images must render as thumbnails. Each thumbnail is a Link + // wrapping an AsyncImage, exposed as an image element inside the gallery. + let twoImages = NSPredicate(format: "count >= 2") + expectation(for: twoImages, evaluatedWith: gallery.images) + waitForExpectations(timeout: 10, handler: nil) + XCTAssertGreaterThanOrEqual( + gallery.images.count, 2, + "Both uploaded images should render as thumbnails in the gallery" + ) + } + + // Tests that a non-image attachment (a text/log file) is handled correctly: + // the inserted markdown is a plain link (not an image embed), and after reload + // the file renders as a file row (paperclip + name) rather than an image + // thumbnail. The -dev_testImageUpload launch argument also exposes a companion + // "Attach file" button that uploads a hardcoded server.log. + @MainActor + func testUploadLogFileInCommentRendersAsFileRow() throws { + app.launchArguments += ["-dev_testImageUpload"] + loginAndWaitForHome() + + // Navigate to an existing issue + navigateToIssueTab() + let issueCell = app.staticTexts["Test issue 2 from integration tests"].firstMatch + XCTAssertTrue(issueCell.waitForExistence(timeout: 10)) + issueCell.tap() + XCTAssertTrue(app.staticTexts["issue-detail-title"].waitForExistence(timeout: 10)) + + // Open the comment sheet + expandActionMenu() + let commentButton = app.buttons["issue-comment-button"] + XCTAssertTrue(commentButton.waitForExistence(timeout: 5)) + commentButton.tap() + + // Tap the file upload button — in test mode this uploads a hardcoded server.log. + let fileButton = app.buttons["markdown-toolbar-file"] + XCTAssertTrue(fileButton.waitForExistence(timeout: 5)) + let textEditor = app.textViews["markdown-text-editor"].firstMatch + XCTAssertTrue(textEditor.waitForExistence(timeout: 5)) + fileButton.tap() + + // Wait for the upload to finish: a markdown reference is inserted. + expectation(for: NSPredicate(format: "value CONTAINS 'server.log'"), evaluatedWith: textEditor) + waitForExpectations(timeout: 15, handler: nil) + + // A non-image attachment must use link syntax [name](url), NOT image-embed + // syntax ![name](url) — otherwise it would render as a broken inline image. + let editorValue = textEditor.value as? String ?? "" + XCTAssertTrue( + editorValue.contains("[server.log]"), + "Expected a markdown link to the uploaded log file" + ) + XCTAssertFalse( + editorValue.contains("!["), + "A non-image file must not use image-embed markdown syntax" + ) + + // Submit the comment + app.buttons["Submit"].tap() + let commentSheetNavBar = app.navigationBars["New Comment"] + expectation(for: NSPredicate(format: "exists == false"), evaluatedWith: commentSheetNavBar) + waitForExpectations(timeout: 10, handler: nil) + + // Navigate back to the issue list, then re-enter to reload issue.assets. + app.navigationBars.buttons.firstMatch.tap() + let listCell = app.staticTexts["Test issue 2 from integration tests"] + XCTAssertTrue(listCell.waitForExistence(timeout: 10)) + listCell.firstMatch.tap() + XCTAssertTrue(app.staticTexts["issue-detail-title"].waitForExistence(timeout: 10)) + + // Scroll down to the Attachments section. + app.swipeUp() + app.swipeUp() + + XCTAssertTrue( + app.staticTexts["Attachments"].waitForExistence(timeout: 10), + "Attachments section header should appear after uploading a file" + ) + // The log must render as a file row (not an image thumbnail), showing its name. + // A SwiftUI Link may surface as a link or a button, so match any element type. + let fileRow = app.descendants(matching: .any).matching(identifier: "attachment-file-row").firstMatch + XCTAssertTrue( + fileRow.waitForExistence(timeout: 5), + "Non-image attachment should render as a file row" + ) + XCTAssertTrue( + app.staticTexts["server.log"].waitForExistence(timeout: 5), + "File row should display the attachment file name" + ) + } +}