v0.1: Fixed comment issues

This commit is contained in:
Akshay Kolli
2026-06-11 22:03:43 -07:00
parent 472198d39c
commit d9f59bb5eb
7 changed files with 704 additions and 51 deletions

View File

@@ -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,
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<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 }

View File

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

View File

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

View File

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