v0.1: Fixed comment issues

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<Int> = []
+    @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,
+            to: parent.annotation,
-            on: item.page,
+            on: parent.page,
-            comment: "",
+            comment: trimmedText,
-            author: AnnotationFactory.defaultAuthor,
+            author: author.isEmpty ? AnnotationFactory.defaultAuthor : author,
-            parentID: item.id
+            parentID: parent.id
         )
         add(insertion)
+        clearSidebarReplyDraft()
         refreshAnnotations()
-        openEditor(
+
-            title: "Reply",
+        if let reply = annotations.first(where: { $0.annotation === insertion.annotation }) {
-            annotations: [insertion.annotation],
+            selectedAnnotationID = reply.id
-            pages: [item.page],
+        }
-            isNew: true
+        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 {
+        if AnnotationKeys.isReply(insertion.annotation) {
-            insertion.page.addAnnotation(popup)
+            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 {
+            detachPopupMarkerFromViewer(for: annotation, on: page)
-                page.addAnnotation(popup)
-            }
             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<PDFPage>()
+
+        for pageIndex in 0..<document.pageCount {
+            guard let page = document.page(at: pageIndex) else { continue }
+            for annotation in page.annotations where !AnnotationKeys.annotation(annotation, hasSubtype: .popup) {
+                if AnnotationFactory.normalizePopupPlacement(for: annotation, on: page) {
+                    changedPages.insert(page)
+                }
+            }
+        }
+
+        for page in changedPages {
+            pdfView?.annotationsChanged(on: page)
+        }
+    }
+
+    private func preparePopupMarkersForExport(in document: PDFDocument) {
+        var changedPages = Set<PDFPage>()
+
+        for pageIndex in 0..<document.pageCount {
+            guard let page = document.page(at: pageIndex) else { continue }
+            for annotation in page.annotations where !AnnotationKeys.annotation(annotation, hasSubtype: .popup) {
+                if AnnotationFactory.restoreCommentTextForExport(annotation) {
+                    changedPages.insert(page)
+                }
+
+                guard !AnnotationKeys.isReply(annotation),
+                      !AnnotationKeys.annotation(annotation, hasSubtype: .freeText)
+                else {
+                    continue
+                }
+
+                if let popup = AnnotationFactory.makePopupIfNeeded(
+                    for: annotation,
+                    on: page,
+                    open: false
+                ), popup.page == nil {
+                    page.addAnnotation(popup)
+                    changedPages.insert(page)
+                }
+
+                if AnnotationFactory.setPopupMarkerVisibility(
+                    for: annotation,
+                    on: page,
+                    isVisible: true
+                ) {
+                    changedPages.insert(page)
+                }
+            }
+        }
+
+        for page in changedPages {
+            pdfView?.annotationsChanged(on: page)
+        }
+    }
+
+    private func hidePopupMarkersInViewer(in document: PDFDocument) {
+        var changedPages = Set<PDFPage>()
+
+        for pageIndex in 0..<document.pageCount {
+            guard let page = document.page(at: pageIndex) else { continue }
+            var popupsToRemove: [PDFAnnotation] = []
+
+            for annotation in page.annotations {
+                if AnnotationKeys.annotation(annotation, hasSubtype: .popup) {
+                    annotation.isOpen = false
+                    annotation.shouldDisplay = false
+                    annotation.shouldPrint = false
+                    popupsToRemove.append(annotation)
+                    continue
+                }
+
+                if detachPopupMarkerFromViewer(for: annotation, on: page) {
+                    changedPages.insert(page)
+                }
+            }
+
+            guard !popupsToRemove.isEmpty else { continue }
+            for popup in popupsToRemove {
+                page.removeAnnotation(popup)
+            }
+            changedPages.insert(page)
+        }
+
+        for page in changedPages {
+            pdfView?.annotationsChanged(on: page)
+        }
+    }
+
+    @discardableResult
+    private func detachPopupMarkerFromViewer(for annotation: PDFAnnotation, on page: PDFPage) -> Bool {
+        AnnotationFactory.detachPopupForViewer(from: annotation, on: page)
+    }
+
     private func navigate(to page: PDFPage, pageIndex: Int) {
         guard let pdfView else { return }
 

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")
             }
         }

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) {

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)
+            if item.hasComment {
-                .font(.callout)
+                Text(item.contents)
-                .foregroundStyle(item.contents.isEmpty ? InterfacePalette.quietText(for: colorScheme) : InterfacePalette.primaryText(for: colorScheme))
+                    .font(.callout)
-                .fixedSize(horizontal: false, vertical: true)
+                    .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)
                     }

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,
+        let contents = AnnotationKeys.commentText(for: annotation)
-              !contents.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty
+        guard !contents.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
-        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)
+        let contents = AnnotationKeys.commentText(for: annotation)
-        annotation.bounds = CGRect(
+        let userName = annotation.userName
-            x: pageBounds.maxX + 32,
+        let modificationDate = annotation.modificationDate
-            y: pageBounds.maxY + 32,
-            width: 1,
-            height: 1
-        )
-        annotation.shouldDisplay = true
-        annotation.shouldPrint = false
 
         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(
+        let indicatorInset: CGFloat = 28
-            x: annotationBounds.maxX + 10,
+        let verticalInset: CGFloat = 12
-            y: max(annotationBounds.minY - 96, pageBounds.minY + 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(

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,

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" })