Skip to content

Commit

Permalink
popover bug fixes, refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
michalrentka committed Aug 8, 2024
1 parent 7793223 commit d0b4813
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 54 deletions.
2 changes: 1 addition & 1 deletion Zotero/Scenes/Detail/HTML:EPUB/HtmlEpubCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ extension HtmlEpubCoordinator: HtmlEpubSidebarCoordinatorDelegate {
popoverDelegate: UIPopoverPresentationControllerDelegate,
userInterfaceStyle: UIUserInterfaceStyle
) -> PublishSubject<AnnotationPopoverState>? {
guard let currentNavigationController = navigationController, let annotation = viewModel.state.selectedAnnotationKey.flatMap({ viewModel.state.annotations[$0] }) else { return nil }
guard let currentNavigationController = navigationController, let annotation = viewModel.state.annotationPopoverKey.flatMap({ viewModel.state.annotations[$0] }) else { return nil }

DDLogInfo("HtmlEpubCoordinator: show annotation popover")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ enum HtmlEpubReaderAction {
case deinitialiseReader
case deselectAnnotationDuringEditing(String)
case deselectSelectedAnnotation
case hideAnnotationPopover
case initialiseReader
case loadDocument
case parseAndCacheComment(key: String, comment: String)
Expand All @@ -22,9 +23,9 @@ enum HtmlEpubReaderAction {
case saveAnnotations([String: Any])
case searchAnnotations(String)
case searchDocument(String)
case selectAnnotationDuringEditing(String)
case selectAnnotationFromSidebar(String)
case selectAnnotationFromDocument(key: String, rect: CGRect)
case selectAnnotationDuringEditing(key: String)
case selectAnnotationFromSidebar(key: String)
case selectAnnotationFromDocument(key: String)
case setColor(key: String, color: String)
case setComment(key: String, comment: NSAttributedString)
case setCommentActive(Bool)
Expand All @@ -33,6 +34,7 @@ enum HtmlEpubReaderAction {
case setTags(key: String, tags: [Tag])
case setToolOptions(color: String?, size: CGFloat?, tool: AnnotationTool)
case setViewState([String: Any])
case showAnnotationPopover(key: String, rect: CGRect)
case toggleTool(AnnotationTool)
case updateAnnotationProperties(key: String, color: String, lineWidth: CGFloat, pageLabel: String, updateSubsequentLabels: Bool, highlightText: String)
}
10 changes: 6 additions & 4 deletions Zotero/Scenes/Detail/HTML:EPUB/Models/HtmlEpubReaderState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct HtmlEpubReaderState: ViewModelState {
static let sidebarEditingSelection = Changes(rawValue: 1 << 7)
static let settings = Changes(rawValue: 1 << 8)
static let readerInitialised = Changes(rawValue: 1 << 9)
static let popover = Changes(rawValue: 1 << 10)
}

struct DocumentData {
Expand Down Expand Up @@ -73,7 +74,11 @@ struct HtmlEpubReaderState: ViewModelState {
var annotationSearchTerm: String?
var annotationFilter: AnnotationsFilter?
var selectedAnnotationKey: String?
var selectedAnnotationRect: CGRect?
var selectedAnnotationCommentActive: Bool
/// Selected annotations when annotations are being edited in sidebar
var selectedAnnotationsDuringEditing: Set<String>
var annotationPopoverKey: String?
var annotationPopoverRect: CGRect?
var documentSearchTerm: String?
var comments: [String: NSAttributedString]
var changes: Changes
Expand All @@ -86,12 +91,9 @@ struct HtmlEpubReaderState: ViewModelState {
var focusSidebarKey: String?
/// Annotation key to focus in document
var focusDocumentKey: String?
var selectedAnnotationCommentActive: Bool
var sidebarEditingEnabled: Bool
var notificationToken: NotificationToken?
var deletionEnabled: Bool
/// Selected annotations when annotations are being edited in sidebar
var selectedAnnotationsDuringEditing: Set<String>

init(url: URL, key: String, settings: HtmlEpubSettings, libraryId: LibraryIdentifier, userId: Int, username: String) {
let originalFile = Files.file(from: url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,26 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro

case .selectAnnotationFromSidebar(let key):
update(viewModel: viewModel) { state in
_select(data: (key, nil), didSelectInDocument: false, state: &state)
_select(key: key, didSelectInDocument: false, state: &state)
}

case .selectAnnotationFromDocument(let key, let rect):
case .selectAnnotationFromDocument(let key):
update(viewModel: viewModel) { state in
_select(data: (key, rect), didSelectInDocument: true, state: &state)
_select(key: key, didSelectInDocument: true, state: &state)
}

case .showAnnotationPopover(let key, let rect):
update(viewModel: viewModel) { state in
state.annotationPopoverKey = key
state.annotationPopoverRect = rect
state.changes.insert(.popover)
}

case .hideAnnotationPopover:
update(viewModel: viewModel) { state in
state.annotationPopoverKey = nil
state.annotationPopoverRect = nil
state.changes.insert(.popover)
}

case .setComment(let key, let comment):
Expand All @@ -91,7 +105,7 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro

case .deselectSelectedAnnotation:
update(viewModel: viewModel) { state in
_select(data: nil, didSelectInDocument: false, state: &state)
_select(key: nil, didSelectInDocument: false, state: &state)
}

case .parseAndCacheComment(key: let key, comment: let comment):
Expand Down Expand Up @@ -188,7 +202,7 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro

if enabled {
// Deselect selected annotation before editing
_select(data: nil, didSelectInDocument: false, state: &state)
_select(key: nil, didSelectInDocument: false, state: &state)
} else {
// Deselect selected annotations during editing
state.selectedAnnotationsDuringEditing = []
Expand Down Expand Up @@ -365,12 +379,11 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
}
}

private func _select(data: (String, CGRect?)?, didSelectInDocument: Bool, state: inout HtmlEpubReaderState) {
guard data?.0 != state.selectedAnnotationKey else { return }
private func _select(key: String?, didSelectInDocument: Bool, state: inout HtmlEpubReaderState) {
guard key != state.selectedAnnotationKey else { return }

if let existing = state.selectedAnnotationKey {
add(updatedAnnotationKey: existing, state: &state)

if state.selectedAnnotationCommentActive {
state.selectedAnnotationCommentActive = false
state.changes.insert(.activeComment)
Expand All @@ -379,21 +392,17 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro

state.changes.insert(.selection)

guard let (key, rect) = data else {
guard let key else {
state.selectedAnnotationKey = nil
state.selectedAnnotationRect = nil
return
}

state.selectedAnnotationKey = key
state.selectedAnnotationRect = rect

if !didSelectInDocument {
state.focusDocumentKey = key
} else {
state.focusSidebarKey = key
}

add(updatedAnnotationKey: key, state: &state)

func add(updatedAnnotationKey key: String, state: inout HtmlEpubReaderState) {
Expand Down Expand Up @@ -792,7 +801,7 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro

// Update selection
if selectionDeleted {
_select(data: nil, didSelectInDocument: true, state: &state)
_select(key: nil, didSelectInDocument: true, state: &state)
}

// Disable sidebar editing if there are no results
Expand Down Expand Up @@ -829,11 +838,6 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro

extension RItem {
fileprivate var htmlEpubAnnotation: (HtmlEpubAnnotation, [String: Any])? {
let tags = Array(tags.map({ typedTag in
let color: String? = (typedTag.tag?.color ?? "").isEmpty ? nil : typedTag.tag?.color
return Tag(name: typedTag.tag?.name ?? "", color: color ?? "")
}))

var type: AnnotationType?
var position: [String: Any] = [:]
var text: String?
Expand All @@ -845,11 +849,8 @@ extension RItem {

for field in fields {
switch (field.key, field.baseKey) {
case (FieldKeys.Item.Annotation.Position.htmlEpubType, FieldKeys.Item.Annotation.position):
position[FieldKeys.Item.Annotation.Position.htmlEpubType] = field.value

case (FieldKeys.Item.Annotation.Position.htmlEpubValue, FieldKeys.Item.Annotation.position):
position[FieldKeys.Item.Annotation.Position.htmlEpubValue] = field.value
case (_, FieldKeys.Item.Annotation.position):
position[field.key] = field.value

case (FieldKeys.Item.Annotation.type, nil):
type = AnnotationType(rawValue: field.value)
Expand Down Expand Up @@ -882,7 +883,12 @@ extension RItem {
return nil
}

let json: [String: Any] = [
let tags = Array(tags.map({ typedTag in
let color: String? = (typedTag.tag?.color ?? "").isEmpty ? nil : typedTag.tag?.color
return Tag(name: typedTag.tag?.name ?? "", color: color ?? "")
}))

var json: [String: Any] = [
"id": key,
"dateCreated": DateFormatter.iso8601WithFractionalSeconds.string(from: dateAdded),
"dateModified": DateFormatter.iso8601WithFractionalSeconds.string(from: dateModified),
Expand All @@ -896,6 +902,10 @@ extension RItem {
"position": position,
"tags": tags.map({ ["name": $0.name, "color": $0.color] })
]
for (key, value) in unknown {
json[key] = value
}

let annotation = HtmlEpubAnnotation(
key: key,
type: type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class HtmlEpubDocumentViewController: UIViewController {

switch event {
case "onInitialized":
self.viewModel.process(action: .loadDocument)
viewModel.process(action: .loadDocument)

case "onSaveAnnotations":
guard let params = data["params"] as? [String: Any] else {
Expand All @@ -225,7 +225,7 @@ class HtmlEpubDocumentViewController: UIViewController {
}

if params.isEmpty {
viewModel.process(action: .deselectSelectedAnnotation)
viewModel.process(action: .hideAnnotationPopover)
return
}

Expand All @@ -236,7 +236,18 @@ class HtmlEpubDocumentViewController: UIViewController {

let navigationBarInset = (parentDelegate?.statusBarHeight ?? 0) + (parentDelegate?.navigationBarHeight ?? 0)
let rect = CGRect(x: rectArray[0], y: rectArray[1] + navigationBarInset, width: rectArray[2] - rectArray[0], height: rectArray[3] - rectArray[1])
viewModel.process(action: .selectAnnotationFromDocument(key: key, rect: rect))
viewModel.process(action: .showAnnotationPopover(key: key, rect: rect))

case "onSelectAnnotations":
guard let params = data["params"] as? [String: Any], let ids = params["ids"] as? [String] else {
DDLogWarn("HtmlEpubDocumentViewController: event \(event) missing params - \(message)")
return
}
if let key = ids.first {
viewModel.process(action: .selectAnnotationFromDocument(key: key))
} else {
viewModel.process(action: .deselectSelectedAnnotation)
}

case "onChangeViewState":
guard let params = data["params"] as? [String: Any] else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,13 @@ class HtmlEpubReaderViewController: UIViewController, ParentWithSidebarControlle
navigationController?.overrideUserInterfaceStyle = state.settings.appearance.userInterfaceStyle
}

handleAnnotationPopover()
if state.changes.contains(.popover) {
if let key = state.annotationPopoverKey, let rect = state.annotationPopoverRect {
showPopover(forKey: key, rect: rect)
} else {
hidePopover()
}
}

func select(activeTool tool: AnnotationTool?) {
if let tool = activeAnnotationTool {
Expand All @@ -290,21 +296,22 @@ class HtmlEpubReaderViewController: UIViewController, ParentWithSidebarControlle
func show(error: HtmlEpubReaderState.Error) {
}

func handleAnnotationPopover() {
if let key = state.selectedAnnotationKey {
if !isSidebarVisible, let rect = state.selectedAnnotationRect {
let observable = coordinatorDelegate?.showAnnotationPopover(
viewModel: viewModel,
sourceRect: rect,
popoverDelegate: self,
userInterfaceStyle: viewModel.state.settings.appearance.userInterfaceStyle
)
observe(key: key, popoverObservable: observable)
}
} else if navigationController?.presentedViewController is AnnotationPopover ||
(navigationController?.presentedViewController as? UINavigationController)?.topViewController is AnnotationPopover {
navigationController?.dismiss(animated: true)
}
func showPopover(forKey key: String, rect: CGRect) {
guard !isSidebarVisible else { return }
let observable = coordinatorDelegate?.showAnnotationPopover(
viewModel: viewModel,
sourceRect: rect,
popoverDelegate: self,
userInterfaceStyle: viewModel.state.settings.appearance.userInterfaceStyle
)
observe(key: key, popoverObservable: observable)
}

func hidePopover() {
guard navigationController?.presentedViewController is AnnotationPopover ||
(navigationController?.presentedViewController as? UINavigationController)?.topViewController is AnnotationPopover
else { return }
navigationController?.dismiss(animated: true)
}

func observe(key: String, popoverObservable observable: PublishSubject<AnnotationPopoverState>?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,9 @@ extension HtmlEpubSidebarViewController: UITableViewDelegate {
func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
guard let key = dataSource.itemIdentifier(for: indexPath) else { return }
if viewModel.state.sidebarEditingEnabled {
viewModel.process(action: .selectAnnotationDuringEditing(key))
viewModel.process(action: .selectAnnotationDuringEditing(key: key))
} else {
viewModel.process(action: .selectAnnotationFromSidebar(key))
viewModel.process(action: .selectAnnotationFromSidebar(key: key))
}
}

Expand Down

0 comments on commit d0b4813

Please sign in to comment.