From 64d6ff45e17a6bf505138fb6edcdaa2eae071040 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 21 Nov 2022 22:20:23 +0800 Subject: [PATCH 01/24] Add a stub test --- packages/rule-handler/test/stub.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 packages/rule-handler/test/stub.test.ts diff --git a/packages/rule-handler/test/stub.test.ts b/packages/rule-handler/test/stub.test.ts new file mode 100644 index 000000000..24ad25c8f --- /dev/null +++ b/packages/rule-handler/test/stub.test.ts @@ -0,0 +1,8 @@ +import 'mocha' +import { expect } from 'chai' + +describe('stub test', () => { + it('should pass', () => { + expect(true).to.be.true + }) +}) From c74911ea86750a9261d234385cd6174e1d33c472 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 16 Nov 2022 13:29:59 +0800 Subject: [PATCH 02/24] Improve how we handle loading the initial items on iOS --- .../App/Views/Home/HomeFeedViewModel.swift | 73 ++++++++++++------- .../Sources/Models/DataModels/FeedItem.swift | 10 +++ .../Public/LinkedItemContentLoading.swift | 11 ++- .../Public/LinkedItemLoading.swift | 16 ++-- 4 files changed, 73 insertions(+), 37 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift index 55ff72e95..418db4b50 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift @@ -91,33 +91,45 @@ import Views items.insert(item, at: 0) } - func loadItems(dataService: DataService, audioController: AudioController, isRefresh: Bool) async { - let syncStartTime = Date() - let thisSearchIdx = searchIdx - searchIdx += 1 - - isLoading = true - showLoadingBar = true - + func loadCurrentViewer(dataService: DataService) async { // Cache the viewer if dataService.currentViewer == nil { - Task { _ = try? await dataService.fetchViewer() } + _ = try? await dataService.fetchViewer() } + } - // Fetch labels if none are available locally + func loadLabels(dataService: DataService) async { let fetchRequest: NSFetchRequest = LinkedItemLabel.fetchRequest() fetchRequest.fetchLimit = 1 if (try? dataService.viewContext.count(for: fetchRequest)) == 0 { _ = try? await dataService.labels() } + } - // Sync items if necessary + func syncItems(dataService: DataService, syncStartTime: Date) async { let lastSyncDate = dateFormatter.date(from: lastItemSyncTime) ?? Date(timeIntervalSinceReferenceDate: 0) let syncResult = try? await dataService.syncLinkedItems(since: lastSyncDate, cursor: nil) if syncResult != nil { lastItemSyncTime = dateFormatter.string(from: syncStartTime) } + // If possible start prefetching new pages in the background + if let itemIDs = syncResult?.updatedItemIDs, + let username = dataService.currentViewer?.username, + itemIDs.count > 0 { + Task { + await dataService.prefetchPages(itemIDs: itemIDs.map { $0.uriRepresentation().absoluteString }, username: username) + } + } + } + + func loadSearchQuery(dataService: DataService, isRefresh: Bool) async { + let thisSearchIdx = searchIdx + searchIdx += 1 + + if thisSearchIdx > 0, thisSearchIdx <= receivedIdx { + return + } let queryResult = try? await dataService.loadLinkedItems( limit: 10, @@ -125,15 +137,6 @@ import Views cursor: isRefresh ? nil : cursor ) - // Search results aren't guaranteed to return in order so this - // will discard old results that are returned while a user is typing. - // For example if a user types 'Canucks', often the search results - // for 'C' are returned after 'Canucks' because it takes the backend - // much longer to compute. - if thisSearchIdx > 0, thisSearchIdx <= receivedIdx { - return - } - if let queryResult = queryResult { let newItems: [LinkedItem] = { var itemObjects = [LinkedItem]() @@ -159,17 +162,33 @@ import Views cursor = queryResult.cursor if let username = dataService.currentViewer?.username { await dataService.prefetchPages(itemIDs: newItems.map(\.unwrappedID), username: username) - // Only preload the first item in the list. We are doing this during the beta - // because it will kick off the user's future items being automatically transcribed. - // This happens because when an article is saved, we check if the user has a recent - // listen. If they do, we will automatically transcribe their message. - if let first = newItems.filter({ !$0.isPDF }).first?.id { - _ = await audioController.preload(itemIDs: [first]) - } } } else { updateFetchController(dataService: dataService) } + } + + func loadItems(dataService: DataService, audioController _: AudioController, isRefresh: Bool) async { + let syncStartTime = Date() + let start = CFAbsoluteTimeGetCurrent() + + isLoading = true + showLoadingBar = true + + _ = await [ + loadCurrentViewer(dataService: dataService), + loadLabels(dataService: dataService), + syncItems(dataService: dataService, syncStartTime: syncStartTime) + ] + print("fetched all prelim data", CFAbsoluteTimeGetCurrent() - start) + + if searchTerm.replacingOccurrences(of: " ", with: "").isEmpty { + updateFetchController(dataService: dataService) + } else { + print("PERFORMING SEARCH FOR ITEMS") + await loadSearchQuery(dataService: dataService, isRefresh: isRefresh) + } + print("loaded the linked items", CFAbsoluteTimeGetCurrent() - start) isLoading = false showLoadingBar = false diff --git a/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift b/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift index 18a464152..8d2e54aa6 100644 --- a/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift +++ b/apple/OmnivoreKit/Sources/Models/DataModels/FeedItem.swift @@ -12,6 +12,16 @@ public struct LinkedItemQueryResult { } } +public struct LinkedItemSyncResult { + public let updatedItemIDs: [String] + public let cursor: String? + + public init(updatedItemIDs: [String], cursor: String?) { + self.updatedItemIDs = updatedItemIDs + self.cursor = cursor + } +} + public struct LinkedItemAudioProperties { public let itemID: String public let objectID: NSManagedObjectID diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift index 3af0f0737..a4bc86ea1 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift @@ -5,9 +5,14 @@ import Utils public extension DataService { func prefetchPages(itemIDs: [String], username: String) async { - // TODO: make this concurrent - for itemID in itemIDs { - await prefetchPage(pendingLink: PendingLink(itemID: itemID, retryCount: 1), username: username) + await withTaskGroup(of: Void.self) { group in + print(" PREFETCHING PAGES: ", itemIDs) + for itemID in itemIDs { + group.addTask { + await self.prefetchPage(pendingLink: PendingLink(itemID: itemID, retryCount: 1), username: username) + } + } + await group.waitForAll() } } diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift index f1e7b517d..ea5d8ff0e 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift @@ -14,8 +14,8 @@ public extension DataService { func syncLinkedItems( since date: Date, cursor: String?, - previousQueryResult: LinkedItemQueryResult? = nil - ) async throws -> LinkedItemQueryResult { + previousQueryResult: LinkedItemSyncResult? = nil + ) async throws -> LinkedItemSyncResult? { if previousQueryResult == nil { // Send offline changes to server before fetching items // only on the first call of this function @@ -26,16 +26,18 @@ public extension DataService { LinkedItem.deleteItems(ids: fetchResult.deletedItemIDs, context: backgroundContext) - guard let itemIDs = fetchResult.items.persist(context: backgroundContext) else { + if fetchResult.items.persist(context: backgroundContext) == nil { throw BasicError.message(messageText: "CoreData error") } - let result = LinkedItemQueryResult( - itemIDs: itemIDs + (previousQueryResult?.itemIDs ?? []), + + let prev = previousQueryResult?.updatedItemIDs ?? [] + let result = LinkedItemSyncResult( + updatedItemIDs: prev + fetchResult.items.map { $0.id }, cursor: fetchResult.cursor ) - if fetchResult.hasMoreItems, (previousQueryResult?.itemIDs.count ?? 0) < 200 { + if fetchResult.hasMoreItems, (previousQueryResult?.updatedItemIDs.count ?? 0) < 200 { return try await syncLinkedItems( since: date, cursor: fetchResult.cursor, @@ -58,7 +60,7 @@ public extension DataService { cursor: String? ) async throws -> LinkedItemQueryResult { // Send offline changes to server before fetching items - try? await syncOfflineItemsWithServerIfNeeded() + // try? await syncOfflineItemsWithServerIfNeeded() let fetchResult = try await fetchLinkedItems(limit: limit, searchQuery: searchQuery, cursor: cursor) From 5e4095f47ee29542d12975a3129e46abdf6b05d4 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 16 Nov 2022 14:10:32 +0800 Subject: [PATCH 03/24] Remove debug, fix formatting --- .../Sources/App/Views/Home/HomeFeedViewModel.swift | 5 +++-- .../DataService/Public/LinkedItemContentLoading.swift | 1 - .../Services/DataService/Public/LinkedItemLoading.swift | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift index 418db4b50..7852265a6 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift @@ -116,9 +116,10 @@ import Views // If possible start prefetching new pages in the background if let itemIDs = syncResult?.updatedItemIDs, let username = dataService.currentViewer?.username, - itemIDs.count > 0 { + itemIDs.count > 0 + { Task { - await dataService.prefetchPages(itemIDs: itemIDs.map { $0.uriRepresentation().absoluteString }, username: username) + await dataService.prefetchPages(itemIDs: itemIDs, username: username) } } } diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift index a4bc86ea1..e755f1510 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemContentLoading.swift @@ -6,7 +6,6 @@ import Utils public extension DataService { func prefetchPages(itemIDs: [String], username: String) async { await withTaskGroup(of: Void.self) { group in - print(" PREFETCHING PAGES: ", itemIDs) for itemID in itemIDs { group.addTask { await self.prefetchPage(pendingLink: PendingLink(itemID: itemID, retryCount: 1), username: username) diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift index ea5d8ff0e..70806fdfe 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Public/LinkedItemLoading.swift @@ -30,10 +30,9 @@ public extension DataService { throw BasicError.message(messageText: "CoreData error") } - let prev = previousQueryResult?.updatedItemIDs ?? [] let result = LinkedItemSyncResult( - updatedItemIDs: prev + fetchResult.items.map { $0.id }, + updatedItemIDs: prev + fetchResult.items.map(\.id), cursor: fetchResult.cursor ) From 60a663e9bb79d03f8344d0c1f4e0c498135da4c0 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 16 Nov 2022 15:33:22 +0800 Subject: [PATCH 04/24] Make sure deleted highlights are synced to iOS --- .../Sources/Services/DataService/ContentLoading.swift | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apple/OmnivoreKit/Sources/Services/DataService/ContentLoading.swift b/apple/OmnivoreKit/Sources/Services/DataService/ContentLoading.swift index db17d6e9c..4de282df4 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/ContentLoading.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/ContentLoading.swift @@ -110,7 +110,15 @@ extension DataService { let highlightObjects = articleProps.highlights.map { $0.asManagedObject(context: self.backgroundContext) } - linkedItem.addToHighlights(NSSet(array: highlightObjects)) + + let unsyncedHighlights = existingItem?.highlights?.filter { highlight in + if let highlight = highlight as? Highlight, highlight.serverSyncStatus == ServerSyncStatus.isNSync.rawValue { + return false + } + return true + }.compactMap { $0 as? Highlight } ?? [] + + linkedItem.highlights = NSSet(array: highlightObjects + unsyncedHighlights) linkedItem.htmlContent = articleProps.htmlContent linkedItem.id = articleProps.item.id linkedItem.state = articleProps.item.state.rawValue From c523010427a5e45ff166e4979fd438b5f446e094 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 17 Nov 2022 14:33:05 +0800 Subject: [PATCH 05/24] Prefetch content and update share extension info after saving items --- .../Share/ShareExtensionViewModel.swift | 13 +++++++++++++ .../App/Views/Labels/LabelsMasonaryView.swift | 4 ---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift index b3f2fa632..517e2a746 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift @@ -172,6 +172,15 @@ public class ShareExtensionViewModel: ObservableObject { } updateStatusOnMain(requestId: newRequestID, newStatus: .synced) + + // Prefetch the newly saved content + if let itemID = newRequestID, + let currentViewer = services.dataService.currentViewer?.username, + (try? await services.dataService.loadArticleContentWithRetries(itemID: itemID, username: currentViewer)) != nil + { + updateStatusOnMain(requestId: requestId, newStatus: .saved, objectID: linkedItemObjectID) + } + return true } @@ -184,6 +193,10 @@ public class ShareExtensionViewModel: ObservableObject { if let objectID = objectID { self.linkedItem = self.services.dataService.viewContext.object(with: objectID) as? LinkedItem + if let title = self.linkedItem?.title { + self.title = title + } + self.url = self.linkedItem?.pageURLString } } } diff --git a/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsMasonaryView.swift b/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsMasonaryView.swift index 1c3139bbb..61d2dc785 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsMasonaryView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsMasonaryView.swift @@ -77,10 +77,6 @@ struct LabelsMasonaryView: View { } private func item(for item: (label: LinkedItemLabel, selected: Bool)) -> some View { - if item.selected { - print(" -- SELECTED LABEL", item.label.name) - } - print("GETTING ITERATION", iteration) let chip = TextChip(feedItemLabel: item.label, negated: false, checked: item.selected) { chip in onLabelTap(item.label, chip) } From 35a9de657a1e78a14661163a4d9085553b8ac48e Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 17 Nov 2022 15:23:18 +0800 Subject: [PATCH 06/24] Use a task group and a detached task to prefetch in the background --- .../App/Views/Home/HomeFeedViewModel.swift | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift index 7852265a6..1a8f79580 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift @@ -110,15 +110,17 @@ import Views func syncItems(dataService: DataService, syncStartTime: Date) async { let lastSyncDate = dateFormatter.date(from: lastItemSyncTime) ?? Date(timeIntervalSinceReferenceDate: 0) let syncResult = try? await dataService.syncLinkedItems(since: lastSyncDate, cursor: nil) + if syncResult != nil { lastItemSyncTime = dateFormatter.string(from: syncStartTime) } + // If possible start prefetching new pages in the background if let itemIDs = syncResult?.updatedItemIDs, let username = dataService.currentViewer?.username, itemIDs.count > 0 { - Task { + Task.detached(priority: .background) { await dataService.prefetchPages(itemIDs: itemIDs, username: username) } } @@ -176,20 +178,18 @@ import Views isLoading = true showLoadingBar = true - _ = await [ - loadCurrentViewer(dataService: dataService), - loadLabels(dataService: dataService), - syncItems(dataService: dataService, syncStartTime: syncStartTime) - ] - print("fetched all prelim data", CFAbsoluteTimeGetCurrent() - start) + await withTaskGroup(of: Void.self) { group in + group.addTask { await self.loadCurrentViewer(dataService: dataService) } + group.addTask { await self.loadLabels(dataService: dataService) } + group.addTask { await self.syncItems(dataService: dataService, syncStartTime: syncStartTime) } + await group.waitForAll() + } if searchTerm.replacingOccurrences(of: " ", with: "").isEmpty { updateFetchController(dataService: dataService) } else { - print("PERFORMING SEARCH FOR ITEMS") await loadSearchQuery(dataService: dataService, isRefresh: isRefresh) } - print("loaded the linked items", CFAbsoluteTimeGetCurrent() - start) isLoading = false showLoadingBar = false From 8c6798a4670175ceb970e7516d80f1926588c1eb Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 17 Nov 2022 15:37:02 +0800 Subject: [PATCH 07/24] Remove the preview image from the share extension --- .../Share/ShareExtensionViewModel.swift | 16 +---- .../Share/Views/ShareExtensionView.swift | 65 ------------------- .../Sources/Models/PageScrapePayload.swift | 9 ++- .../Services/DataService/DataService.swift | 3 +- 4 files changed, 7 insertions(+), 86 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift index 517e2a746..54d0ce76e 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift @@ -9,7 +9,6 @@ public class ShareExtensionViewModel: ObservableObject { @Published public var status: ShareExtensionStatus = .processing @Published public var title: String = "" @Published public var url: String? - @Published public var iconURL: String? @Published public var linkedItem: LinkedItem? @Published public var requestId = UUID().uuidString.lowercased() @Published var debugText: String? @@ -88,26 +87,15 @@ public class ShareExtensionViewModel: ObservableObject { let hostname = URL(string: payload.url)?.host ?? "" switch payload.contentType { - case let .html(html: _, title: title, iconURL: iconURL): + case let .html(html: _, title: title): self.title = title ?? "" - self.iconURL = iconURL self.url = hostname case .none: self.url = hostname self.title = payload.url - if var url = url { - url.path = "/favicon.ico" - self.iconURL = url.url?.absoluteString - } case let .pdf(localUrl: localUrl): self.url = hostname self.title = PDFUtils.titleFromPdfFile(localUrl.absoluteString) - Task { - let localThumbnail = try await PDFUtils.createThumbnailFor(inputUrl: localUrl) - DispatchQueue.main.async { - self.iconURL = localThumbnail?.absoluteString - } - } } } @@ -155,7 +143,7 @@ public class ShareExtensionViewModel: ObservableObject { localPdfURL: localUrl, url: pageScrapePayload.url ) - case let .html(html, title, _): + case let .html(html, title): newRequestID = try await services.dataService.createPage( id: requestId, originalHtml: html, diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift index c4a0ea82d..240ecf704 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift @@ -81,71 +81,6 @@ public struct ShareExtensionView: View { return nil } - public var previewCard: some View { - HStack { - if let iconURLStr = viewModel.iconURL, let iconURL = URL(string: iconURLStr) { - if !iconURL.isFileURL { - AsyncImage( - url: iconURL, - content: { image in - image - .resizable() - .aspectRatio(contentMode: .fill) - .frame(width: 61, height: 61) - .clipped() - }, - placeholder: { - Color.appButtonBackground - .aspectRatio(contentMode: .fill) - .frame(width: 61, height: 61) - } - ) - } else { - if let localImage = localImage(from: iconURL) { - localImage - .resizable() - .aspectRatio(contentMode: .fill) - .frame(width: 61, height: 61) - .clipped() - } else { - Color.appButtonBackground - .aspectRatio(contentMode: .fill) - .frame(width: 61, height: 61) - } - } - } else { - Color.appButtonBackground - .aspectRatio(contentMode: .fill) - .frame(width: 61, height: 61) - } - - VStack(alignment: .leading) { - Text(viewModel.title ?? "") - .lineLimit(1) - .foregroundColor(.appGrayTextContrast) - .font(Font.system(size: 15, weight: .semibold)) - Text(viewModel.url ?? "") - .lineLimit(1) - .foregroundColor(.appGrayText) - .font(Font.system(size: 12, weight: .regular)) - } - Spacer() - VStack { - Spacer() - Image(systemName: cloudIconName) - .resizable() - .aspectRatio(contentMode: .fill) - .frame(width: 12, height: 12, alignment: .trailing) - .foregroundColor(cloudIconColor) - // .padding(.trailing, 6) - .padding(EdgeInsets(top: 0, leading: 0, bottom: 8, trailing: 8)) - } - } - .background(Color.appButtonBackground) - .frame(maxWidth: .infinity, maxHeight: 61) - .cornerRadius(8) - } - var isSynced: Bool { switch viewModel.status { case .synced: diff --git a/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift b/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift index c0cc1ff94..b21265db9 100644 --- a/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift +++ b/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift @@ -11,7 +11,7 @@ let URLREGEX = #"[(http(s)?):\/\/(www\.)?a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6} public struct PageScrapePayload { public enum ContentType { case none - case html(html: String, title: String?, iconURL: String?) + case html(html: String, title: String?) case pdf(localUrl: URL) } @@ -33,9 +33,9 @@ public struct PageScrapePayload { self.contentType = .pdf(localUrl: localUrl) } - init(url: String, title: String?, html: String, iconURL: String? = nil) { + init(url: String, title: String?, html: String) { self.url = url - self.contentType = .html(html: html, title: title, iconURL: iconURL) + self.contentType = .html(html: html, title: title) } } @@ -302,7 +302,6 @@ private extension PageScrapePayload { guard let url = results?["url"] as? String else { return nil } let html = results?["originalHTML"] as? String let title = results?["title"] as? String - let iconURL = results?["iconURL"] as? String let contentType = results?["contentType"] as? String // If we were not able to capture any HTML, treat this as a URL and @@ -318,7 +317,7 @@ private extension PageScrapePayload { } if let html = html { - return PageScrapePayload(url: url, title: title, html: html, iconURL: iconURL) + return PageScrapePayload(url: url, title: title, html: html) } return PageScrapePayload(url: url) diff --git a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift index 4757256b3..072d777b1 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift @@ -177,10 +177,9 @@ public final class DataService: ObservableObject { linkedItem.contentReader = "PDF" linkedItem.tempPDFURL = localUrl linkedItem.title = PDFUtils.titleFromPdfFile(pageScrape.url) - case let .html(html: html, title: title, iconURL: iconURL): + case let .html(html: html, title: title): linkedItem.contentReader = "WEB" linkedItem.originalHtml = html - linkedItem.imageURLString = iconURL linkedItem.title = title ?? PDFUtils.titleFromPdfFile(pageScrape.url) case .none: linkedItem.contentReader = "WEB" From 75a5e9fb0307a4e885f37eae5a2e1671d8d2a6df Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 17 Nov 2022 18:20:34 +0800 Subject: [PATCH 08/24] MOve the lastItemSyncTime into the dataservice so we can reset the instance value --- .../Sources/App/Views/Home/HomeFeedViewModel.swift | 8 ++------ .../Sources/Services/DataService/DataService.swift | 10 ++++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift index 1a8f79580..d78ec3692 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift @@ -36,10 +36,6 @@ import Views @AppStorage(UserDefaultKey.lastSelectedLinkedItemFilter.rawValue) var appliedFilter = LinkedItemFilter.inbox.rawValue - @AppStorage(UserDefaultKey.lastItemSyncTime.rawValue) var lastItemSyncTime = DateFormatter.formatterISO8601.string( - from: Date(timeIntervalSinceReferenceDate: 0) - ) - func handleReaderItemNotification(objectID: NSManagedObjectID, dataService: DataService) { // Pop the current selected item if needed if selectedItem != nil, selectedItem?.objectID != objectID { @@ -108,11 +104,11 @@ import Views } func syncItems(dataService: DataService, syncStartTime: Date) async { - let lastSyncDate = dateFormatter.date(from: lastItemSyncTime) ?? Date(timeIntervalSinceReferenceDate: 0) + let lastSyncDate = dateFormatter.date(from: dataService.lastItemSyncTime) ?? Date(timeIntervalSinceReferenceDate: 0) let syncResult = try? await dataService.syncLinkedItems(since: lastSyncDate, cursor: nil) if syncResult != nil { - lastItemSyncTime = dateFormatter.string(from: syncStartTime) + dataService.lastItemSyncTime = dateFormatter.string(from: syncStartTime) } // If possible start prefetching new pages in the background diff --git a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift index 072d777b1..92628976e 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift @@ -4,6 +4,7 @@ import Foundation import Models import OSLog import QuickLookThumbnailing +import SwiftUI import Utils #if os(iOS) @@ -28,6 +29,10 @@ public final class DataService: ObservableObject { persistentContainer.viewContext } + @AppStorage(UserDefaultKey.lastItemSyncTime.rawValue) public var lastItemSyncTime = DateFormatter.formatterISO8601.string( + from: Date(timeIntervalSinceReferenceDate: 0) + ) + public init(appEnvironment: AppEnvironment, networker: Networker) { self.appEnvironment = appEnvironment self.networker = networker @@ -96,10 +101,7 @@ public final class DataService: ObservableObject { } public func resetCoreData() { - UserDefaults.standard.set( - DateFormatter.formatterISO8601.string(from: Date(timeIntervalSinceReferenceDate: 0)), - forKey: UserDefaultKey.lastItemSyncTime.rawValue - ) + lastItemSyncTime = DateFormatter.formatterISO8601.string(from: Date(timeIntervalSinceReferenceDate: 0)) clearCoreData() From 062215afa499b59983f6cd715dc9da440461aab7 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Fri, 18 Nov 2022 11:39:22 +0800 Subject: [PATCH 09/24] If there is no search term but there is an applied filter, perform a search after updating from the synced items This fixes issues with filters that have very few items in them not showing historical data if the filter is applied. --- .../OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift index d78ec3692..fb32aa425 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Home/HomeFeedViewModel.swift @@ -183,6 +183,9 @@ import Views if searchTerm.replacingOccurrences(of: " ", with: "").isEmpty { updateFetchController(dataService: dataService) + if appliedFilter != LinkedItemFilter.inbox.rawValue { + await loadSearchQuery(dataService: dataService, isRefresh: isRefresh) + } } else { await loadSearchQuery(dataService: dataService, isRefresh: isRefresh) } From 2e7b7c1bb5c83538d4ed18c2e81e99eec08fe6da Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Fri, 18 Nov 2022 13:06:22 +0800 Subject: [PATCH 10/24] Make warning message on reader match library --- .../Views/WebReader/WebReaderContainer.swift | 5 ++-- .../Sources/ShareExtension/ShareExtension.js | 23 ++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift index 74f1238e6..a7848a50d 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift @@ -257,8 +257,9 @@ struct WebReaderContainerView: View { .frame(height: readerViewNavBarHeight * navBarVisibilityRatio) .opacity(navBarVisibilityRatio) .background(Color.systemBackground) - .alert("Are you sure?", isPresented: $showDeleteConfirmation) { - Button("Remove Link", role: .destructive) { + .alert("Are you sure you want to remove this item? All associated notes and highlights will be deleted.", + isPresented: $showDeleteConfirmation) { + Button("Remove Item", role: .destructive) { Snackbar.show(message: "Link removed") dataService.removeLink(objectID: item.objectID) #if os(iOS) diff --git a/apple/Sources/ShareExtension/ShareExtension.js b/apple/Sources/ShareExtension/ShareExtension.js index 7474b0ba7..22e7ddda5 100644 --- a/apple/Sources/ShareExtension/ShareExtension.js +++ b/apple/Sources/ShareExtension/ShareExtension.js @@ -11,13 +11,34 @@ function iconURL() { } ShareExtension.prototype = { + getHighlightHTML: function() { + try { + var sel = window.getSelection() + return (function () { + var html = ""; + var sel = window.getSelection(); + if (sel.rangeCount) { + var container = document.createElement("div"); + for (var i = 0, len = sel.rangeCount; i < len; ++i) { + container.appendChild(sel.getRangeAt(i).cloneContents()); + } + html = container.innerHTML; + } + return html; + })() + } catch { + + } + return null + }, run: function(arguments) { arguments.completionFunction({ 'url': window.location.href, 'title': document.title.toString(), 'iconURL': iconURL(), 'contentType': document.contentType, - 'originalHTML': new XMLSerializer().serializeToString(document) + 'originalHTML': new XMLSerializer().serializeToString(document), + 'highlightHTML': this.getHighlightHTML() }); } }; From bf97194208e75cf5c6e8a445f22df5b71ab1c5fb Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Fri, 18 Nov 2022 13:07:00 +0800 Subject: [PATCH 11/24] Create a highlight of the selected text when a user saves --- .../Share/ShareExtensionViewModel.swift | 4 +- .../Sources/Models/PageScrapePayload.swift | 26 +++++++++-- .../Services/DataService/DataService.swift | 2 +- .../Sources/ShareExtension/ShareExtension.js | 45 ++++++++++++------- 4 files changed, 54 insertions(+), 23 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift index 54d0ce76e..e0c075476 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift @@ -87,7 +87,7 @@ public class ShareExtensionViewModel: ObservableObject { let hostname = URL(string: payload.url)?.host ?? "" switch payload.contentType { - case let .html(html: _, title: title): + case let .html(html: _, title: title, _): self.title = title ?? "" self.url = hostname case .none: @@ -143,7 +143,7 @@ public class ShareExtensionViewModel: ObservableObject { localPdfURL: localUrl, url: pageScrapePayload.url ) - case let .html(html, title): + case let .html(html, title, _): newRequestID = try await services.dataService.createPage( id: requestId, originalHtml: html, diff --git a/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift b/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift index b21265db9..aafd4eb4c 100644 --- a/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift +++ b/apple/OmnivoreKit/Sources/Models/PageScrapePayload.swift @@ -8,11 +8,26 @@ import UniformTypeIdentifiers let URLREGEX = #"[(http(s)?):\/\/(www\.)?a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)"# +public struct HighlightData { + public let highlightHTML: String + public let highlightText: String + + public static func make(dict: NSDictionary?) -> HighlightData? { + if let dict = dict, + let highlightHTML = dict["highlightHTML"] as? String, + let highlightText = dict["highlightText"] as? String + { + return HighlightData(highlightHTML: highlightHTML, highlightText: highlightText) + } + return nil + } +} + public struct PageScrapePayload { public enum ContentType { case none - case html(html: String, title: String?) case pdf(localUrl: URL) + case html(html: String, title: String?, highlightData: HighlightData?) } public let url: String @@ -33,9 +48,9 @@ public struct PageScrapePayload { self.contentType = .pdf(localUrl: localUrl) } - init(url: String, title: String?, html: String) { + init(url: String, title: String?, html: String, highlightData: HighlightData?) { self.url = url - self.contentType = .html(html: html, title: title) + self.contentType = .html(html: html, title: title, highlightData: highlightData) } } @@ -317,7 +332,10 @@ private extension PageScrapePayload { } if let html = html { - return PageScrapePayload(url: url, title: title, html: html) + return PageScrapePayload(url: url, + title: title, + html: html, + highlightData: HighlightData.make(dict: results)) } return PageScrapePayload(url: url) diff --git a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift index 92628976e..3c640c914 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift @@ -179,7 +179,7 @@ public final class DataService: ObservableObject { linkedItem.contentReader = "PDF" linkedItem.tempPDFURL = localUrl linkedItem.title = PDFUtils.titleFromPdfFile(pageScrape.url) - case let .html(html: html, title: title): + case let .html(html: html, title: title, highlightData: _): linkedItem.contentReader = "WEB" linkedItem.originalHtml = html linkedItem.title = title ?? PDFUtils.titleFromPdfFile(pageScrape.url) diff --git a/apple/Sources/ShareExtension/ShareExtension.js b/apple/Sources/ShareExtension/ShareExtension.js index 22e7ddda5..15aa00efc 100644 --- a/apple/Sources/ShareExtension/ShareExtension.js +++ b/apple/Sources/ShareExtension/ShareExtension.js @@ -11,34 +11,47 @@ function iconURL() { } ShareExtension.prototype = { - getHighlightHTML: function() { + markHighlightSelection: () => { try { - var sel = window.getSelection() - return (function () { - var html = ""; - var sel = window.getSelection(); - if (sel.rangeCount) { - var container = document.createElement("div"); - for (var i = 0, len = sel.rangeCount; i < len; ++i) { - container.appendChild(sel.getRangeAt(i).cloneContents()); - } - html = container.innerHTML; + const sel = window.getSelection(); + if (sel.rangeCount) { + const range = sel.getRangeAt(0) + const endMarker = document.createElement("span") + const startMarker = document.createElement("span") + endMarker.setAttribute("data-omnivore-highlight-end", true) + startMarker.setAttribute("data-omnivore-highlight-start", true) + + var container = document.createElement("div"); + for (var i = 0, len = sel.rangeCount; i < len; ++i) { + container.appendChild(sel.getRangeAt(i).cloneContents()); } - return html; - })() - } catch { - + + const endRange = range.cloneRange() + endRange.collapse(false) + endRange.insertNode(endMarker) + + range.insertNode(startMarker) + + return { + highlightHTML: container.innerHTML, + highlightText: container.innerText + } + } + } catch(error) { + console.log("ERROR", error) } return null }, run: function(arguments) { + const highlightData = this.markHighlightSelection() + arguments.completionFunction({ 'url': window.location.href, 'title': document.title.toString(), 'iconURL': iconURL(), 'contentType': document.contentType, 'originalHTML': new XMLSerializer().serializeToString(document), - 'highlightHTML': this.getHighlightHTML() + ...highlightData }); } }; From 2ec4328a90d36c3885a25549f0be91cf4d11b288 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Fri, 18 Nov 2022 13:39:09 +0800 Subject: [PATCH 12/24] Linting --- .../Sources/App/Views/WebReader/WebReaderContainer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift index a7848a50d..f4bc6bc11 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift @@ -258,7 +258,7 @@ struct WebReaderContainerView: View { .opacity(navBarVisibilityRatio) .background(Color.systemBackground) .alert("Are you sure you want to remove this item? All associated notes and highlights will be deleted.", - isPresented: $showDeleteConfirmation) { + isPresented: $showDeleteConfirmation) { Button("Remove Item", role: .destructive) { Snackbar.show(message: "Link removed") dataService.removeLink(objectID: item.objectID) From 39b0cbd453746fab4a5c98a6715ae5e28a9c876c Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Fri, 18 Nov 2022 13:39:30 +0800 Subject: [PATCH 13/24] Use strings for highlight marker elements --- apple/Sources/ShareExtension/ShareExtension.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apple/Sources/ShareExtension/ShareExtension.js b/apple/Sources/ShareExtension/ShareExtension.js index 15aa00efc..6ca9c7287 100644 --- a/apple/Sources/ShareExtension/ShareExtension.js +++ b/apple/Sources/ShareExtension/ShareExtension.js @@ -18,8 +18,8 @@ ShareExtension.prototype = { const range = sel.getRangeAt(0) const endMarker = document.createElement("span") const startMarker = document.createElement("span") - endMarker.setAttribute("data-omnivore-highlight-end", true) - startMarker.setAttribute("data-omnivore-highlight-start", true) + endMarker.setAttribute("data-omnivore-highlight-end", "true") + startMarker.setAttribute("data-omnivore-highlight-start", "true") var container = document.createElement("div"); for (var i = 0, len = sel.rangeCount; i < len; ++i) { From 64e2bb39b6905e9adbc110967b4b607315677568 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 15:52:57 +0800 Subject: [PATCH 14/24] Before adding any markers to local HTML ensure we dont have any already --- apple/Sources/ShareExtension/ShareExtension.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/apple/Sources/ShareExtension/ShareExtension.js b/apple/Sources/ShareExtension/ShareExtension.js index 6ca9c7287..acff8657f 100644 --- a/apple/Sources/ShareExtension/ShareExtension.js +++ b/apple/Sources/ShareExtension/ShareExtension.js @@ -12,6 +12,19 @@ function iconURL() { ShareExtension.prototype = { markHighlightSelection: () => { + // First remove any previous markers, this would only normally happen during debugging + try { + const markers = window.document.querySelectorAll( + `span[data-omnivore-highlight-start="true"], + span[data-omnivore-highlight-end="true"]` + ) + + for (let i = 0; i < markers.length; i++) { + markers[i].remove(); + } + } catch { + // This should be OK + } try { const sel = window.getSelection(); if (sel.rangeCount) { From 9741f6b12dce0f7fc7d1968790334651611625cb Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 15:54:27 +0800 Subject: [PATCH 15/24] Add highlights to newly saved pages if they contain highlight markers --- packages/api/src/generated/graphql.ts | 1 + packages/api/src/generated/schema.graphql | 1 + packages/api/src/schema.ts | 1 + packages/api/src/services/save_page.ts | 29 +- packages/api/src/utils/highlightGenerator.ts | 328 +++++++++++++++++++ packages/api/src/utils/parser.ts | 12 + packages/api/test/utils/parser.test.ts | 21 +- 7 files changed, 390 insertions(+), 3 deletions(-) create mode 100644 packages/api/src/utils/highlightGenerator.ts diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index adf7d2e74..dc609a3b7 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -1747,6 +1747,7 @@ export type SaveError = { }; export enum SaveErrorCode { + EmbeddedHighlightFailed = 'EMBEDDED_HIGHLIGHT_FAILED', Unauthorized = 'UNAUTHORIZED', Unknown = 'UNKNOWN' } diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index c1eaec68d..e2548394a 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -1242,6 +1242,7 @@ type SaveError { } enum SaveErrorCode { + EMBEDDED_HIGHLIGHT_FAILED UNAUTHORIZED UNKNOWN } diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 371037663..c8ef50ce2 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -490,6 +490,7 @@ const schema = gql` enum SaveErrorCode { UNKNOWN UNAUTHORIZED + EMBEDDED_HIGHLIGHT_FAILED } type SaveError { diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index b0f492364..6647e7a42 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -14,6 +14,7 @@ import normalizeUrl from 'normalize-url' import { createPageSaveRequest } from './create_page_save_request' import { ArticleSavingRequestStatus, Page } from '../elastic/types' import { createPage, getPageByParam, updatePage } from '../elastic/pages' +import { addHighlightToPage } from '../elastic/highlights' type SaveContext = { pubsub: PubsubClient @@ -101,12 +102,15 @@ export const savePage = async ( savedAt: new Date(), } + let pageId: string | undefined = undefined const existingPage = await getPageByParam({ userId: saver.userId, url: articleToSave.url, state: ArticleSavingRequestStatus.Succeeded, }) + if (existingPage) { + pageId = existingPage.id if ( !(await updatePage( existingPage.id, @@ -139,7 +143,8 @@ export const savePage = async ( } } } else { - if (!(await createPage(articleToSave, ctx))) { + pageId = await createPage(articleToSave, ctx) + if (!pageId) { return { errorCodes: [SaveErrorCode.Unknown], message: 'Failed to create new page', @@ -147,6 +152,28 @@ export const savePage = async ( } } + if (pageId && parseResult.highlightData) { + const highlight = { + updatedAt: new Date(), + createdAt: new Date(), + userId: ctx.uid, + elasticPageId: pageId, + ...parseResult.highlightData, + } + + if ( + !(await addHighlightToPage(pageId, highlight, { + pubsub: ctx.pubsub, + uid: ctx.uid, + })) + ) { + return { + errorCodes: [SaveErrorCode.EmbeddedHighlightFailed], + message: 'Failed to save highlight', + } + } + } + return { clientRequestId: input.clientRequestId, url: `${homePageURL()}/${saver.username}/${slug}`, diff --git a/packages/api/src/utils/highlightGenerator.ts b/packages/api/src/utils/highlightGenerator.ts new file mode 100644 index 000000000..6af5cae16 --- /dev/null +++ b/packages/api/src/utils/highlightGenerator.ts @@ -0,0 +1,328 @@ +import { diff_match_patch as DiffMatchPatch } from 'diff-match-patch' +import { interpolationSearch } from './interpolationSearch' +import { v4 as uuidv4 } from 'uuid' +import { nanoid } from 'nanoid' + +const highlightTag = 'omnivore_highlight' +export const maxHighlightLength = 2000 + +const nonParagraphTagsRegEx = + /^(a|b|basefont|bdo|big|em|font|i|s|small|span|strike|strong|su[bp]|tt|u|code|mark)$/i +const highlightContentRegex = new RegExp( + `<${highlightTag}>([\\s\\S]*)<\\/${highlightTag}>`, + 'i' +) +const maxDeepPatchDistance = 4000 +const maxDeepPatchThreshhold = 0.5 +const maxSurroundingTextLength = 2000 + +type TextNode = { + startIndex: number + node: Node + isParagraphStart?: boolean +} + +type ArticleTextContent = { + textNodes: TextNode[] + articleText: string +} + +export type EmbeddedHighlightData = { + prefix: string + suffix: string + quote: string + id: string + shortId: string + patch: string +} + +function getTextNodesBetween(rootNode: Node, startNode: Node, endNode: Node) { + let textNodeStartingPoint = 0 + let articleText = '' + let newParagraph = false + const textNodes: TextNode[] = [] + let pastStartNode = false, + reachedEndNode = false + + function pushNode(node: Node) { + textNodes.push({ + node, + startIndex: textNodeStartingPoint, + isParagraphStart: newParagraph, + }) + textNodeStartingPoint += node.nodeValue?.length || 0 + articleText += node.nodeValue + newParagraph = false + } + + function getTextNodes(node: Node) { + if (node == startNode) { + pastStartNode = true + } + + if (node.nodeType == 3) { + if ( + pastStartNode && + !reachedEndNode && + !/^\s*$/.test(node.nodeValue || '') + ) { + pushNode(node) + } + } else { + if (!nonParagraphTagsRegEx.test((node as Element).tagName)) + newParagraph = true + } + + for ( + var i = 0, len = node.childNodes.length; + !reachedEndNode && i < len; + ++i + ) { + getTextNodes(node.childNodes[i]) + } + + if (node == endNode) { + reachedEndNode = true + } + } + + getTextNodes(rootNode) + + return { + textNodes, + articleText, + } +} + +export function findEmbeddedHighlight( + dom: Element +): EmbeddedHighlightData | undefined { + const startNode = dom.querySelector( + 'span[data-omnivore-highlight-start="true"]' + ) + const endNode = dom.querySelector('span[data-omnivore-highlight-end="true"]') + + const articleContentElement = dom + if (!articleContentElement || !startNode || !endNode) { + return undefined + } + + const beforeNodes = getTextNodesBetween(dom, articleContentElement, startNode) + const highlightNodes = getTextNodesBetween(dom, startNode, endNode) + const afterNodes = getTextNodesBetween(dom, endNode, articleContentElement) + const allArticleNodes = getTextNodesBetween( + dom, + articleContentElement, + articleContentElement + ) + + const patch = generateDiffPatch( + allArticleNodes, + beforeNodes, + highlightNodes, + afterNodes + ) + + const id = uuidv4() + const shortId = nanoid(8) + const info = getPrefixAndSuffix(allArticleNodes, patch) + const quote = getQuoteText(highlightNodes) + + return { + id, + shortId, + quote, + patch, + prefix: info.prefix, + suffix: info.suffix, + } +} + +const getQuoteText = (highlight: ArticleTextContent): string => { + let quote = '' + + highlight.textNodes.forEach((textNode, i) => { + if (textNode.isParagraphStart && i > 0) { + quote += '\n' + } + quote += textNode.node.textContent + }) + + return quote +} + +function generateDiffPatch( + allArticleNodes: ArticleTextContent, + beforeNodes: ArticleTextContent, + highlightNodes: ArticleTextContent, + afterNodes: ArticleTextContent +): string { + const textWithTags = `${beforeNodes.articleText}<${highlightTag}>${highlightNodes.articleText}${afterNodes.articleText}` + + const diffMatchPatch = new DiffMatchPatch() + const patch = diffMatchPatch.patch_toText( + diffMatchPatch.patch_make(allArticleNodes.articleText, textWithTags) + ) + + if (!patch) throw new Error('Invalid patch') + return patch +} + +function getPrefixAndSuffix( + articleTextNodes: ArticleTextContent, + patch: string +): { + prefix: string + suffix: string + highlightTextStart: number + highlightTextEnd: number + textNodes: TextNode[] + textNodeIndex: number +} { + if (!patch) throw new Error('Invalid patch') + const textNodes = articleTextNodes.textNodes + + const { highlightTextStart, highlightTextEnd } = selectionOffsetsFromPatch( + articleTextNodes.articleText, + patch + ) + + // Searching for the starting text node using interpolation search algorithm + const textNodeIndex = interpolationSearch( + textNodes.map(({ startIndex: startIndex }) => startIndex), + highlightTextStart + ) + const endTextNodeIndex = interpolationSearch( + textNodes.map(({ startIndex: startIndex }) => startIndex), + highlightTextEnd + ) + + const prefix = getSurroundingText({ + textNodes, + startingTextNodeIndex: textNodeIndex, + startingOffset: highlightTextStart - textNodes[textNodeIndex].startIndex, + side: 'prefix', + }) + const suffix = getSurroundingText({ + textNodes, + startingTextNodeIndex: endTextNodeIndex, + startingOffset: highlightTextEnd - textNodes[endTextNodeIndex].startIndex, + side: 'suffix', + }) + return { + prefix, + suffix, + highlightTextStart, + highlightTextEnd, + textNodes, + textNodeIndex, + } +} + +/** + * Gets the part of text from the starting point to the paragraph ending from the + * specified side + * @param param0 - Object that includes textNodes array, starting point and the + * way of movement (prefix, suffix) + * @returns String of text to fulfill the paragraph that surrounds the + * highlight from either starting or ending point + */ +const getSurroundingText = ({ + textNodes, + startingTextNodeIndex, + startingOffset, + side, +}: { + textNodes: TextNode[] + startingTextNodeIndex: number + startingOffset: number + side: 'prefix' | 'suffix' +}): string => { + const isPrefix = side === 'prefix' + let i = startingTextNodeIndex + const getTextPart = (): string => { + i += isPrefix ? -1 : 1 + const { node, isParagraphStart: startsParagraph } = textNodes[i] + const text = node.nodeValue || '' + + if (isPrefix) { + if (startsParagraph) return text + if (text.length > maxSurroundingTextLength) return text + return getTextPart() + text + } else { + if (!textNodes[i + 1] || textNodes[i + 1].isParagraphStart) return text + if (text.length > maxSurroundingTextLength) return text + return text + getTextPart() + } + } + const truncateText = (str: string): string => { + if (str.length <= maxSurroundingTextLength) return str + if (isPrefix) { + return str.slice(str.length - maxSurroundingTextLength) + } + return str.substring(0, maxSurroundingTextLength) + } + + const { isParagraphStart: startsParagraph, node } = + textNodes[startingTextNodeIndex] + const nodeText = node.nodeValue || '' + + const text = isPrefix + ? nodeText.substring(0, startingOffset) + : nodeText.substring(startingOffset) + + if (isPrefix) { + return truncateText(startsParagraph ? text : getTextPart() + text) + } else { + return truncateText( + !textNodes[i + 1] || textNodes[i + 1].isParagraphStart + ? text + : text + getTextPart() + ) + } +} + +const selectionOffsetsFromPatch = ( + articleText: string, + patch: string +): { + highlightTextStart: number + highlightTextEnd: number + matchingHighlightContent: RegExpExecArray +} => { + if (!patch) throw new Error('Invalid patch') + const dmp = new DiffMatchPatch() + // Applying a patch to the whole article text to find the selection content via regexp + const appliedPatch = dmp.patch_apply(dmp.patch_fromText(patch), articleText) + + let matchingHighlightContent + if (!appliedPatch[1][0]) { + dmp.Match_Threshold = maxDeepPatchThreshhold + dmp.Match_Distance = maxDeepPatchDistance + const deeperAppliedPatch = dmp.patch_apply( + dmp.patch_fromText(patch), + articleText + ) + if (!deeperAppliedPatch[1][0]) { + throw new Error('Unable to find the highlight') + } else { + matchingHighlightContent = highlightContentRegex.exec( + deeperAppliedPatch[0] + ) + } + } else { + matchingHighlightContent = highlightContentRegex.exec(appliedPatch[0]) + } + + if (!matchingHighlightContent) + throw new Error('Unable to find the highlight from patch') + + const highlightTextStart = matchingHighlightContent.index + const highlightTextEnd = + highlightTextStart + matchingHighlightContent[1].length + return { + highlightTextStart, + highlightTextEnd, + matchingHighlightContent, + } +} diff --git a/packages/api/src/utils/parser.ts b/packages/api/src/utils/parser.ts index 291fa5d4f..e16195f84 100644 --- a/packages/api/src/utils/parser.ts +++ b/packages/api/src/utils/parser.ts @@ -16,6 +16,11 @@ import { ILike } from 'typeorm' import { v4 as uuid } from 'uuid' import addressparser from 'addressparser' import { preParseContent } from '@omnivore/content-handler' +import { + findEmbeddedHighlight, + EmbeddedHighlightData, +} from './highlightGenerator' +import { HighlightData } from '../datalayer/highlight/model' const logger = buildLogger('utils.parse') @@ -70,6 +75,7 @@ export type ParsedContentPuppeteer = { parsedContent: Readability.ParseResult | null canonicalUrl?: string | null pageType: PageType + highlightData?: EmbeddedHighlightData } /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -178,6 +184,7 @@ export const parsePreparedContent = async ( } let article = null + let highlightData = undefined const { document, pageInfo } = preparedDocument // Checking for content type acceptance or if there are no contentType @@ -234,6 +241,10 @@ export const parsePreparedContent = async ( article.content = article.dom.outerHTML } + if (article?.dom) { + highlightData = findEmbeddedHighlight(article?.dom) + } + const ANCHOR_ELEMENTS_BLOCKED_ATTRIBUTES = [ 'omnivore-highlight-id', 'data-twitter-tweet-id', @@ -315,6 +326,7 @@ export const parsePreparedContent = async ( parsedContent: article, canonicalUrl, pageType: parseOriginalContent(dom), + highlightData, } } diff --git a/packages/api/test/utils/parser.test.ts b/packages/api/test/utils/parser.test.ts index eee4a24e8..f26312c92 100644 --- a/packages/api/test/utils/parser.test.ts +++ b/packages/api/test/utils/parser.test.ts @@ -38,9 +38,9 @@ describe('parseMetadata', async () => { describe('parsePreparedContent', async () => { it('gets published date when JSONLD fails to load', async () => { - nock('https://stratechery.com:443', {"encodedQueryParams":true}) + nock('https://stratechery.com:443', { encodedQueryParams: true }) .get('/wp-json/oembed/1.0/embed') - .query({"url":"https%3A%2F%2Fstratechery.com%2F2016%2Fits-a-tesla%2F"}) + .query({ url: 'https%3A%2F%2Fstratechery.com%2F2016%2Fits-a-tesla%2F' }) .reply(401) const html = load('./test/utils/data/stratechery-blog-post.html') @@ -53,6 +53,23 @@ describe('parsePreparedContent', async () => { new Date('2016-04-05T15:27:51+00:00').getTime() ) }) + it('returns a highlight range if markers are found in the HTML', async () => { + const html = ` + + + This is some text within the highlight markers + + + ` + const result = await parsePreparedContent('https://blog.omnivore.app/', { + document: html, + pageInfo: {}, + }) + + expect(result.highlightData?.quote).to.eq( + 'This is some text within the highlight markers' + ) + }) }) describe('parsePreparedContent', async () => { From a3f477f57a1a0851a840a847bedacb91382c1233 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 16:02:56 +0800 Subject: [PATCH 16/24] Linting fix --- packages/api/src/utils/highlightGenerator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/utils/highlightGenerator.ts b/packages/api/src/utils/highlightGenerator.ts index 6af5cae16..2b472f8b6 100644 --- a/packages/api/src/utils/highlightGenerator.ts +++ b/packages/api/src/utils/highlightGenerator.ts @@ -74,7 +74,7 @@ function getTextNodesBetween(rootNode: Node, startNode: Node, endNode: Node) { } for ( - var i = 0, len = node.childNodes.length; + let i = 0, len = node.childNodes.length; !reachedEndNode && i < len; ++i ) { From c0d5d12ec03fc27875a054ecfe2251bdaeca8073 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 16:11:52 +0800 Subject: [PATCH 17/24] Add missing file --- packages/api/src/utils/interpolationSearch.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 packages/api/src/utils/interpolationSearch.ts diff --git a/packages/api/src/utils/interpolationSearch.ts b/packages/api/src/utils/interpolationSearch.ts new file mode 100644 index 000000000..a834e16ca --- /dev/null +++ b/packages/api/src/utils/interpolationSearch.ts @@ -0,0 +1,32 @@ +/** + * Finds the index of the element which value is the closest to the target + * @param arr - An array of numbers to search from + * @param target - The target number to find + * @returns The index of the closest to the target value array element + */ + +export function interpolationSearch(arr: number[], target: number): number { + let left = 0 + let right = arr.length - 1 + while (left < right) { + const rangeDelta = arr[right] - arr[left] + const indexDelta = right - left + const valueDelta = target - arr[left] + if (valueDelta < 0) { + throw new Error('Unable to find text node') + } + if (!rangeDelta) { + return left + } + const middleIndex = + left + Math.floor((valueDelta * indexDelta) / rangeDelta) + if (target < arr[middleIndex]) { + right = middleIndex + } else if (target >= arr[middleIndex + 1]) { + left = middleIndex + 1 + } else { + return middleIndex + } + } + throw new Error('Unable to find text node') +} From 7b79af481fe79d7f655d521dd133838f0a5b3aa9 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 16:40:17 +0800 Subject: [PATCH 18/24] Add prefix and suffix text to make text matching work --- packages/api/test/utils/parser.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/api/test/utils/parser.test.ts b/packages/api/test/utils/parser.test.ts index f26312c92..0a2f80aa4 100644 --- a/packages/api/test/utils/parser.test.ts +++ b/packages/api/test/utils/parser.test.ts @@ -57,7 +57,13 @@ describe('parsePreparedContent', async () => { const html = ` - This is some text within the highlight markers +
+
+ some prefix text + This is some text within the highlight markers + some suffix text +
+
` From dfaba9dbb3f70aa7b3afd812f742d9d4e69fd52c Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 17:00:26 +0800 Subject: [PATCH 19/24] Start to display a highlight banner in the share extension --- .../Share/ShareExtensionViewModel.swift | 8 +- .../Share/Views/ShareExtensionView.swift | 78 +++++++++++++++---- 2 files changed, 67 insertions(+), 19 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift index e0c075476..b62c7276e 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift @@ -166,7 +166,7 @@ public class ShareExtensionViewModel: ObservableObject { let currentViewer = services.dataService.currentViewer?.username, (try? await services.dataService.loadArticleContentWithRetries(itemID: itemID, username: currentViewer)) != nil { - updateStatusOnMain(requestId: requestId, newStatus: .saved, objectID: linkedItemObjectID) + updateStatusOnMain(requestId: requestId, newStatus: .synced, objectID: linkedItemObjectID) } return true @@ -190,7 +190,11 @@ public class ShareExtensionViewModel: ObservableObject { } } -public enum ShareExtensionStatus { +public enum ShareExtensionStatus: Equatable { + public static func == (lhs: ShareExtensionStatus, rhs: ShareExtensionStatus) -> Bool { + lhs.displayMessage == rhs.displayMessage + } + case processing case saved case synced diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift index 240ecf704..57393a7aa 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift @@ -14,6 +14,7 @@ public struct ShareExtensionView: View { @State var hideUntilReminded = false @State var editingTitle = false @State var editingLabels = false + @State var viewingHighlight = false @State var previousLabels: [LinkedItemLabel]? @State var messageText: String? @@ -44,27 +45,14 @@ public struct ShareExtensionView: View { } } - private var cloudIconName: String { + private var titleColor: Color { switch viewModel.status { - case .synced: - return "checkmark.icloud" case .saved, .processing: - return "icloud" - case .failed(error: _), .syncFailed(error: _): - return "exclamationmark.icloud" - } - } - - private var cloudIconColor: Color { - switch viewModel.status { - case .saved: return .appGrayText - case .processing: - return .clear case .failed(error: _), .syncFailed(error: _): return .red case .synced: - return .blue + return .appGreenSuccess } } @@ -101,7 +89,7 @@ public struct ShareExtensionView: View { Text(messageText ?? titleText) .font(.appSubheadline) - .foregroundColor(isSynced ? .appGreenSuccess : .appGrayText) + .foregroundColor(titleColor) Spacer() } @@ -192,6 +180,56 @@ public struct ShareExtensionView: View { .cornerRadius(8) } + var highlightSection: some View { + HStack { + if !viewingHighlight { + ZStack { + Circle() + .foregroundColor(Color.appBackground) + .frame(width: 34, height: 34) + + Image(systemName: "highlighter") + .font(.appCallout) + .frame(width: 34, height: 34) + .foregroundColor(Color.black) + } + .padding(.trailing, 8) + + VStack { + Text("Highlight") + .font(.appSubheadline) + .foregroundColor(Color.appGrayTextContrast) + .frame(maxWidth: .infinity, alignment: .leading) + + let labelCount = 0 + Text(labelCount > 0 ? + "\(labelCount) label\(labelCount > 1 ? "s" : "") selected" + : "Select text before saving to create highlight") + .font(.appFootnote) + .foregroundColor(Color.appGrayText) + .frame(maxWidth: .infinity, alignment: .leading) + } + + Spacer() + + Image(systemName: "chevron.right") + .font(.appCallout) + } else { +// Text(self.pageSc) +// ScrollView { +// LabelsMasonaryView(labels: labelsViewModel.labels, +// selectedLabels: labelsViewModel.selectedLabels, +// onLabelTap: onLabelTap) +// }.background(Color.appButtonBackground) +// .cornerRadius(8) + } + } + .padding(16) + .frame(maxWidth: .infinity, maxHeight: self.editingLabels ? .infinity : 60) + .background(Color.appButtonBackground) + .cornerRadius(8) + } + func onLabelTap(label: LinkedItemLabel, textChip _: TextChip) { if let selectedIndex = labelsViewModel.selectedLabels.firstIndex(of: label) { labelsViewModel.selectedLabels.remove(at: selectedIndex) @@ -373,13 +411,18 @@ public struct ShareExtensionView: View { if !editingTitle { labelsSection - .padding(.top, 12) .onTapGesture { withAnimation { previousLabels = self.labelsViewModel.selectedLabels editingLabels = true } } + highlightSection + .onTapGesture { + withAnimation { + viewingHighlight = true + } + } } Spacer() @@ -402,6 +445,7 @@ public struct ShareExtensionView: View { .onAppear { viewModel.savePage(extensionContext: extensionContext) } + .environmentObject(viewModel.services.dataService) .task { await labelsViewModel.loadLabelsFromStore(dataService: viewModel.services.dataService) From 1e4658b771af127f7cd803f4c841ee9577712f29 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 18:31:47 +0800 Subject: [PATCH 20/24] Consistent function declaration syntax --- .../Sources/App/Views/WebReader/HighlightViewer.swift | 8 ++++++++ packages/api/src/utils/highlightGenerator.ts | 5 ++--- 2 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift new file mode 100644 index 000000000..a361a99b2 --- /dev/null +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift @@ -0,0 +1,8 @@ +// +// File.swift +// +// +// Created by Jackson Harper on 11/21/22. +// + +import Foundation diff --git a/packages/api/src/utils/highlightGenerator.ts b/packages/api/src/utils/highlightGenerator.ts index 2b472f8b6..52aa86a4c 100644 --- a/packages/api/src/utils/highlightGenerator.ts +++ b/packages/api/src/utils/highlightGenerator.ts @@ -94,9 +94,9 @@ function getTextNodesBetween(rootNode: Node, startNode: Node, endNode: Node) { } } -export function findEmbeddedHighlight( +export const findEmbeddedHighlight = ( dom: Element -): EmbeddedHighlightData | undefined { +): EmbeddedHighlightData | undefined => { const startNode = dom.querySelector( 'span[data-omnivore-highlight-start="true"]' ) @@ -158,7 +158,6 @@ function generateDiffPatch( afterNodes: ArticleTextContent ): string { const textWithTags = `${beforeNodes.articleText}<${highlightTag}>${highlightNodes.articleText}${afterNodes.articleText}` - const diffMatchPatch = new DiffMatchPatch() const patch = diffMatchPatch.patch_toText( diffMatchPatch.patch_make(allArticleNodes.articleText, textWithTags) From 080a1982b2d474d591b2cea520aac3d65d8a5bde Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 18:32:38 +0800 Subject: [PATCH 21/24] Remove unused import --- packages/api/src/utils/parser.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/api/src/utils/parser.ts b/packages/api/src/utils/parser.ts index e16195f84..e19681821 100644 --- a/packages/api/src/utils/parser.ts +++ b/packages/api/src/utils/parser.ts @@ -20,7 +20,6 @@ import { findEmbeddedHighlight, EmbeddedHighlightData, } from './highlightGenerator' -import { HighlightData } from '../datalayer/highlight/model' const logger = buildLogger('utils.parse') From fc2e4f7d8cf507422a9812b5393585033cde705e Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 18:49:36 +0800 Subject: [PATCH 22/24] Display some UI for highlight on save --- .../Share/ShareExtensionViewModel.swift | 5 +- .../Share/Views/ShareExtensionView.swift | 139 +++++++++++------- .../App/Views/WebReader/HighlightViewer.swift | 107 +++++++++++++- 3 files changed, 190 insertions(+), 61 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift index b62c7276e..9f1e6bbc5 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/ShareExtensionViewModel.swift @@ -9,6 +9,7 @@ public class ShareExtensionViewModel: ObservableObject { @Published public var status: ShareExtensionStatus = .processing @Published public var title: String = "" @Published public var url: String? + @Published public var highlightData: HighlightData? @Published public var linkedItem: LinkedItem? @Published public var requestId = UUID().uuidString.lowercased() @Published var debugText: String? @@ -83,13 +84,13 @@ public class ShareExtensionViewModel: ObservableObject { DispatchQueue.main.async { self.status = .saved - let url = URLComponents(string: payload.url) let hostname = URL(string: payload.url)?.host ?? "" switch payload.contentType { - case let .html(html: _, title: title, _): + case let .html(html: _, title: title, highlightData: highlightData): self.title = title ?? "" self.url = hostname + self.highlightData = highlightData case .none: self.url = hostname self.title = payload.url diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift index 57393a7aa..82788db6a 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift @@ -12,16 +12,22 @@ public struct ShareExtensionView: View { @State var reminderTime: ReminderTime? @State var hideUntilReminded = false - @State var editingTitle = false - @State var editingLabels = false - @State var viewingHighlight = false @State var previousLabels: [LinkedItemLabel]? @State var messageText: String? + @State var viewState = ViewState.mainView + enum FocusField: Hashable { case titleEditor } + enum ViewState { + case mainView + case editingTitle + case editingLabels + case viewingHighlight + } + @FocusState private var focusedField: FocusField? private func handleReminderTimeSelection(_ selectedTime: ReminderTime) { @@ -102,14 +108,14 @@ public struct ShareExtensionView: View { .font(.appFootnote) .padding(.trailing, 8) .onTapGesture { - editingTitle = true + viewState = .editingTitle } }) - .disabled(editingTitle) - .opacity(editingTitle ? 0.0 : 1.0) + .disabled(viewState == .editingTitle) + .opacity(viewState == .editingTitle ? 0.0 : 1.0) VStack(alignment: .leading) { - if !editingTitle { + if viewState != .editingTitle { Text(self.viewModel.title) .font(.appSubheadline) .foregroundColor(.appGrayTextContrast) @@ -134,7 +140,7 @@ public struct ShareExtensionView: View { var labelsSection: some View { HStack { - if !editingLabels { + if viewState != .editingLabels { ZStack { Circle() .foregroundColor(Color.blue) @@ -166,23 +172,37 @@ public struct ShareExtensionView: View { Image(systemName: "chevron.right") .font(.appCallout) } else { - ScrollView { - LabelsMasonaryView(labels: labelsViewModel.labels, - selectedLabels: labelsViewModel.selectedLabels, - onLabelTap: onLabelTap) - }.background(Color.appButtonBackground) - .cornerRadius(8) + VStack { + ScrollView { + LabelsMasonaryView(labels: labelsViewModel.labels, + selectedLabels: labelsViewModel.selectedLabels, + onLabelTap: onLabelTap) + }.background(Color.appButtonBackground) + .cornerRadius(8) + + Button( + action: { labelsViewModel.showCreateLabelModal = true }, + label: { + HStack { + Spacer() + Image(systemName: "plus") + Text("Create label") + Spacer() + } + } + ).buttonStyle(RoundedRectButtonStyle(color: .blue, textColor: .white)) + } } } .padding(16) - .frame(maxWidth: .infinity, maxHeight: self.editingLabels ? .infinity : 60) + .frame(maxWidth: .infinity, maxHeight: viewState == .editingLabels ? .infinity : 60) .background(Color.appButtonBackground) .cornerRadius(8) } var highlightSection: some View { HStack { - if !viewingHighlight { + if viewState != .viewingHighlight { ZStack { Circle() .foregroundColor(Color.appBackground) @@ -201,9 +221,8 @@ public struct ShareExtensionView: View { .foregroundColor(Color.appGrayTextContrast) .frame(maxWidth: .infinity, alignment: .leading) - let labelCount = 0 - Text(labelCount > 0 ? - "\(labelCount) label\(labelCount > 1 ? "s" : "") selected" + Text(viewModel.highlightData != nil ? + viewModel.highlightData!.highlightText : "Select text before saving to create highlight") .font(.appFootnote) .foregroundColor(Color.appGrayText) @@ -214,18 +233,15 @@ public struct ShareExtensionView: View { Image(systemName: "chevron.right") .font(.appCallout) - } else { -// Text(self.pageSc) -// ScrollView { -// LabelsMasonaryView(labels: labelsViewModel.labels, -// selectedLabels: labelsViewModel.selectedLabels, -// onLabelTap: onLabelTap) -// }.background(Color.appButtonBackground) -// .cornerRadius(8) + } else if let highlightText = self.viewModel.highlightData?.highlightText { + Text(highlightText) + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + .cornerRadius(8) + .padding(0) } } .padding(16) - .frame(maxWidth: .infinity, maxHeight: self.editingLabels ? .infinity : 60) + .frame(maxWidth: .infinity, maxHeight: viewState == .viewingHighlight ? .infinity : 60) .background(Color.appButtonBackground) .cornerRadius(8) } @@ -324,6 +340,19 @@ public struct ShareExtensionView: View { } } + var editingViewTitle: String { + switch viewState { + case .editingTitle: + return "Edit Title" + case .editingLabels: + return "Labels" + case .viewingHighlight: + return "Highlight" + default: + return "" + } + } + public var body: some View { VStack(alignment: .center) { Capsule() @@ -331,7 +360,7 @@ public struct ShareExtensionView: View { .frame(width: 60, height: 4) .padding(.top, 10) - if !editingLabels, !editingTitle { + if viewState == .mainView { titleBar .padding(.top, 10) .padding(.bottom, 12) @@ -339,28 +368,28 @@ public struct ShareExtensionView: View { ZStack { Button(action: { withAnimation { - if editingLabels { + if viewState == .editingLabels { if let linkedItem = self.viewModel.linkedItem { self.labelsViewModel.selectedLabels = previousLabels ?? [] self.labelsViewModel.saveItemLabelChanges(itemID: linkedItem.unwrappedID, dataService: self.viewModel.services.dataService) } } - editingTitle = false - editingLabels = false + viewState = .mainView } }, label: { Text("Cancel") }) .frame(maxWidth: .infinity, alignment: .leading) + .opacity(viewState == .viewingHighlight ? 0.0 : 1.0) + // Don't show viewState when viewing the highlight - Text(editingTitle ? "Edit Title" : "Labels").bold() + Text(editingViewTitle).bold() .frame(maxWidth: .infinity, alignment: .center) Button(action: { withAnimation { - editingTitle = false - editingLabels = false + viewState = .mainView - if editingTitle { + if viewState == .editingTitle { if let linkedItem = self.viewModel.linkedItem { viewModel.submitTitleEdit(dataService: self.viewModel.services.dataService, itemID: linkedItem.unwrappedID, @@ -376,11 +405,11 @@ public struct ShareExtensionView: View { .padding(.bottom, 4) } - if !editingLabels, !editingTitle { + if viewState == .mainView { titleBox } - if editingTitle { + if viewState == .editingTitle { ScrollView(showsIndicators: false) { VStack(alignment: .center, spacing: 16) { VStack(alignment: .leading, spacing: 6) { @@ -409,25 +438,29 @@ public struct ShareExtensionView: View { Spacer() } - if !editingTitle { - labelsSection - .onTapGesture { - withAnimation { - previousLabels = self.labelsViewModel.selectedLabels - editingLabels = true + if viewState != .editingTitle { + if viewState != .viewingHighlight { + labelsSection + .onTapGesture { + withAnimation { + previousLabels = self.labelsViewModel.selectedLabels + viewState = .editingLabels + } } - } - highlightSection - .onTapGesture { - withAnimation { - viewingHighlight = true + } + if viewState != .editingLabels { + highlightSection + .onTapGesture { + withAnimation { + viewState = .viewingHighlight + } } - } + } } Spacer() - if !editingLabels, !editingTitle { + if viewState == .mainView { Divider() .padding(.bottom, 20) @@ -445,7 +478,9 @@ public struct ShareExtensionView: View { .onAppear { viewModel.savePage(extensionContext: extensionContext) } - + .sheet(isPresented: $labelsViewModel.showCreateLabelModal) { + CreateLabelView(viewModel: labelsViewModel) + } .environmentObject(viewModel.services.dataService) .task { await labelsViewModel.loadLabelsFromStore(dataService: viewModel.services.dataService) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift index a361a99b2..e27a2d50c 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/HighlightViewer.swift @@ -1,8 +1,101 @@ -// -// File.swift -// -// -// Created by Jackson Harper on 11/21/22. -// +import Models +import SwiftUI +import Utils +import Views +import WebKit -import Foundation +struct HighlightViewer: PlatformViewRepresentable { + let highlightData: HighlightData + + func makeCoordinator() -> WebReaderCoordinator { + WebReaderCoordinator() + } + + private func makePlatformView(context: Context) -> WKWebView { + let webView = WebViewManager.shared() + let contentController = WKUserContentController() + + webView.navigationDelegate = context.coordinator + webView.configuration.userContentController = contentController + webView.configuration.userContentController.removeAllScriptMessageHandlers() + + #if os(iOS) + webView.isOpaque = false + webView.backgroundColor = .clear + webView.scrollView.delegate = context.coordinator + webView.scrollView.contentInset.top = readerViewNavBarHeight + webView.scrollView.verticalScrollIndicatorInsets.top = readerViewNavBarHeight + webView.configuration.userContentController.add(webView, name: "viewerAction") + #else + webView.setValue(false, forKey: "drawsBackground") + #endif + + for action in WebViewAction.allCases { + webView.configuration.userContentController.add(context.coordinator, name: action.rawValue) + } + + webView.configuration.userContentController.addScriptMessageHandler( + context.coordinator, contentWorld: .page, name: "articleAction" + ) + + loadContent(webView: webView) + + return webView + } + + private func updatePlatformView(_: WKWebView, context _: Context) { + // If the webview had been terminated `needsReload` will have been set to true + // Or if the articleContent value has changed then it's id will be different from the coordinator's +// if context.coordinator.needsReload { +// loadContent(webView: webView) +// context.coordinator.needsReload = false +// return +// } + } + + private func loadContent(webView: WKWebView) { + let themeKey = ThemeManager.currentThemeName + let content = """ + + + + + + + + +
+
+ \(highlightData.highlightHTML) +
+ + + """ + + webView.loadHTMLString(content, baseURL: ViewsPackage.resourceURL) + } +} + +#if os(iOS) + extension HighlightViewer { + func makeUIView(context: Context) -> WKWebView { + makePlatformView(context: context) + } + + func updateUIView(_ webView: WKWebView, context: Context) { + updatePlatformView(webView, context: context) + } + } +#else + extension WebReader { + func makeNSView(context: Context) -> WKWebView { + makePlatformView(context: context) + } + + func updateNSView(_ webView: WKWebView, context: Context) { + updatePlatformView(webView, context: context) + } + } +#endif From 4966af354ae9b21fa1e3db7c884a84a2b5e18067 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 20:50:02 +0800 Subject: [PATCH 23/24] Add some nocks for failing URL tests --- packages/api/test/services/save_email.test.ts | 3 +++ packages/api/test/services/save_newsletter_email.test.ts | 2 ++ 2 files changed, 5 insertions(+) diff --git a/packages/api/test/services/save_email.test.ts b/packages/api/test/services/save_email.test.ts index 0343d512f..4707fa490 100644 --- a/packages/api/test/services/save_email.test.ts +++ b/packages/api/test/services/save_email.test.ts @@ -5,6 +5,7 @@ import { createTestUser, deleteTestUser } from '../db' import { SaveContext, saveEmail } from '../../src/services/save_email' import { createPubSubClient } from '../../src/datalayer/pubsub' import { getPageByParam } from '../../src/elastic/pages' +import nock from 'nock/types' describe('saveEmail', () => { const username = 'fakeUser' @@ -15,6 +16,8 @@ describe('saveEmail', () => { }) it('doesnt fail if saved twice', async () => { + nock('https://blog.omnivore.app').get('/fake-url').reply(404) + const url = 'https://blog.omnivore.app/fake-url' const title = 'fake title' const author = 'fake author' diff --git a/packages/api/test/services/save_newsletter_email.test.ts b/packages/api/test/services/save_newsletter_email.test.ts index 1b1e7e952..ce4eed049 100644 --- a/packages/api/test/services/save_newsletter_email.test.ts +++ b/packages/api/test/services/save_newsletter_email.test.ts @@ -9,6 +9,7 @@ import { NewsletterEmail } from '../../src/entity/newsletter_email' import { SaveContext } from '../../src/services/save_email' import { createPubSubClient } from '../../src/datalayer/pubsub' import { getPageByParam } from '../../src/elastic/pages' +import nock from 'nock/types' describe('saveNewsletterEmail', () => { const username = 'fakeUser' @@ -35,6 +36,7 @@ describe('saveNewsletterEmail', () => { }) it('adds the newsletter to the library', async () => { + nock('https://blog.omnivore.app').get('/fake-url').reply(404) const url = 'https://blog.omnivore.app/fake-url' await saveNewsletterEmail( From 6ca8b3610b1939aeb8519de2eebb775296ea3a78 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 21 Nov 2022 21:04:09 +0800 Subject: [PATCH 24/24] Fix nock imports --- packages/api/test/services/save_email.test.ts | 2 +- packages/api/test/services/save_newsletter_email.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/test/services/save_email.test.ts b/packages/api/test/services/save_email.test.ts index 4707fa490..4b0b345d8 100644 --- a/packages/api/test/services/save_email.test.ts +++ b/packages/api/test/services/save_email.test.ts @@ -5,7 +5,7 @@ import { createTestUser, deleteTestUser } from '../db' import { SaveContext, saveEmail } from '../../src/services/save_email' import { createPubSubClient } from '../../src/datalayer/pubsub' import { getPageByParam } from '../../src/elastic/pages' -import nock from 'nock/types' +import nock from 'nock' describe('saveEmail', () => { const username = 'fakeUser' diff --git a/packages/api/test/services/save_newsletter_email.test.ts b/packages/api/test/services/save_newsletter_email.test.ts index ce4eed049..0b2d874fe 100644 --- a/packages/api/test/services/save_newsletter_email.test.ts +++ b/packages/api/test/services/save_newsletter_email.test.ts @@ -9,7 +9,7 @@ import { NewsletterEmail } from '../../src/entity/newsletter_email' import { SaveContext } from '../../src/services/save_email' import { createPubSubClient } from '../../src/datalayer/pubsub' import { getPageByParam } from '../../src/elastic/pages' -import nock from 'nock/types' +import nock from 'nock' describe('saveNewsletterEmail', () => { const username = 'fakeUser'