From d9f59bb5eb806ee43e3eac896ab571456e34d76a Mon Sep 17 00:00:00 2001 From: Akshay Kolli Date: Thu, 11 Jun 2026 22:03:43 -0700 Subject: [PATCH] v0.1: Fixed comment issues --- Sources/IHatePDFs/AppState.swift | 254 ++++++++++++++++-- Sources/IHatePDFs/CommentEditorView.swift | 21 ++ Sources/IHatePDFs/PDFKitRepresentedView.swift | 70 ++++- Sources/IHatePDFs/SidebarViews.swift | 133 ++++++++- Sources/IHatePDFsCore/AnnotationFactory.swift | 155 +++++++++-- Sources/IHatePDFsCore/AnnotationModels.swift | 18 +- .../AnnotationFactoryTests.swift | 104 ++++++- 7 files changed, 704 insertions(+), 51 deletions(-) diff --git a/Sources/IHatePDFs/AppState.swift b/Sources/IHatePDFs/AppState.swift index 926a09e..0086234 100644 --- a/Sources/IHatePDFs/AppState.swift +++ b/Sources/IHatePDFs/AppState.swift @@ -125,6 +125,10 @@ final class AppState: NSObject, ObservableObject { @Published var selectedAuthorFilter = "All Authors" @Published var selectedStatusFilter = ReviewState.allStatuses @Published var collapsedPageIndexes: Set = [] + @Published var sidebarReplyParentID: String? + @Published var sidebarReplyTargetID: String? + @Published var sidebarReplyDraft = "" + @Published var sidebarReplyAuthor = AnnotationFactory.defaultAuthor @Published var statusMessage = "Open a PDF to begin." private var pageObserver: NSObjectProtocol? @@ -196,7 +200,15 @@ final class AppState: NSObject, ObservableObject { } var repliesByParent: [String: [AnnotationSnapshot]] { - Dictionary(grouping: filteredAnnotations.filter(\.isReply), by: \.parentID!) + Dictionary( + grouping: filteredAnnotations.filter { $0.isReply && $0.hasComment }, + by: \.parentID! + ) + } + + var sidebarReplyTarget: AnnotationSnapshot? { + guard let sidebarReplyTargetID else { return nil } + return annotations.first { $0.id == sidebarReplyTargetID } } func attachPDFView(_ view: PDFView) { @@ -266,6 +278,7 @@ final class AppState: NSObject, ObservableObject { selectedAnnotationID = nil activeEditor = nil placementTool = nil + clearSidebarReplyDraft() refreshAnnotations() statusMessage = "Opened \(url.lastPathComponent)." } @@ -278,6 +291,7 @@ final class AppState: NSObject, ObservableObject { selectedAnnotationID = nil activeEditor = nil placementTool = nil + clearSidebarReplyDraft() searchResults = [] searchText = "" showToolbarSearch = false @@ -338,7 +352,9 @@ final class AppState: NSObject, ObservableObject { switch alert.runModal() { case .alertFirstButtonReturn: + preparePopupMarkersForExport(in: document) guard document.write(to: url) else { + hidePopupMarkersInViewer(in: document) showAlert(title: "Save Failed", message: "The PDF could not be written to \(url.path).") return } @@ -405,21 +421,85 @@ final class AppState: NSObject, ObservableObject { } func addReply(to item: AnnotationSnapshot) { + beginSidebarReply(to: item) + } + + func beginSidebarReply( + to target: AnnotationSnapshot, + inThread threadRoot: AnnotationSnapshot? = nil + ) { + guard let root = threadRoot ?? rootComment(for: target) else { + statusMessage = "Original comment no longer exists." + return + } + + activeEditor = nil + showCommentsSidebar = true + sidebarReplyParentID = root.id + sidebarReplyTargetID = target.id + sidebarReplyDraft = "" + sidebarReplyAuthor = AnnotationFactory.defaultAuthor + selectedAnnotationID = target.id + statusMessage = "Replying to \(target.author)." + } + + func cancelSidebarReply() { + clearSidebarReplyDraft() + statusMessage = "Reply canceled." + } + + func commitSidebarReply() { + let trimmedText = sidebarReplyDraft.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmedText.isEmpty else { + statusMessage = "Type a reply before saving." + return + } + + guard let sidebarReplyParentID, + let parent = annotations.first(where: { $0.id == sidebarReplyParentID }) + else { + clearSidebarReplyDraft() + statusMessage = "Original comment no longer exists." + return + } + + let author = sidebarReplyAuthor.trimmingCharacters(in: .whitespacesAndNewlines) let insertion = AnnotationFactory.replyInsertion( - to: item.annotation, - on: item.page, - comment: "", - author: AnnotationFactory.defaultAuthor, - parentID: item.id + to: parent.annotation, + on: parent.page, + comment: trimmedText, + author: author.isEmpty ? AnnotationFactory.defaultAuthor : author, + parentID: parent.id ) add(insertion) + clearSidebarReplyDraft() refreshAnnotations() - openEditor( - title: "Reply", - annotations: [insertion.annotation], - pages: [item.page], - isNew: true - ) + + if let reply = annotations.first(where: { $0.annotation === insertion.annotation }) { + selectedAnnotationID = reply.id + } + statusMessage = "Reply added." + } + + func replyFromEditor( + _ context: AnnotationEditorContext, + text: String, + author: String + ) { + updateAnnotations(in: context, text: text, author: author) + refreshAnnotations() + + guard let annotation = context.primaryAnnotation, + let item = snapshot(for: annotation) + else { + activeEditor = nil + showCommentsSidebar = true + statusMessage = "Comment saved." + return + } + + activeEditor = nil + beginSidebarReply(to: item) } func edit(_ item: AnnotationSnapshot) { @@ -447,6 +527,10 @@ final class AppState: NSObject, ObservableObject { if hoveredAnnotationID.map(targetIDs.contains) == true { hoveredAnnotationID = nil } + if sidebarReplyParentID.map(targetIDs.contains) == true + || sidebarReplyTargetID.map(targetIDs.contains) == true { + clearSidebarReplyDraft() + } activeEditor = nil refreshAnnotations() @@ -557,10 +641,14 @@ final class AppState: NSObject, ObservableObject { func refreshAnnotations() { guard let document else { annotations = [] + clearSidebarReplyDraft() return } hideReplyMarkers(in: document) + normalizePopupMarkers(in: document) + hidePopupMarkersInViewer(in: document) annotations = AnnotationReader.snapshots(in: document) + pruneSidebarReplyDraftIfNeeded() } func runSearch() { @@ -727,9 +815,10 @@ final class AppState: NSObject, ObservableObject { private func add(_ insertion: AnnotationInsertion) { insertion.page.addAnnotation(insertion.annotation) - if let popup = insertion.popup { - insertion.page.addAnnotation(popup) + if AnnotationKeys.isReply(insertion.annotation) { + AnnotationFactory.hideReplyMarker(insertion.annotation, on: insertion.page) } + detachPopupMarkerFromViewer(for: insertion.annotation, on: insertion.page) pdfView?.annotationsChanged(on: insertion.page) } @@ -744,15 +833,13 @@ final class AppState: NSObject, ObservableObject { for (index, annotation) in context.annotations.enumerated() { guard index < context.pages.count else { continue } let page = context.pages[index] - let popup = AnnotationFactory.updateComment( + _ = AnnotationFactory.updateComment( for: annotation, on: page, text: text, author: authorValue ) - if let popup { - page.addAnnotation(popup) - } + detachPopupMarkerFromViewer(for: annotation, on: page) pdfView?.annotationsChanged(on: page) } } @@ -788,7 +875,7 @@ final class AppState: NSObject, ObservableObject { pages: pages, isNewAnnotation: isNew, allowsDelete: true, - initialText: first?.contents ?? "", + initialText: first.map(AnnotationKeys.commentText(for:)) ?? "", initialAuthor: first?.userName ?? AnnotationFactory.defaultAuthor ) } @@ -805,6 +892,38 @@ final class AppState: NSObject, ObservableObject { } } + private func rootComment(for target: AnnotationSnapshot) -> AnnotationSnapshot? { + guard let parentID = target.parentID else { return target } + return annotations.first { $0.id == parentID } + } + + private func snapshot(for annotation: PDFAnnotation) -> AnnotationSnapshot? { + annotations.first { $0.annotation === annotation } + } + + private func clearSidebarReplyDraft() { + sidebarReplyParentID = nil + sidebarReplyTargetID = nil + sidebarReplyDraft = "" + sidebarReplyAuthor = AnnotationFactory.defaultAuthor + } + + private func pruneSidebarReplyDraftIfNeeded() { + guard sidebarReplyParentID != nil || sidebarReplyTargetID != nil else { return } + + let ids = Set(annotations.map(\.id)) + guard let parentID = sidebarReplyParentID, + ids.contains(parentID) + else { + clearSidebarReplyDraft() + return + } + + if sidebarReplyTargetID.map(ids.contains) != true { + sidebarReplyTargetID = parentID + } + } + private func configure(_ view: PDFView) { view.displayMode = .singlePageContinuous view.displayDirection = .vertical @@ -821,7 +940,9 @@ final class AppState: NSObject, ObservableObject { } private func write(_ document: PDFDocument, to url: URL) { + preparePopupMarkersForExport(in: document) guard document.write(to: url) else { + hidePopupMarkersInViewer(in: document) showAlert(title: "Save Failed", message: "The PDF could not be written to \(url.path).") return } @@ -871,6 +992,101 @@ final class AppState: NSObject, ObservableObject { } } + private func normalizePopupMarkers(in document: PDFDocument) { + var changedPages = Set() + + for pageIndex in 0..() + + for pageIndex in 0..() + + for pageIndex in 0.. Bool { + AnnotationFactory.detachPopupForViewer(from: annotation, on: page) + } + private func navigate(to page: PDFPage, pageIndex: Int) { guard let pdfView else { return } diff --git a/Sources/IHatePDFs/CommentEditorView.swift b/Sources/IHatePDFs/CommentEditorView.swift index 99f596b..c22c0c6 100644 --- a/Sources/IHatePDFs/CommentEditorView.swift +++ b/Sources/IHatePDFs/CommentEditorView.swift @@ -34,6 +34,12 @@ final class CommentPopoverModel: ObservableObject { guard !didFinish else { return } appState?.updateEditorDraft(context, text: text, author: author) } + + func reply() { + guard !didFinish else { return } + didFinish = true + appState?.replyFromEditor(context, text: text, author: author) + } } struct CommentEditorView: View { @@ -130,9 +136,23 @@ struct CommentEditorView: View { .stroke(InterfacePalette.hairline(for: colorScheme), lineWidth: 1) } .frame(width: 190) + .layoutPriority(1) Spacer() + if !model.context.isNewAnnotation, + model.context.primaryAnnotation != nil { + Button { + model.reply() + } label: { + Label("Reply", systemImage: "arrowshape.turn.up.left") + } + .labelStyle(.iconOnly) + .frame(width: 34) + .help("Reply") + .accessibilityLabel("Reply") + } + if model.context.allowsDelete { Button(role: .destructive) { model.delete() @@ -140,6 +160,7 @@ struct CommentEditorView: View { Label("Delete Annotation", systemImage: "trash") } .labelStyle(.iconOnly) + .frame(width: 34) .help("Delete Annotation") } } diff --git a/Sources/IHatePDFs/PDFKitRepresentedView.swift b/Sources/IHatePDFs/PDFKitRepresentedView.swift index a3bd6d9..b382bb1 100644 --- a/Sources/IHatePDFs/PDFKitRepresentedView.swift +++ b/Sources/IHatePDFs/PDFKitRepresentedView.swift @@ -15,13 +15,15 @@ final class AcademicPDFView: PDFView { window?.invalidateCursorRects(for: self) } } + private var handledAnnotationMouseDown = false override var acceptsFirstResponder: Bool { true } override func mouseDown(with event: NSEvent) { + handledAnnotationMouseDown = false let point = convert(event.locationInWindow, from: nil) - if let page = page(for: point, nearest: false) { + if let page = page(for: point, nearest: false) ?? page(for: point, nearest: true) { closeNativePopups(on: page) let pagePoint = convert(point, to: page) @@ -31,6 +33,7 @@ final class AcademicPDFView: PDFView { } if let annotation = editableAnnotation(on: page, at: pagePoint) { + handledAnnotationMouseDown = true closeNativePopups(on: page) onAnnotationClick?(annotation, page) return @@ -49,6 +52,34 @@ final class AcademicPDFView: PDFView { } } + override func mouseUp(with event: NSEvent) { + if handledAnnotationMouseDown { + handledAnnotationMouseDown = false + return + } + + let point = convert(event.locationInWindow, from: nil) + let page = page(for: point, nearest: false) ?? page(for: point, nearest: true) + let pagePoint = page.map { convert(point, to: $0) } + + super.mouseUp(with: event) + + guard let page else { return } + DispatchQueue.main.async { [weak self] in + guard let self else { return } + + let clickedAnnotation = pagePoint.flatMap { + self.editableAnnotation(on: page, at: $0) + } + let target = clickedAnnotation ?? self.openNativePopupOwner(on: page) + + self.closeNativePopups(on: page) + if let target { + self.onAnnotationClick?(target, page) + } + } + } + override func rightMouseDown(with event: NSEvent) { guard hasCommentableSelection else { super.rightMouseDown(with: event) @@ -142,7 +173,7 @@ final class AcademicPDFView: PDFView { } if isTextMarkup(editable), - editable.bounds.insetBy(dx: -24, dy: -24).contains(point) { + textMarkupInteractionBounds(for: editable, on: page).contains(point) { return editable } } @@ -171,11 +202,46 @@ final class AcademicPDFView: PDFView { } } + private func openNativePopupOwner(on page: PDFPage) -> PDFAnnotation? { + for annotation in page.annotations.reversed() { + if annotation.popup?.isOpen == true, + isEditableAcademicAnnotation(annotation) { + return annotation + } + + guard AnnotationKeys.annotation(annotation, hasSubtype: .popup), + annotation.isOpen, + let owner = popupOwner(for: annotation, on: page), + isEditableAcademicAnnotation(owner) + else { + continue + } + + return owner + } + + return nil + } + private func isTextMarkup(_ annotation: PDFAnnotation) -> Bool { AnnotationKeys.annotation(annotation, hasSubtype: .highlight) || AnnotationKeys.annotation(annotation, hasSubtype: .underline) } + private func textMarkupInteractionBounds( + for annotation: PDFAnnotation, + on page: PDFPage + ) -> CGRect { + var bounds = annotation.bounds.insetBy(dx: -48, dy: -48) + + if let popup = annotation.popup { + bounds = bounds.union(popup.bounds.insetBy(dx: -16, dy: -16)) + } + + let pageBounds = page.bounds(for: displayBox).insetBy(dx: -64, dy: -64) + return bounds.intersection(pageBounds) + } + private func closeNativePopups(on page: PDFPage) { for annotation in page.annotations { if AnnotationKeys.annotation(annotation, hasSubtype: .popup) { diff --git a/Sources/IHatePDFs/SidebarViews.swift b/Sources/IHatePDFs/SidebarViews.swift index 0472d17..eeeab9d 100644 --- a/Sources/IHatePDFs/SidebarViews.swift +++ b/Sources/IHatePDFs/SidebarViews.swift @@ -330,9 +330,14 @@ private struct CommentRow: View { parentComment ForEach(replies) { reply in - ReplyRow(item: reply) + ReplyRow(item: reply, threadRoot: item) .id(reply.sidebarRenderID) } + + if appState.sidebarReplyParentID == item.id { + SidebarReplyComposer(threadRoot: item) + .id("reply-composer-\(item.id)") + } } } .padding(.horizontal, 10) @@ -371,7 +376,7 @@ private struct CommentRow: View { appState.edit(item) } Button("Reply") { - appState.addReply(to: item) + appState.beginSidebarReply(to: item, inThread: item) } Button("Delete", role: .destructive) { appState.delete(item) @@ -405,10 +410,17 @@ private struct CommentRow: View { .lineLimit(1) } - Text(item.contents.isEmpty ? "No text" : item.contents) - .font(.callout) - .foregroundStyle(item.contents.isEmpty ? InterfacePalette.quietText(for: colorScheme) : InterfacePalette.primaryText(for: colorScheme)) - .fixedSize(horizontal: false, vertical: true) + if item.hasComment { + Text(item.contents) + .font(.callout) + .foregroundStyle(InterfacePalette.primaryText(for: colorScheme)) + .fixedSize(horizontal: false, vertical: true) + } else if replies.isEmpty { + Text("No comment text") + .font(.callout) + .foregroundStyle(InterfacePalette.quietText(for: colorScheme)) + .fixedSize(horizontal: false, vertical: true) + } } } @@ -460,6 +472,111 @@ private struct CommentMarker: View { } } +private struct SidebarReplyComposer: View { + @EnvironmentObject private var appState: AppState + @Environment(\.colorScheme) private var colorScheme + @FocusState private var isFocused: Bool + let threadRoot: AnnotationSnapshot + + private let editorHorizontalInset: CGFloat = 7 + private let editorVerticalInset: CGFloat = 6 + + var body: some View { + HStack(alignment: .top, spacing: 8) { + CommentMarker(symbolName: "arrowshape.turn.up.left", size: 22, font: .caption2) + .frame(width: 28, alignment: .center) + .padding(.top, 9) + .help("Reply") + + VStack(alignment: .leading, spacing: 7) { + HStack(alignment: .firstTextBaseline, spacing: 6) { + Text("Reply") + .font(.caption.weight(.semibold)) + .foregroundStyle(InterfacePalette.primaryText(for: colorScheme)) + + if let target = appState.sidebarReplyTarget { + Text("to \(target.author)") + .font(.caption2) + .foregroundStyle(InterfacePalette.secondaryText(for: colorScheme)) + .lineLimit(1) + } else { + Text("to \(threadRoot.author)") + .font(.caption2) + .foregroundStyle(InterfacePalette.secondaryText(for: colorScheme)) + .lineLimit(1) + } + + Spacer() + } + + ZStack(alignment: .topLeading) { + TextEditor(text: $appState.sidebarReplyDraft) + .font(.callout) + .foregroundStyle(InterfacePalette.primaryText(for: colorScheme)) + .scrollContentBackground(.hidden) + .focused($isFocused) + .padding(.horizontal, editorHorizontalInset) + .padding(.vertical, editorVerticalInset) + + if appState.sidebarReplyDraft.isEmpty { + Text("Write a reply") + .font(.callout) + .foregroundStyle(InterfacePalette.quietText(for: colorScheme)) + .padding(.leading, editorHorizontalInset + 6) + .padding(.top, editorVerticalInset) + .allowsHitTesting(false) + } + } + .frame(minHeight: 76) + .background(InterfacePalette.fieldFill(for: colorScheme)) + .clipShape(RoundedRectangle(cornerRadius: 6)) + .overlay { + RoundedRectangle(cornerRadius: 6) + .stroke(InterfacePalette.hairline(for: colorScheme), lineWidth: 1) + } + + HStack(spacing: 8) { + TextField("Author", text: $appState.sidebarReplyAuthor) + .textFieldStyle(.plain) + .foregroundStyle(InterfacePalette.primaryText(for: colorScheme)) + .padding(.horizontal, 7) + .frame(height: 26) + .background(InterfacePalette.fieldFill(for: colorScheme)) + .clipShape(RoundedRectangle(cornerRadius: 6)) + .overlay { + RoundedRectangle(cornerRadius: 6) + .stroke(InterfacePalette.hairline(for: colorScheme), lineWidth: 1) + } + + Spacer() + + Button("Cancel") { + appState.cancelSidebarReply() + } + .buttonStyle(.plain) + .foregroundStyle(InterfacePalette.secondaryText(for: colorScheme)) + + Button { + appState.commitSidebarReply() + } label: { + Label("Reply", systemImage: "arrowshape.turn.up.left") + } + .disabled(appState.sidebarReplyDraft.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) + .keyboardShortcut(.return, modifiers: [.command]) + } + .font(.caption.weight(.medium)) + } + } + .padding(.top, 9) + .padding(.bottom, 2) + .onAppear { + DispatchQueue.main.async { + isFocused = true + } + } + } +} + private struct ReviewStatusChip: View { @EnvironmentObject private var appState: AppState @Environment(\.colorScheme) private var colorScheme @@ -514,6 +631,7 @@ private struct ReplyRow: View { @EnvironmentObject private var appState: AppState @Environment(\.colorScheme) private var colorScheme let item: AnnotationSnapshot + let threadRoot: AnnotationSnapshot var body: some View { HStack(alignment: .top, spacing: 8) { @@ -542,6 +660,9 @@ private struct ReplyRow: View { Button("Edit") { appState.edit(item) } + Button("Reply") { + appState.beginSidebarReply(to: item, inThread: threadRoot) + } Button("Delete", role: .destructive) { appState.delete(item) } diff --git a/Sources/IHatePDFsCore/AnnotationFactory.swift b/Sources/IHatePDFsCore/AnnotationFactory.swift index 9bd0e05..ce2d846 100644 --- a/Sources/IHatePDFsCore/AnnotationFactory.swift +++ b/Sources/IHatePDFsCore/AnnotationFactory.swift @@ -206,7 +206,8 @@ public enum AnnotationFactory { ?? UUID().uuidString _ = annotation.setValue(parentIdentifier, forAnnotationKey: AnnotationKeys.inReplyTo) _ = annotation.setValue("R", forAnnotationKey: AnnotationKeys.replyType) - hideReplyMarker(annotation, on: page) + annotation.shouldDisplay = false + annotation.shouldPrint = false return AnnotationInsertion(page: page, annotation: annotation, popup: nil) } @@ -217,6 +218,7 @@ public enum AnnotationFactory { author: String, date: Date = Date() ) -> PDFAnnotation? { + AnnotationKeys.setCommentText(text, for: annotation) annotation.contents = text annotation.userName = author annotation.modificationDate = date @@ -263,6 +265,7 @@ public enum AnnotationFactory { author: String, date: Date ) { + AnnotationKeys.setCommentText(comment, for: annotation) annotation.contents = comment annotation.userName = author annotation.modificationDate = date @@ -286,9 +289,8 @@ public enum AnnotationFactory { ) -> PDFAnnotation? { guard !AnnotationKeys.annotation(annotation, hasSubtype: .popup) else { return nil } guard !AnnotationKeys.annotation(annotation, hasSubtype: .freeText) else { return nil } - guard let contents = annotation.contents, - !contents.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty - else { + let contents = AnnotationKeys.commentText(for: annotation) + guard !contents.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { return nil } @@ -297,6 +299,7 @@ public enum AnnotationFactory { popup.userName = annotation.userName popup.modificationDate = annotation.modificationDate popup.isOpen = open + popup.bounds = popupRect(for: annotation.bounds, on: page) popup.shouldDisplay = true popup.shouldPrint = true return popup.page == nil ? popup : nil @@ -314,23 +317,131 @@ public enum AnnotationFactory { return popup } + @discardableResult + public static func normalizePopupPlacement( + for annotation: PDFAnnotation, + on page: PDFPage + ) -> Bool { + guard let popup = annotation.popup else { return false } + + let bounds = popupRect(for: annotation.bounds, on: page) + guard popup.bounds != bounds else { return false } + popup.bounds = bounds + return true + } + + @discardableResult + public static func setPopupMarkerVisibility( + for annotation: PDFAnnotation, + on page: PDFPage, + isVisible: Bool + ) -> Bool { + guard let popup = annotation.popup else { return false } + + let oldBounds = popup.bounds + let oldShouldDisplay = popup.shouldDisplay + let oldShouldPrint = popup.shouldPrint + let oldIsOpen = popup.isOpen + + popup.bounds = popupRect(for: annotation.bounds, on: page) + popup.shouldDisplay = isVisible + popup.shouldPrint = isVisible + popup.isOpen = false + + return oldBounds != popup.bounds + || oldShouldDisplay != popup.shouldDisplay + || oldShouldPrint != popup.shouldPrint + || oldIsOpen != popup.isOpen + } + + @discardableResult + public static func restoreCommentTextForExport(_ annotation: PDFAnnotation) -> Bool { + let contents = AnnotationKeys.commentText(for: annotation) + let exportedContents = contents.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + ? nil + : contents + let oldContents = annotation.contents + + annotation.contents = exportedContents + if !contents.isEmpty { + AnnotationKeys.setCommentText(contents, for: annotation) + } + + return oldContents != annotation.contents + } + + @discardableResult + public static func detachPopupForViewer( + from annotation: PDFAnnotation, + on page: PDFPage + ) -> Bool { + let contents = AnnotationKeys.commentText(for: annotation) + let userName = annotation.userName + let modificationDate = annotation.modificationDate + let creationDate = annotation.value(forAnnotationKey: AnnotationKeys.creationDate) + let textLabel = annotation.value(forAnnotationKey: .textLabel) + let date = annotation.value(forAnnotationKey: .date) + let shouldSuppressNativeContents = !AnnotationKeys.isReply(annotation) + && !AnnotationKeys.annotation(annotation, hasSubtype: .freeText) + let oldContents = annotation.contents + var didChange = false + + if !contents.isEmpty || annotation.value(forAnnotationKey: AnnotationKeys.appCommentText) == nil { + AnnotationKeys.setCommentText(contents, for: annotation) + } + + if let popup = annotation.popup { + popup.isOpen = false + popup.shouldDisplay = false + popup.shouldPrint = false + if popup.page != nil { + page.removeAnnotation(popup) + } + annotation.popup = nil + didChange = true + } + + annotation.contents = shouldSuppressNativeContents ? nil : contents + annotation.userName = userName + annotation.modificationDate = modificationDate + if let creationDate { + _ = annotation.setValue(creationDate, forAnnotationKey: AnnotationKeys.creationDate) + } + if let textLabel { + _ = annotation.setValue(textLabel, forAnnotationKey: .textLabel) + } + if let date { + _ = annotation.setValue(date, forAnnotationKey: .date) + } + + return didChange || oldContents != annotation.contents + } + public static func hideReplyMarker(_ annotation: PDFAnnotation, on page: PDFPage) { guard AnnotationKeys.isReply(annotation) else { return } - let pageBounds = page.bounds(for: .cropBox) - annotation.bounds = CGRect( - x: pageBounds.maxX + 32, - y: pageBounds.maxY + 32, - width: 1, - height: 1 - ) - annotation.shouldDisplay = true - annotation.shouldPrint = false + let contents = AnnotationKeys.commentText(for: annotation) + let userName = annotation.userName + let modificationDate = annotation.modificationDate if let popup = annotation.popup { page.removeAnnotation(popup) annotation.popup = nil } + + let pageBounds = page.bounds(for: .cropBox) + annotation.bounds = CGRect( + x: pageBounds.maxX + 32, + y: pageBounds.maxY + 32, + width: 24, + height: 24 + ) + annotation.shouldDisplay = false + annotation.shouldPrint = false + AnnotationKeys.setCommentText(contents, for: annotation) + annotation.contents = contents + annotation.userName = userName + annotation.modificationDate = modificationDate } public static func parentAnnotation(for annotation: PDFAnnotation) -> PDFAnnotation { @@ -357,17 +468,19 @@ public enum AnnotationFactory { private static func popupRect(for annotationBounds: CGRect, on page: PDFPage) -> CGRect { let pageBounds = page.bounds(for: .cropBox) - let desired = CGRect( - x: annotationBounds.maxX + 10, - y: max(annotationBounds.minY - 96, pageBounds.minY + 12), + let indicatorInset: CGFloat = 28 + let verticalInset: CGFloat = 12 + let y = min( + max(annotationBounds.maxY - indicatorInset, pageBounds.minY + verticalInset), + pageBounds.maxY - indicatorInset - verticalInset + ) + + return CGRect( + x: pageBounds.maxX - indicatorInset, + y: y, width: 240, height: 120 ) - return clampedRect( - desired: desired, - on: page, - fallbackSize: CGSize(width: 240, height: 120) - ) } private static func clampedRect( diff --git a/Sources/IHatePDFsCore/AnnotationModels.swift b/Sources/IHatePDFsCore/AnnotationModels.swift index 6c02303..9f41f73 100644 --- a/Sources/IHatePDFsCore/AnnotationModels.swift +++ b/Sources/IHatePDFsCore/AnnotationModels.swift @@ -154,6 +154,19 @@ public enum AnnotationKeys { public static let stateModel = PDFAnnotationKey(rawValue: "StateModel") public static let appKind = PDFAnnotationKey(rawValue: "IHatePDFsKind") public static let appKindComment = "Comment" + public static let appCommentText = PDFAnnotationKey(rawValue: "IHatePDFsCommentText") + + public static func commentText(for annotation: PDFAnnotation) -> String { + if let value = annotation.value(forAnnotationKey: appCommentText) as? String { + return value + } + + return annotation.contents ?? "" + } + + public static func setCommentText(_ text: String, for annotation: PDFAnnotation) { + _ = annotation.setValue(text, forAnnotationKey: appCommentText) + } public static func stableID( for annotation: PDFAnnotation, @@ -282,7 +295,8 @@ public enum AnnotationReader { guard !AnnotationKeys.annotation(annotation, hasSubtype: .popup) else { continue } let kind = AcademicAnnotationKind(annotation: annotation) - guard kind != .other || annotation.contents?.isEmpty == false else { continue } + let contents = AnnotationKeys.commentText(for: annotation) + guard kind != .other || !contents.isEmpty else { continue } let id = AnnotationKeys.stableID( for: annotation, @@ -310,7 +324,7 @@ public enum AnnotationReader { createdAt: createdAt, modifiedAt: annotation.modificationDate, status: status, - contents: annotation.contents ?? "", + contents: contents, bounds: annotation.bounds, annotation: annotation, page: page, diff --git a/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift b/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift index f027c13..f1b66d7 100644 --- a/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift +++ b/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift @@ -110,6 +110,101 @@ final class AnnotationFactoryTests: XCTestCase { }) } + func testPopupMarkerIsPlacedInRightPageMargin() throws { + let document = try makeSelectableTextDocument() + let page = try XCTUnwrap(document.page(at: 0)) + let selection = try XCTUnwrap(page.selection(for: NSRange(location: 0, length: 29))) + let insertion = try XCTUnwrap( + AnnotationFactory.markupInsertions( + from: selection, + style: .comment, + comment: "Margin marker.", + author: "Professor" + ).first + ) + let popup = try XCTUnwrap(insertion.popup) + let pageBounds = page.bounds(for: .cropBox) + + XCTAssertEqual(popup.bounds.minX, pageBounds.maxX - 28, accuracy: 0.01) + XCTAssertGreaterThanOrEqual(popup.bounds.minY, pageBounds.minY + 12) + XCTAssertLessThanOrEqual(popup.bounds.minY, pageBounds.maxY - 40) + + popup.bounds = insertion.annotation.bounds.offsetBy(dx: 10, dy: 0) + XCTAssertTrue(AnnotationFactory.normalizePopupPlacement(for: insertion.annotation, on: page)) + XCTAssertEqual(popup.bounds.minX, pageBounds.maxX - 28, accuracy: 0.01) + } + + func testDetachingPopupForViewerKeepsSidebarCommentText() throws { + let document = try makeSelectableTextDocument() + let page = try XCTUnwrap(document.page(at: 0)) + let selection = try XCTUnwrap(page.selection(for: NSRange(location: 0, length: 29))) + let insertion = try XCTUnwrap( + AnnotationFactory.markupInsertions( + from: selection, + style: .comment, + comment: "", + author: "Professor" + ).first + ) + + page.addAnnotation(insertion.annotation) + let popup = try XCTUnwrap(AnnotationFactory.updateComment( + for: insertion.annotation, + on: page, + text: "Visible comment text.", + author: "Professor" + )) + page.addAnnotation(popup) + + XCTAssertTrue(AnnotationFactory.detachPopupForViewer(from: insertion.annotation, on: page)) + XCTAssertNil(insertion.annotation.popup) + XCTAssertNil(insertion.annotation.contents) + XCTAssertEqual(AnnotationKeys.commentText(for: insertion.annotation), "Visible comment text.") + XCTAssertFalse(page.annotations.contains { AnnotationKeys.annotation($0, hasSubtype: .popup) }) + + let snapshot = try XCTUnwrap(AnnotationReader.snapshots(in: document).first { + $0.annotation === insertion.annotation + }) + XCTAssertEqual(snapshot.contents, "Visible comment text.") + } + + func testRestoreCommentTextForExportWritesStandardPDFContents() throws { + let document = try makeSelectableTextDocument() + let page = try XCTUnwrap(document.page(at: 0)) + let selection = try XCTUnwrap(page.selection(for: NSRange(location: 0, length: 29))) + let insertion = try XCTUnwrap( + AnnotationFactory.markupInsertions( + from: selection, + style: .comment, + comment: "Exported comment text.", + author: "Professor" + ).first + ) + + page.addAnnotation(insertion.annotation) + if let popup = insertion.popup { + page.addAnnotation(popup) + } + AnnotationFactory.detachPopupForViewer(from: insertion.annotation, on: page) + + XCTAssertNil(insertion.annotation.contents) + XCTAssertTrue(AnnotationFactory.restoreCommentTextForExport(insertion.annotation)) + let popup = try XCTUnwrap(AnnotationFactory.makePopupIfNeeded( + for: insertion.annotation, + on: page, + open: false + )) + if popup.page == nil { + page.addAnnotation(popup) + } + + let reopenedPage = try saveAndReopen(document).page(at: 0).unwrap() + XCTAssertTrue(reopenedPage.annotations.contains { + AnnotationKeys.annotation($0, hasSubtype: .highlight) + && $0.contents == "Exported comment text." + }) + } + func testAddingAnnotationPreservesPriorAnnotation() throws { let document = try makeSelectableTextDocument() let page = try XCTUnwrap(document.page(at: 0)) @@ -239,8 +334,14 @@ final class AnnotationFactoryTests: XCTestCase { XCTAssertTrue(AnnotationKeys.annotation(reply.annotation, hasSubtype: .text)) XCTAssertEqual(reply.annotation.value(forAnnotationKey: AnnotationKeys.inReplyTo) as? String, "parent-id") XCTAssertEqual(reply.annotation.value(forAnnotationKey: AnnotationKeys.replyType) as? String, "R") - XCTAssertTrue(reply.annotation.shouldDisplay) + XCTAssertFalse(reply.annotation.shouldDisplay) XCTAssertFalse(reply.annotation.shouldPrint) + + page.addAnnotation(reply.annotation) + AnnotationFactory.hideReplyMarker(reply.annotation, on: page) + XCTAssertFalse(reply.annotation.shouldDisplay) + XCTAssertFalse(reply.annotation.shouldPrint) + XCTAssertEqual(AnnotationKeys.commentText(for: reply.annotation), "Reply") XCTAssertGreaterThan(reply.annotation.bounds.minX, page.bounds(for: .cropBox).maxX) XCTAssertGreaterThan(reply.annotation.bounds.minY, page.bounds(for: .cropBox).maxY) XCTAssertNil(reply.popup) @@ -268,6 +369,7 @@ final class AnnotationFactoryTests: XCTestCase { parentID: parentID ).annotation page.addAnnotation(reply) + AnnotationFactory.hideReplyMarker(reply, on: page) let snapshots = AnnotationReader.snapshots(in: document) let parentSnapshot = try XCTUnwrap(snapshots.first { $0.contents == "Parent" })