v0.1: Fixed replies icons

This commit is contained in:
Akshay Kolli
2026-06-11 18:34:41 -07:00
parent a75582584a
commit 472198d39c
9 changed files with 132 additions and 14 deletions

View File

@@ -559,6 +559,7 @@ final class AppState: NSObject, ObservableObject {
annotations = [] annotations = []
return return
} }
hideReplyMarkers(in: document)
annotations = AnnotationReader.snapshots(in: document) annotations = AnnotationReader.snapshots(in: document)
} }
@@ -854,6 +855,22 @@ final class AppState: NSObject, ObservableObject {
pdfView.go(to: selection) pdfView.go(to: selection)
} }
private func hideReplyMarkers(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.isReply(annotation) {
AnnotationFactory.hideReplyMarker(annotation, on: page)
changedPages.insert(page)
}
}
for page in changedPages {
pdfView?.annotationsChanged(on: page)
}
}
private func navigate(to page: PDFPage, pageIndex: Int) { private func navigate(to page: PDFPage, pageIndex: Int) {
guard let pdfView else { return } guard let pdfView else { return }

View File

@@ -22,6 +22,7 @@ final class AcademicPDFView: PDFView {
let point = convert(event.locationInWindow, from: nil) let point = convert(event.locationInWindow, from: nil)
if let page = page(for: point, nearest: false) { if let page = page(for: point, nearest: false) {
closeNativePopups(on: page)
let pagePoint = convert(point, to: page) let pagePoint = convert(point, to: page)
if placementTool != nil { if placementTool != nil {
@@ -38,6 +39,14 @@ final class AcademicPDFView: PDFView {
super.mouseDown(with: event) super.mouseDown(with: event)
window?.makeFirstResponder(self) window?.makeFirstResponder(self)
DispatchQueue.main.async { [weak self] in
guard let self,
let page = self.page(for: point, nearest: false)
else {
return
}
self.closeNativePopups(on: page)
}
} }
override func rightMouseDown(with event: NSEvent) { override func rightMouseDown(with event: NSEvent) {
@@ -116,12 +125,12 @@ final class AcademicPDFView: PDFView {
private func editableAnnotation(on page: PDFPage, at point: CGPoint) -> PDFAnnotation? { private func editableAnnotation(on page: PDFPage, at point: CGPoint) -> PDFAnnotation? {
if let direct = page.annotation(at: point), if let direct = page.annotation(at: point),
let editable = editableParent(for: direct) { let editable = editableParent(for: direct, on: page) {
return editable return editable
} }
for annotation in page.annotations.reversed() { 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) { if annotation.bounds.insetBy(dx: -8, dy: -8).contains(point) {
return editable return editable
@@ -141,11 +150,27 @@ final class AcademicPDFView: PDFView {
return nil 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) let parent = AnnotationFactory.parentAnnotation(for: annotation)
return isEditableAcademicAnnotation(parent) ? parent : nil 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 { private func isTextMarkup(_ annotation: PDFAnnotation) -> Bool {
AnnotationKeys.annotation(annotation, hasSubtype: .highlight) AnnotationKeys.annotation(annotation, hasSubtype: .highlight)
|| AnnotationKeys.annotation(annotation, hasSubtype: .underline) || AnnotationKeys.annotation(annotation, hasSubtype: .underline)
@@ -166,7 +191,6 @@ final class AcademicPDFView: PDFView {
|| AnnotationKeys.annotation(annotation, hasSubtype: .underline) || AnnotationKeys.annotation(annotation, hasSubtype: .underline)
|| AnnotationKeys.annotation(annotation, hasSubtype: .text) || AnnotationKeys.annotation(annotation, hasSubtype: .text)
|| AnnotationKeys.annotation(annotation, hasSubtype: .freeText) || AnnotationKeys.annotation(annotation, hasSubtype: .freeText)
|| AnnotationKeys.annotation(annotation, hasSubtype: .popup)
} }
} }

View File

@@ -206,8 +206,8 @@ public enum AnnotationFactory {
?? UUID().uuidString ?? UUID().uuidString
_ = annotation.setValue(parentIdentifier, forAnnotationKey: AnnotationKeys.inReplyTo) _ = annotation.setValue(parentIdentifier, forAnnotationKey: AnnotationKeys.inReplyTo)
_ = annotation.setValue("R", forAnnotationKey: AnnotationKeys.replyType) _ = annotation.setValue("R", forAnnotationKey: AnnotationKeys.replyType)
let popup = makePopupIfNeeded(for: annotation, on: page, open: false) hideReplyMarker(annotation, on: page)
return AnnotationInsertion(page: page, annotation: annotation, popup: popup) return AnnotationInsertion(page: page, annotation: annotation, popup: nil)
} }
public static func updateComment( public static func updateComment(
@@ -241,6 +241,11 @@ public enum AnnotationFactory {
return nil return nil
} }
if AnnotationKeys.isReply(annotation) {
hideReplyMarker(annotation, on: page)
return nil
}
if let popup = annotation.popup { if let popup = annotation.popup {
popup.contents = text popup.contents = text
popup.userName = author popup.userName = author
@@ -309,6 +314,25 @@ public enum AnnotationFactory {
return popup 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 { public static func parentAnnotation(for annotation: PDFAnnotation) -> PDFAnnotation {
if AnnotationKeys.annotation(annotation, hasSubtype: .popup), if AnnotationKeys.annotation(annotation, hasSubtype: .popup),
let parent = annotation.value(forAnnotationKey: .parent) as? PDFAnnotation { let parent = annotation.value(forAnnotationKey: .parent) as? PDFAnnotation {

View File

@@ -180,7 +180,7 @@ public enum AnnotationKeys {
) -> String? { ) -> String? {
if let parentID = annotation.value(forAnnotationKey: inReplyTo) as? String, if let parentID = annotation.value(forAnnotationKey: inReplyTo) as? String,
!parentID.isEmpty { !parentID.isEmpty {
return parentID return stableIDForAnnotation(named: parentID, in: document) ?? parentID
} }
guard let parent = annotation.value(forAnnotationKey: inReplyTo) as? PDFAnnotation else { 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) 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..<document.pageCount {
guard let page = document.page(at: pageIndex) else { continue }
for (annotationIndex, candidate) in page.annotations.enumerated() {
guard candidate.value(forAnnotationKey: .name) as? String == name else { continue }
return stableID(for: candidate, pageIndex: pageIndex, annotationIndex: annotationIndex)
}
}
return nil
}
public static func isReply(_ annotation: PDFAnnotation) -> Bool { public static func isReply(_ annotation: PDFAnnotation) -> Bool {
annotation.value(forAnnotationKey: inReplyTo) is PDFAnnotation annotation.value(forAnnotationKey: inReplyTo) is PDFAnnotation
|| annotation.value(forAnnotationKey: inReplyTo) is String || annotation.value(forAnnotationKey: inReplyTo) is String

View File

@@ -220,7 +220,7 @@ final class AnnotationFactoryTests: XCTestCase {
XCTAssertTrue(insertion.popup.map { AnnotationKeys.annotation($0, hasSubtype: .popup) } ?? false) XCTAssertTrue(insertion.popup.map { AnnotationKeys.annotation($0, hasSubtype: .popup) } ?? false)
} }
func testReplyStoresVisibleTextAnnotationWithBestEffortParentID() throws { func testReplyStoresHiddenTextAnnotationWithBestEffortParentID() throws {
let page = PDFPage() let page = PDFPage()
let parent = AnnotationFactory.noteInsertion( let parent = AnnotationFactory.noteInsertion(
on: page, on: page,
@@ -239,6 +239,42 @@ final class AnnotationFactoryTests: XCTestCase {
XCTAssertTrue(AnnotationKeys.annotation(reply.annotation, hasSubtype: .text)) XCTAssertTrue(AnnotationKeys.annotation(reply.annotation, hasSubtype: .text))
XCTAssertEqual(reply.annotation.value(forAnnotationKey: AnnotationKeys.inReplyTo) as? String, "parent-id") XCTAssertEqual(reply.annotation.value(forAnnotationKey: AnnotationKeys.inReplyTo) as? String, "parent-id")
XCTAssertEqual(reply.annotation.value(forAnnotationKey: AnnotationKeys.replyType) as? String, "R") 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 { func testFreeTextCreatesStandardFreeTextAnnotation() throws {

View File

@@ -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. 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. 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. 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. 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. 13. Save As an annotated copy.
14. Reopen the annotated copy in I Hate PDFs and verify the annotations and comments remain. 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 ## 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.

View File

@@ -211,6 +211,7 @@ It should:
- Let users change review state directly from a compact Reviewed/Not reviewed chip in each comment row. - 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. - Include search and filters, but hide advanced filters behind a compact filter menu or disclosure control.
- Keep replies visually subordinate to their parent comment. - 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. - 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. - 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. - Temporarily highlight the referenced PDF text when a comment row or reply row is hovered.

View File

@@ -98,13 +98,13 @@ for pageNumber in 1...document.numberOfPages {
try requirePopup(in: annotation, page: pageNumber, index: annotationIndex, subtype: "Underline") try requirePopup(in: annotation, page: pageNumber, index: annotationIndex, subtype: "Underline")
case "Text": case "Text":
try requireTextKeys(in: annotation, page: pageNumber, index: annotationIndex) 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") { if hasString(in: annotation, key: "IRT") || hasString(in: annotation, key: "RT") {
summary.replies += 1 summary.replies += 1
try requireString(in: annotation, key: "IRT", page: pageNumber, index: annotationIndex) try requireString(in: annotation, key: "IRT", page: pageNumber, index: annotationIndex)
try requireString(in: annotation, key: "RT", page: pageNumber, index: annotationIndex) try requireString(in: annotation, key: "RT", page: pageNumber, index: annotationIndex)
} else { } else {
try requirePopup(in: annotation, page: pageNumber, index: annotationIndex, subtype: "Text")
summary.textNotes += 1 summary.textNotes += 1
} }
case "FreeText": case "FreeText":
@@ -145,7 +145,7 @@ guard summary.replies > 0 else {
guard summary.freeText > 0 else { guard summary.freeText > 0 else {
throw VerificationError.missingExpectedSubtype("FreeText") throw VerificationError.missingExpectedSubtype("FreeText")
} }
guard summary.popups >= 5 else { guard summary.popups >= 4 else {
throw VerificationError.missingExpectedSubtype("Popup") throw VerificationError.missingExpectedSubtype("Popup")
} }

View File

@@ -91,11 +91,13 @@ reply.color = NSColor(calibratedRed: 0.52, green: 0.58, blue: 0.60, alpha: 0.88)
standardize( standardize(
reply, reply,
name: "verify-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" author: "Reader"
) )
_ = reply.setValue("verify-text-note", forAnnotationKey: PDFAnnotationKey(rawValue: "IRT")) _ = reply.setValue("verify-text-note", forAnnotationKey: PDFAnnotationKey(rawValue: "IRT"))
_ = reply.setValue("R", forAnnotationKey: PDFAnnotationKey(rawValue: "RT")) _ = reply.setValue("R", forAnnotationKey: PDFAnnotationKey(rawValue: "RT"))
reply.shouldDisplay = false
reply.shouldPrint = false
page.addAnnotation(reply) page.addAnnotation(reply)
let freeText = PDFAnnotation( 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, .highlight) && $0.contents?.contains("selected-text comment") == true })
precondition(annotations.contains { matches($0, .underline) && $0.contents?.contains("underline") == 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("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 }) precondition(annotations.contains { matches($0, .freeText) && $0.contents?.contains("Free text") == true })
print("Verified standard PDF annotations in \(outputURL.path)") print("Verified standard PDF annotations in \(outputURL.path)")