From 57289cb0c4c04b6dc27aa4cbbae64d8f8650f5f4 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 23 Jan 2023 11:59:37 +0800 Subject: [PATCH] Improve label search in the apply views --- .../Share/Views/ShareExtensionView.swift | 36 ++--- .../App/Views/Labels/ApplyLabelsView.swift | 135 ++++++++++-------- .../Sources/App/Views/Labels/LabelsView.swift | 9 +- .../App/Views/Labels/LabelsViewModel.swift | 2 +- .../OmnivoreKit/Sources/Views/SearchBar.swift | 3 + packages/readabilityjs/Readability.js | 2 +- 6 files changed, 105 insertions(+), 82 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift index b828f8d3a..c231a87f2 100644 --- a/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift +++ b/apple/OmnivoreKit/Sources/App/AppExtensions/Share/Views/ShareExtensionView.swift @@ -181,11 +181,24 @@ public struct ShareExtensionView: View { LabelsMasonaryView(labels: labelsViewModel.labels.applySearchFilter(labelsViewModel.labelSearchFilter), selectedLabels: labelsViewModel.selectedLabels.applySearchFilter(labelsViewModel.labelSearchFilter), onLabelTap: onLabelTap) - Spacer() - } - .padding(.bottom, 16) - .background(Color.appButtonBackground) - .cornerRadius(8) + Button( + action: { labelsViewModel.showCreateLabelModal = true }, + label: { + HStack { + Image(systemName: "tag").foregroundColor(.blue) + Text( + labelsViewModel.labelSearchFilter.count > 0 ? + "Create: \"\(labelsViewModel.labelSearchFilter)\" label" : + LocalText.createLabelMessage + ).foregroundColor(.blue) + .font(Font.system(size: 14)) + Spacer() + } + } + ) + .buttonStyle(PlainButtonStyle()) + .padding(10) + }.background(Color.appButtonBackground) } } } @@ -462,7 +475,7 @@ public struct ShareExtensionView: View { viewModel.savePage(extensionContext: extensionContext) } .sheet(isPresented: $labelsViewModel.showCreateLabelModal) { - CreateLabelView(viewModel: labelsViewModel) + CreateLabelView(viewModel: labelsViewModel, newLabelName: labelsViewModel.labelSearchFilter) } .alert("Before saving an article select text in Safari to create a highlight on save.", isPresented: $showHighlightInstructionAlert) { @@ -470,15 +483,6 @@ public struct ShareExtensionView: View { } .task { await labelsViewModel.loadLabelsFromStore(dataService: viewModel.services.dataService) - } - .sheet(isPresented: $showSearchLabels) { - ApplyLabelsView(mode: .list(self.labelsViewModel.selectedLabels), isSearchFocused: true) { labels in - self.labelsViewModel.selectedLabels = labels - if let itemID = self.viewModel.linkedItem?.unwrappedID { - self.labelsViewModel.saveItemLabelChanges(itemID: itemID, dataService: self.viewModel.services.dataService) - } - } - } - .environmentObject(viewModel.services.dataService) + }.environmentObject(viewModel.services.dataService) } } diff --git a/apple/OmnivoreKit/Sources/App/Views/Labels/ApplyLabelsView.swift b/apple/OmnivoreKit/Sources/App/Views/Labels/ApplyLabelsView.swift index 5facfca14..d80959395 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Labels/ApplyLabelsView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Labels/ApplyLabelsView.swift @@ -48,75 +48,87 @@ struct ApplyLabelsView: View { } var innerBody: some View { - List { - Section(header: Spacer(minLength: 0)) { - SearchBar(searchTerm: $viewModel.labelSearchFilter) - .listRowInsets(EdgeInsets(top: 0, leading: 0, bottom: 0, trailing: 0)) - .listRowBackground(Color.clear) - } - Section { - Button( - action: { viewModel.showCreateLabelModal = true }, - label: { - HStack { - Image(systemName: "plus.circle.fill").foregroundColor(.green) - Text(LocalText.createLabelMessage).foregroundColor(.appGrayTextContrast) - Spacer() - } - } - ) - .disabled(viewModel.isLoading) - } - Section { - ForEach(viewModel.labels.applySearchFilter(viewModel.labelSearchFilter), id: \.self) { label in - Button( - action: { - if isSelected(label) { - viewModel.selectedLabels.removeAll(where: { $0.id == label.id }) - } else { - viewModel.selectedLabels.append(label) - } - }, - label: { - HStack { - TextChip(feedItemLabel: label) - Spacer() + VStack { + SearchBar(searchTerm: $viewModel.labelSearchFilter) + .padding(.vertical, 8) + .padding(.horizontal, 16) + + List { + Section { + ForEach(viewModel.labels.applySearchFilter(viewModel.labelSearchFilter), id: \.self) { label in + Button( + action: { if isSelected(label) { - Image(systemName: "checkmark") + viewModel.selectedLabels.removeAll(where: { $0.id == label.id }) + } else { + viewModel.selectedLabels.append(label) + } + }, + label: { + HStack { + TextChip(feedItemLabel: label) + Spacer() + if isSelected(label) { + Image(systemName: "checkmark") + } } } - } - ) - .listRowInsets(EdgeInsets(top: 0, leading: 8, bottom: 0, trailing: 8)) - #if os(macOS) - .buttonStyle(PlainButtonStyle()) - #endif + ) + .padding(.vertical, 5) + #if os(macOS) + .buttonStyle(PlainButtonStyle()) + #endif + } + createLabelButton } + Spacer() + } + .listStyle(PlainListStyle()) + .navigationTitle(mode.navTitle) + #if os(iOS) + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .navigationBarLeading) { + cancelButton + } + ToolbarItem(placement: .navigationBarTrailing) { + saveItemChangesButton + } + } + #else + .toolbar { + ToolbarItemGroup { + cancelButton + saveItemChangesButton + } + } + #endif + .sheet(isPresented: $viewModel.showCreateLabelModal) { + CreateLabelView(viewModel: viewModel, newLabelName: viewModel.labelSearchFilter) } } - .padding(.top, 0) - .navigationTitle(mode.navTitle) - #if os(iOS) - .navigationBarTitleDisplayMode(.inline) - .toolbar { - ToolbarItem(placement: .navigationBarLeading) { - cancelButton - } - ToolbarItem(placement: .navigationBarTrailing) { - saveItemChangesButton + } + + var createLabelButton: some View { + Button( + action: { viewModel.showCreateLabelModal = true }, + label: { + HStack { + Image(systemName: "tag").foregroundColor(.blue) + Text( + viewModel.labelSearchFilter.count > 0 ? + "Create: \"\(viewModel.labelSearchFilter)\" label" : + LocalText.createLabelMessage + ).foregroundColor(.blue) + .font(Font.system(size: 14)) + Spacer() } } - #else - .toolbar { - ToolbarItemGroup { - cancelButton - saveItemChangesButton - } - } - #endif - .sheet(isPresented: $viewModel.showCreateLabelModal) { - CreateLabelView(viewModel: viewModel) - } + ) + .buttonStyle(PlainButtonStyle()) + .disabled(viewModel.isLoading) + .listRowSeparator(.hidden, edges: .bottom) + .padding(.vertical, 10) } var saveItemChangesButton: some View { @@ -151,7 +163,6 @@ struct ApplyLabelsView: View { EmptyView() } else { innerBody - .padding(.top, -20) // This is a hack to give us a bit more room on the page } } #elseif os(macOS) diff --git a/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsView.swift b/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsView.swift index c145391f1..e4266561d 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsView.swift @@ -38,7 +38,7 @@ struct LabelsView: View { Button(LocalText.cancelGeneric, role: .cancel) { self.labelToRemove = nil } } .sheet(isPresented: $viewModel.showCreateLabelModal) { - CreateLabelView(viewModel: viewModel) + CreateLabelView(viewModel: viewModel, newLabelName: viewModel.labelSearchFilter) } .task { await viewModel.loadLabels(dataService: dataService, item: nil) } } @@ -85,9 +85,14 @@ struct CreateLabelView: View { @EnvironmentObject var dataService: DataService @ObservedObject var viewModel: LabelsViewModel - @State private var newLabelName = "" + @State private var newLabelName: String @State private var newLabelColor = Color.clear + init(viewModel: LabelsViewModel, newLabelName: String = "") { + self.viewModel = viewModel + self.newLabelName = newLabelName + } + var shouldDisableCreateButton: Bool { viewModel.isLoading || newLabelName.isEmpty || newLabelColor == .clear } diff --git a/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsViewModel.swift index 6c7d7d5b0..6fe2bf268 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Labels/LabelsViewModel.swift @@ -114,7 +114,7 @@ import Views if let label = dataService.viewContext.object(with: labelObjectID) as? LinkedItemLabel { labels.insert(label, at: 0) - unselectedLabels.insert(label, at: 0) + selectedLabels.insert(label, at: 0) } isLoading = false diff --git a/apple/OmnivoreKit/Sources/Views/SearchBar.swift b/apple/OmnivoreKit/Sources/Views/SearchBar.swift index 5b716e192..e11c75ce5 100644 --- a/apple/OmnivoreKit/Sources/Views/SearchBar.swift +++ b/apple/OmnivoreKit/Sources/Views/SearchBar.swift @@ -52,5 +52,8 @@ public struct SearchBar: View { searchTerm = "" } } + .onTapGesture { + isFocused = true + } } } diff --git a/packages/readabilityjs/Readability.js b/packages/readabilityjs/Readability.js index 707a90096..f57dcb127 100644 --- a/packages/readabilityjs/Readability.js +++ b/packages/readabilityjs/Readability.js @@ -171,7 +171,7 @@ Readability.prototype = { // Readability-readerable.js. Please keep both copies in sync. articleNegativeLookBehindCandidates: /breadcrumbs|breadcrumb|utils|trilist/i, articleNegativeLookAheadCandidates: /outstream(.?)_|sub(.?)_|m_|omeda-promo-|in-article-advert|block-ad-.*/i, - unlikelyCandidates: /\bad\b|ai2html|banner|breadcrumbs|breadcrumb|combx|comment|community|cover-wrap|disqus|extra|footer|gdpr|header|legends|menu|related|remark|replies|rss|shoutbox|sidebar|skyscraper|social|sponsor|supplemental|ad-break|agegate|pagination|pager(?!ow)|popup|yom-remote|copyright|keywords|outline|infinite-list|beta|recirculation|site-index|hide-for-print|post-end-share-cta|post-end-cta-full|post-footer|post-head|post-tag|li-date|main-navigation|programtic-ads|outstream_article|hfeed|comment-holder|back-to-top|show-up-next|onward-journey|topic-tracker|list-nav|block-ad-entity|adSpecs|gift-article-button|modal-title|in-story-masthead|share-tools|standard-dock|expanded-dock|margins-h|subscribe-dialog|icon|bumped|dvz-social-media-buttons/i, + unlikelyCandidates: /\bad\b|ai2html|banner|breadcrumbs|breadcrumb|combx|comment|community|cover-wrap|disqus|extra|footer|gdpr|header|legends|menu|related|remark|replies|rss|shoutbox|sidebar|skyscraper|social|sponsor|supplemental|ad-break|agegate|pagination|pager(?!ow)|popup|yom-remote|copyright|keywords|outline|infinite-list|beta|recirculation|site-index|hide-for-print|post-end-share-cta|post-end-cta-full|post-footer|post-head|post-tag|li-date|main-navigation|programtic-ads|outstream_article|hfeed|comment-holder|back-to-top|show-up-next|onward-journey|topic-tracker|list-nav|block-ad-entity|adSpecs|gift-article-button|modal-title|in-story-masthead|share-tools|standard-dock|expanded-dock|margins-h|subscribe-dialog|icon|bumped|dvz-social-media-buttons|post-toc|mobile-menu|mobile-navbar/i, // okMaybeItsACandidate: /and|article(?!-breadcrumb)|body|column|content|main|shadow|post-header/i, get okMaybeItsACandidate() { return new RegExp(`and|(?