From 76bc28ae025729177b58cff0cb8dead04940087c Mon Sep 17 00:00:00 2001 From: Ilyas Hallak Date: Wed, 20 Aug 2025 20:38:42 +0200 Subject: [PATCH] feat: Improve UI components and performance optimizations - Refactor BookmarksView with better error handling and loading states - Optimize BookmarkLabelsViewModel with cached properties and reduced recomputation - Fix Core Data thread safety in LabelsRepository with performAndWait - Enhance TagManagementView with sorted selected labels display - Clean up ShareBookmarkViewModel comments - Update localization strings for error states - Bump build version to 19 These changes improve overall app performance and user experience across bookmark management workflows. --- Localizable.xcstrings | 39 +-- URLShare/ShareBookmarkViewModel.swift | 3 - readeck.xcodeproj/project.pbxproj | 8 +- .../Data/Repository/LabelsRepository.swift | 11 +- .../BookmarkLabelsViewModel.swift | 57 ++-- readeck/UI/Bookmarks/BookmarksView.swift | 306 +++++++++++------- readeck/UI/Components/TagManagementView.swift | 2 +- 7 files changed, 243 insertions(+), 183 deletions(-) diff --git a/Localizable.xcstrings b/Localizable.xcstrings index 68238a3..bfc66a3 100644 --- a/Localizable.xcstrings +++ b/Localizable.xcstrings @@ -48,9 +48,6 @@ }, "%lld min" : { - }, - "%lld minutes" : { - }, "%lld." : { @@ -102,12 +99,6 @@ }, "Are you sure you want to log out? This will delete all your login credentials and return you to setup." : { - }, - "Automatic sync" : { - - }, - "Automatically mark articles as read" : { - }, "Available tags" : { @@ -120,9 +111,6 @@ }, "Changes take effect immediately. Lower log levels include higher ones (Debug includes all, Critical includes only critical messages)." : { - }, - "Clear cache" : { - }, "Close" : { @@ -132,9 +120,6 @@ }, "Critical" : { - }, - "Data Management" : { - }, "Debug" : { @@ -273,9 +258,6 @@ }, "OK" : { - }, - "Open external links in in-app Safari" : { - }, "Optional: Custom title" : { @@ -319,18 +301,12 @@ } } } - }, - "Reading Settings" : { - }, "Remove" : { }, "Reset" : { - }, - "Reset settings" : { - }, "Reset to Defaults" : { @@ -340,9 +316,6 @@ }, "Resume listening" : { - }, - "Safari Reader Mode" : { - }, "Save bookmark" : { @@ -391,12 +364,6 @@ }, "Speed" : { - }, - "Sync interval" : { - - }, - "Sync Settings" : { - }, "Syncing with server..." : { @@ -406,6 +373,12 @@ }, "This is how your bookmark descriptions and article text will appear in the app. The quick brown fox jumps over the lazy dog." : { + }, + "Try Again" : { + + }, + "Unable to load bookmarks" : { + }, "Unarchive Bookmark" : { diff --git a/URLShare/ShareBookmarkViewModel.swift b/URLShare/ShareBookmarkViewModel.swift index d6c0221..5027275 100644 --- a/URLShare/ShareBookmarkViewModel.swift +++ b/URLShare/ShareBookmarkViewModel.swift @@ -103,12 +103,10 @@ class ShareBookmarkViewModel: ObservableObject { let measurement = PerformanceMeasurement(operation: "loadLabels", logger: logger) logger.debug("Starting to load labels") Task { - // Check if server is reachable let serverReachable = ServerConnectivity.isServerReachableSync() logger.debug("Server reachable for labels: \(serverReachable)") if serverReachable { - // Load from API let loaded = await SimpleAPI.getBookmarkLabels { [weak self] message, error in self?.statusMessage = (message, error, error ? "❌" : "✅") } ?? [] @@ -119,7 +117,6 @@ class ShareBookmarkViewModel: ObservableObject { measurement.end() } } else { - // Load from local database let localTags = OfflineBookmarkManager.shared.getTags() let localLabels = localTags.enumerated().map { index, tagName in BookmarkLabelDto(name: tagName, count: 0, href: "local://\(index)") diff --git a/readeck.xcodeproj/project.pbxproj b/readeck.xcodeproj/project.pbxproj index e8aeaed..fa9636e 100644 --- a/readeck.xcodeproj/project.pbxproj +++ b/readeck.xcodeproj/project.pbxproj @@ -436,7 +436,7 @@ buildSettings = { CODE_SIGN_ENTITLEMENTS = URLShare/URLShare.entitlements; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 16; + CURRENT_PROJECT_VERSION = 19; DEVELOPMENT_TEAM = 8J69P655GN; GENERATE_INFOPLIST_FILE = YES; INFOPLIST_FILE = URLShare/Info.plist; @@ -469,7 +469,7 @@ buildSettings = { CODE_SIGN_ENTITLEMENTS = URLShare/URLShare.entitlements; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 16; + CURRENT_PROJECT_VERSION = 19; DEVELOPMENT_TEAM = 8J69P655GN; GENERATE_INFOPLIST_FILE = YES; INFOPLIST_FILE = URLShare/Info.plist; @@ -624,7 +624,7 @@ CODE_SIGN_ENTITLEMENTS = readeck/readeck.entitlements; CODE_SIGN_IDENTITY = "Apple Development"; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 16; + CURRENT_PROJECT_VERSION = 19; DEVELOPMENT_ASSET_PATHS = "\"readeck/Preview Content\""; DEVELOPMENT_TEAM = 8J69P655GN; ENABLE_HARDENED_RUNTIME = YES; @@ -668,7 +668,7 @@ CODE_SIGN_ENTITLEMENTS = readeck/readeck.entitlements; CODE_SIGN_IDENTITY = "Apple Development"; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 16; + CURRENT_PROJECT_VERSION = 19; DEVELOPMENT_ASSET_PATHS = "\"readeck/Preview Content\""; DEVELOPMENT_TEAM = 8J69P655GN; ENABLE_HARDENED_RUNTIME = YES; diff --git a/readeck/Data/Repository/LabelsRepository.swift b/readeck/Data/Repository/LabelsRepository.swift index 94a3d22..fda950e 100644 --- a/readeck/Data/Repository/LabelsRepository.swift +++ b/readeck/Data/Repository/LabelsRepository.swift @@ -29,6 +29,15 @@ class LabelsRepository: PLabelsRepository { let fetchRequest: NSFetchRequest = TagEntity.fetchRequest() fetchRequest.predicate = NSPredicate(format: "name == %@", name) - return (try? coreDataManager.context.fetch(fetchRequest).isEmpty == false) ?? false + var exists = false + coreDataManager.context.performAndWait { + do { + let results = try coreDataManager.context.fetch(fetchRequest) + exists = !results.isEmpty + } catch { + exists = false + } + } + return exists } } diff --git a/readeck/UI/BookmarkDetail/BookmarkLabelsViewModel.swift b/readeck/UI/BookmarkDetail/BookmarkLabelsViewModel.swift index 72735d5..0a225b0 100644 --- a/readeck/UI/BookmarkDetail/BookmarkLabelsViewModel.swift +++ b/readeck/UI/BookmarkDetail/BookmarkLabelsViewModel.swift @@ -12,36 +12,40 @@ class BookmarkLabelsViewModel { var showErrorAlert = false var currentLabels: [String] = [] { didSet { - calculatePages() + if oldValue != currentLabels { + calculatePages() + } } } var newLabelText = "" var searchText = "" { didSet { - calculatePages() + if oldValue != searchText { + calculatePages() + } } } var allLabels: [BookmarkLabel] = [] { didSet { - calculatePages() + if oldValue != allLabels { + calculatePages() + } } } var labelPages: [[BookmarkLabel]] = [] - // Computed property for available labels (excluding current labels) + // Cached properties to avoid recomputation + private var _availableLabels: [BookmarkLabel] = [] + private var _filteredLabels: [BookmarkLabel] = [] + var availableLabels: [BookmarkLabel] { - return allLabels.filter { currentLabels.contains($0.name) == false } + return _availableLabels } - // Computed property for filtered labels based on search text var filteredLabels: [BookmarkLabel] { - if searchText.isEmpty { - return availableLabels - } else { - return availableLabels.filter { $0.name.localizedCaseInsensitiveContains(searchText) } - } + return _filteredLabels } var availableLabelPages: [[BookmarkLabel]] = [] @@ -76,8 +80,8 @@ class BookmarkLabelsViewModel { errorMessage = nil do { - currentLabels.append(contentsOf: labels) - currentLabels = Array(Set(currentLabels)) // Remove duplicates + let uniqueLabels = Set(currentLabels + labels) + currentLabels = currentLabels.filter { uniqueLabels.contains($0) } + labels.filter { !currentLabels.contains($0) } try await addLabelsUseCase.execute(bookmarkId: bookmarkId, labels: labels) } catch let error as BookmarkUpdateError { @@ -89,7 +93,6 @@ class BookmarkLabelsViewModel { } isLoading = false - calculatePages() } @MainActor @@ -120,7 +123,6 @@ class BookmarkLabelsViewModel { } isLoading = false - calculatePages() } @MainActor @@ -136,8 +138,6 @@ class BookmarkLabelsViewModel { } else { await addLabel(to: bookmarkId, label: label) } - - calculatePages() } func updateLabels(_ labels: [String]) { @@ -147,24 +147,31 @@ class BookmarkLabelsViewModel { private func calculatePages() { let pageSize = Constants.Labels.pageSize + // Update cached available labels + _availableLabels = allLabels.filter { !currentLabels.contains($0.name) } + + // Update cached filtered labels + if searchText.isEmpty { + _filteredLabels = _availableLabels + } else { + _filteredLabels = _availableLabels.filter { $0.name.localizedCaseInsensitiveContains(searchText) } + } + // Calculate pages for all labels if allLabels.count <= pageSize { labelPages = [allLabels] } else { - // Normal pagination for larger datasets labelPages = stride(from: 0, to: allLabels.count, by: pageSize).map { Array(allLabels[$0.. some View { + VStack(spacing: 16) { + Image(systemName: "exclamationmark.triangle.fill") + .font(.system(size: 48)) + .foregroundColor(.orange) + + VStack(spacing: 8) { + Text("Unable to load bookmarks") + .font(.headline) + .foregroundColor(.primary) + + Text(message) + .font(.subheadline) + .foregroundColor(.secondary) + .multilineTextAlignment(.center) + } + + Button("Try Again") { + Task { + await viewModel.loadBookmarks(state: state, type: type, tag: tag) + } + } + .buttonStyle(.borderedProminent) + .controlSize(.large) + } + .padding(.horizontal, 40) + } + + @ViewBuilder + private var bookmarksList: some View { + List { + ForEach(viewModel.bookmarks?.bookmarks ?? [], id: \.id) { bookmark in + Button(action: { + if UIDevice.isPhone { + selectedBookmarkId = bookmark.id + } else { + if selectedBookmark?.id == bookmark.id { + selectedBookmark = nil + DispatchQueue.main.async { + selectedBookmark = bookmark + } + } else { + selectedBookmark = bookmark + } + } + }) { + BookmarkCardView( + bookmark: bookmark, + currentState: state, + onArchive: { bookmark in + Task { + await viewModel.toggleArchive(bookmark: bookmark) + } + }, + onDelete: { bookmark in + bookmarkToDelete = bookmark + }, + onToggleFavorite: { bookmark in + Task { + await viewModel.toggleFavorite(bookmark: bookmark) + } + }, + namespace: namespace + ) + .onAppear { + if bookmark.id == viewModel.bookmarks?.bookmarks.last?.id { + Task { + await viewModel.loadMoreBookmarks() + } + } + } + } + .buttonStyle(PlainButtonStyle()) + .listRowInsets(EdgeInsets(top: 8, leading: 16, bottom: 8, trailing: 16)) + .listRowSeparator(.hidden) + .listRowBackground(Color(R.color.bookmark_list_bg)) + .matchedTransitionSource(id: bookmark.id, in: namespace) + } + + // Show loading indicator for pagination + if viewModel.isLoading && !(viewModel.bookmarks?.bookmarks.isEmpty == true) { + HStack { + Spacer() + ProgressView() + .scaleEffect(0.8) + Spacer() + } + .listRowBackground(Color(R.color.bookmark_list_bg)) + .listRowSeparator(.hidden) + } + } + .listStyle(.plain) + .background(Color(R.color.bookmark_list_bg)) + .scrollContentBackground(.hidden) + .refreshable { + await viewModel.refreshBookmarks() + } + .overlay { + if viewModel.bookmarks?.bookmarks.isEmpty == true && !viewModel.isLoading && viewModel.errorMessage == nil { + ContentUnavailableView( + "No bookmarks", + systemImage: "bookmark", + description: Text( + "No bookmarks found in \(state.displayName.lowercased())." + ) + ) + } + } + } + + @ViewBuilder + private var fabButton: some View { + VStack { + Spacer() + HStack { + Spacer() + + Button(action: { + showingAddBookmark = true + }) { + Image(systemName: "plus") + .font(.title2) + .fontWeight(.semibold) + .foregroundColor(.white) + .frame(width: 56, height: 56) + .background(Color.accentColor) + .clipShape(Circle()) + .shadow(color: .black.opacity(0.25), radius: 6, x: 0, y: 3) + } + .padding(.trailing, 20) + .padding(.bottom, 20) + } + } + } } #Preview { diff --git a/readeck/UI/Components/TagManagementView.swift b/readeck/UI/Components/TagManagementView.swift index b2227d0..015e02c 100644 --- a/readeck/UI/Components/TagManagementView.swift +++ b/readeck/UI/Components/TagManagementView.swift @@ -191,7 +191,7 @@ struct TagManagementView: View { .fontWeight(.medium) LazyVGrid(columns: [GridItem(.flexible()), GridItem(.flexible()), GridItem(.flexible()), GridItem(.flexible())], alignment: .leading, spacing: 8) { - ForEach(Array(selectedLabelsSet), id: \.self) { label in + ForEach(selectedLabelsSet.sorted(), id: \.self) { label in UnifiedLabelChip( label: label, isSelected: false,