diff --git a/Sources/IHatePDFs/AppState.swift b/Sources/IHatePDFs/AppState.swift index eae3ffd..926a09e 100644 --- a/Sources/IHatePDFs/AppState.swift +++ b/Sources/IHatePDFs/AppState.swift @@ -559,6 +559,7 @@ final class AppState: NSObject, ObservableObject { annotations = [] return } + hideReplyMarkers(in: document) annotations = AnnotationReader.snapshots(in: document) } @@ -854,6 +855,22 @@ final class AppState: NSObject, ObservableObject { pdfView.go(to: selection) } + private func hideReplyMarkers(in document: PDFDocument) { + var changedPages = Set() + + for pageIndex in 0.. PDFAnnotation? { if let direct = page.annotation(at: point), - let editable = editableParent(for: direct) { + let editable = editableParent(for: direct, on: page) { return editable } for annotation in page.annotations.reversed() { - guard let editable = editableParent(for: annotation) else { continue } + guard let editable = editableParent(for: annotation, on: page) else { continue } if annotation.bounds.insetBy(dx: -8, dy: -8).contains(point) { return editable @@ -141,11 +150,27 @@ final class AcademicPDFView: PDFView { return nil } - private func editableParent(for annotation: PDFAnnotation) -> PDFAnnotation? { + private func editableParent(for annotation: PDFAnnotation, on page: PDFPage) -> PDFAnnotation? { + if let owner = popupOwner(for: annotation, on: page) { + return isEditableAcademicAnnotation(owner) ? owner : nil + } + let parent = AnnotationFactory.parentAnnotation(for: annotation) return isEditableAcademicAnnotation(parent) ? parent : nil } + private func popupOwner(for annotation: PDFAnnotation, on page: PDFPage) -> PDFAnnotation? { + guard AnnotationKeys.annotation(annotation, hasSubtype: .popup) else { return nil } + + if let parent = annotation.value(forAnnotationKey: .parent) as? PDFAnnotation { + return parent + } + + return page.annotations.first { candidate in + candidate.popup === annotation + } + } + private func isTextMarkup(_ annotation: PDFAnnotation) -> Bool { AnnotationKeys.annotation(annotation, hasSubtype: .highlight) || AnnotationKeys.annotation(annotation, hasSubtype: .underline) @@ -166,7 +191,6 @@ final class AcademicPDFView: PDFView { || AnnotationKeys.annotation(annotation, hasSubtype: .underline) || AnnotationKeys.annotation(annotation, hasSubtype: .text) || AnnotationKeys.annotation(annotation, hasSubtype: .freeText) - || AnnotationKeys.annotation(annotation, hasSubtype: .popup) } } diff --git a/Sources/IHatePDFsCore/AnnotationFactory.swift b/Sources/IHatePDFsCore/AnnotationFactory.swift index 0bc57ab..9bd0e05 100644 --- a/Sources/IHatePDFsCore/AnnotationFactory.swift +++ b/Sources/IHatePDFsCore/AnnotationFactory.swift @@ -206,8 +206,8 @@ public enum AnnotationFactory { ?? UUID().uuidString _ = annotation.setValue(parentIdentifier, forAnnotationKey: AnnotationKeys.inReplyTo) _ = annotation.setValue("R", forAnnotationKey: AnnotationKeys.replyType) - let popup = makePopupIfNeeded(for: annotation, on: page, open: false) - return AnnotationInsertion(page: page, annotation: annotation, popup: popup) + hideReplyMarker(annotation, on: page) + return AnnotationInsertion(page: page, annotation: annotation, popup: nil) } public static func updateComment( @@ -241,6 +241,11 @@ public enum AnnotationFactory { return nil } + if AnnotationKeys.isReply(annotation) { + hideReplyMarker(annotation, on: page) + return nil + } + if let popup = annotation.popup { popup.contents = text popup.userName = author @@ -309,6 +314,25 @@ public enum AnnotationFactory { return popup } + 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 + + if let popup = annotation.popup { + page.removeAnnotation(popup) + annotation.popup = nil + } + } + public static func parentAnnotation(for annotation: PDFAnnotation) -> PDFAnnotation { if AnnotationKeys.annotation(annotation, hasSubtype: .popup), let parent = annotation.value(forAnnotationKey: .parent) as? PDFAnnotation { diff --git a/Sources/IHatePDFsCore/AnnotationModels.swift b/Sources/IHatePDFsCore/AnnotationModels.swift index 5866351..6c02303 100644 --- a/Sources/IHatePDFsCore/AnnotationModels.swift +++ b/Sources/IHatePDFsCore/AnnotationModels.swift @@ -180,7 +180,7 @@ public enum AnnotationKeys { ) -> String? { if let parentID = annotation.value(forAnnotationKey: inReplyTo) as? String, !parentID.isEmpty { - return parentID + return stableIDForAnnotation(named: parentID, in: document) ?? parentID } guard let parent = annotation.value(forAnnotationKey: inReplyTo) as? PDFAnnotation else { @@ -199,6 +199,20 @@ public enum AnnotationKeys { return stableID(for: parent, pageIndex: pageIndex, annotationIndex: annotationIndex) } + private static func stableIDForAnnotation(named name: String, in document: PDFDocument?) -> String? { + guard let document else { return nil } + + for pageIndex in 0.. Bool { annotation.value(forAnnotationKey: inReplyTo) is PDFAnnotation || annotation.value(forAnnotationKey: inReplyTo) is String diff --git a/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift b/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift index af60a7a..f027c13 100644 --- a/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift +++ b/Tests/IHatePDFsCoreTests/AnnotationFactoryTests.swift @@ -220,7 +220,7 @@ final class AnnotationFactoryTests: XCTestCase { XCTAssertTrue(insertion.popup.map { AnnotationKeys.annotation($0, hasSubtype: .popup) } ?? false) } - func testReplyStoresVisibleTextAnnotationWithBestEffortParentID() throws { + func testReplyStoresHiddenTextAnnotationWithBestEffortParentID() throws { let page = PDFPage() let parent = AnnotationFactory.noteInsertion( on: page, @@ -239,6 +239,42 @@ 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.shouldPrint) + XCTAssertGreaterThan(reply.annotation.bounds.minX, page.bounds(for: .cropBox).maxX) + XCTAssertGreaterThan(reply.annotation.bounds.minY, page.bounds(for: .cropBox).maxY) + XCTAssertNil(reply.popup) + } + + func testStringReplyParentIDResolvesToParentStableID() throws { + let document = PDFDocument() + let page = PDFPage() + document.insert(page, at: 0) + + let parent = AnnotationFactory.noteInsertion( + on: page, + near: CGPoint(x: 100, y: 100), + comment: "Parent", + author: "Professor" + ).annotation + page.addAnnotation(parent) + + let parentID = try XCTUnwrap(parent.value(forAnnotationKey: .name) as? String) + let reply = AnnotationFactory.replyInsertion( + to: parent, + on: page, + comment: "Reply", + author: "Reader", + parentID: parentID + ).annotation + page.addAnnotation(reply) + + let snapshots = AnnotationReader.snapshots(in: document) + let parentSnapshot = try XCTUnwrap(snapshots.first { $0.contents == "Parent" }) + let replySnapshot = try XCTUnwrap(snapshots.first { $0.contents == "Reply" }) + + XCTAssertEqual(replySnapshot.kind, .reply) + XCTAssertEqual(replySnapshot.parentID, parentSnapshot.id) } func testFreeTextCreatesStandardFreeTextAnnotation() throws { diff --git a/docs/QA.md b/docs/QA.md index 704a6a7..92e0907 100644 --- a/docs/QA.md +++ b/docs/QA.md @@ -24,7 +24,7 @@ Use at least: 8. Quit and reopen the same PDF at the same approximate window width and verify the app restores that PDF's sidebar state; then open a different PDF and verify it starts in focused single-pane reading unless that document has its own saved state. 9. Add at least one reply and verify the comments sidebar presents the thread like a clean review/chat stream, with a visible connector line from the parent comment to the reply. 10. Hover a comment row and verify the corresponding PDF text is highlighted; click both the parent comment text and the reply text in the sidebar and verify the PDF view navigates to and selects the corresponding annotation. -11. Verify highlights, comment markers, reply icons, and selected sidebar rows use muted native-feeling colors in light mode and do not visually overpower the document. +11. Verify highlights, comment markers, hidden page-level replies, and selected sidebar rows use muted native-feeling colors in light mode and do not visually overpower the document. 12. Switch the app to dark mode and verify the reading background, comments sidebar, editor popover, connector lines, selected rows, text fields, and annotation markers remain legible and restrained. 13. Save As an annotated copy. 14. Reopen the annotated copy in I Hate PDFs and verify the annotations and comments remain. @@ -69,4 +69,4 @@ Capture current screenshots in `docs/screenshots` for: ## Known Version 1 Limitation -PDFKit rejects object-valued `/IRT` reply relationships through its public API. Replies created in this app are saved as visible standard `/Text` annotations, while full cross-reader reply-thread presentation must be verified and improved with a lower-level PDF writer if needed. +PDFKit rejects object-valued `/IRT` reply relationships through its public API. Replies created in this app are saved as standard `/Text` annotations with string `/IRT` and `/RT` reply keys, but the reply annotation is hidden on the PDF page so it appears as a threaded sidebar reply instead of a second page icon. Full cross-reader reply-thread presentation must be verified and improved with a lower-level PDF writer if needed. diff --git a/goal.md b/goal.md index c431718..3ccd4ee 100644 --- a/goal.md +++ b/goal.md @@ -211,6 +211,7 @@ It should: - Let users change review state directly from a compact Reviewed/Not reviewed chip in each comment row. - Include search and filters, but hide advanced filters behind a compact filter menu or disclosure control. - Keep replies visually subordinate to their parent comment. +- Replies should appear in the comments sidebar thread and should not create additional visible page icons on the PDF. - Draw subtle vertical connector lines that make reply threads visually clear, like a clean comments/chat section. - Navigate to and select the associated PDF annotation when a parent comment or reply is clicked. - Temporarily highlight the referenced PDF text when a comment row or reply row is hovered. diff --git a/scripts/verify-pdf-annotations.swift b/scripts/verify-pdf-annotations.swift index c15d018..52d61e6 100644 --- a/scripts/verify-pdf-annotations.swift +++ b/scripts/verify-pdf-annotations.swift @@ -98,13 +98,13 @@ for pageNumber in 1...document.numberOfPages { try requirePopup(in: annotation, page: pageNumber, index: annotationIndex, subtype: "Underline") case "Text": try requireTextKeys(in: annotation, page: pageNumber, index: annotationIndex) - try requirePopup(in: annotation, page: pageNumber, index: annotationIndex, subtype: "Text") if hasString(in: annotation, key: "IRT") || hasString(in: annotation, key: "RT") { summary.replies += 1 try requireString(in: annotation, key: "IRT", page: pageNumber, index: annotationIndex) try requireString(in: annotation, key: "RT", page: pageNumber, index: annotationIndex) } else { + try requirePopup(in: annotation, page: pageNumber, index: annotationIndex, subtype: "Text") summary.textNotes += 1 } case "FreeText": @@ -145,7 +145,7 @@ guard summary.replies > 0 else { guard summary.freeText > 0 else { throw VerificationError.missingExpectedSubtype("FreeText") } -guard summary.popups >= 5 else { +guard summary.popups >= 4 else { throw VerificationError.missingExpectedSubtype("Popup") } diff --git a/scripts/verify-sample-pdf.swift b/scripts/verify-sample-pdf.swift index 4bde683..9d8a052 100644 --- a/scripts/verify-sample-pdf.swift +++ b/scripts/verify-sample-pdf.swift @@ -91,11 +91,13 @@ reply.color = NSColor(calibratedRed: 0.52, green: 0.58, blue: 0.60, alpha: 0.88) standardize( reply, name: "verify-reply", - contents: "This reply is saved as a visible PDF text annotation.", + contents: "This reply is saved as PDF reply data without drawing an extra page icon.", author: "Reader" ) _ = reply.setValue("verify-text-note", forAnnotationKey: PDFAnnotationKey(rawValue: "IRT")) _ = reply.setValue("R", forAnnotationKey: PDFAnnotationKey(rawValue: "RT")) +reply.shouldDisplay = false +reply.shouldPrint = false page.addAnnotation(reply) let freeText = PDFAnnotation( @@ -128,7 +130,7 @@ precondition(annotations.contains { matches($0, .highlight) && $0.contents?.cont precondition(annotations.contains { matches($0, .highlight) && $0.contents?.contains("selected-text comment") == true }) precondition(annotations.contains { matches($0, .underline) && $0.contents?.contains("underline") == true }) precondition(annotations.contains { matches($0, .text) && $0.contents?.contains("text annotation") == true }) -precondition(annotations.contains { matches($0, .text) && $0.contents?.contains("reply") == true }) +precondition(annotations.contains { matches($0, .text) && $0.contents?.contains("reply") == true && !$0.shouldDisplay && !$0.shouldPrint }) precondition(annotations.contains { matches($0, .freeText) && $0.contents?.contains("Free text") == true }) print("Verified standard PDF annotations in \(outputURL.path)")