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.
This commit is contained in:
Ilyas Hallak 2025-08-20 20:38:42 +02:00
parent 692f34d2ce
commit 76bc28ae02
7 changed files with 243 additions and 183 deletions

View File

@ -48,9 +48,6 @@
}, },
"%lld min" : { "%lld min" : {
},
"%lld minutes" : {
}, },
"%lld." : { "%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." : { "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" : { "Available tags" : {
@ -120,9 +111,6 @@
}, },
"Changes take effect immediately. Lower log levels include higher ones (Debug includes all, Critical includes only critical messages)." : { "Changes take effect immediately. Lower log levels include higher ones (Debug includes all, Critical includes only critical messages)." : {
},
"Clear cache" : {
}, },
"Close" : { "Close" : {
@ -132,9 +120,6 @@
}, },
"Critical" : { "Critical" : {
},
"Data Management" : {
}, },
"Debug" : { "Debug" : {
@ -273,9 +258,6 @@
}, },
"OK" : { "OK" : {
},
"Open external links in in-app Safari" : {
}, },
"Optional: Custom title" : { "Optional: Custom title" : {
@ -319,18 +301,12 @@
} }
} }
} }
},
"Reading Settings" : {
}, },
"Remove" : { "Remove" : {
}, },
"Reset" : { "Reset" : {
},
"Reset settings" : {
}, },
"Reset to Defaults" : { "Reset to Defaults" : {
@ -340,9 +316,6 @@
}, },
"Resume listening" : { "Resume listening" : {
},
"Safari Reader Mode" : {
}, },
"Save bookmark" : { "Save bookmark" : {
@ -391,12 +364,6 @@
}, },
"Speed" : { "Speed" : {
},
"Sync interval" : {
},
"Sync Settings" : {
}, },
"Syncing with server..." : { "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." : { "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" : { "Unarchive Bookmark" : {

View File

@ -103,12 +103,10 @@ class ShareBookmarkViewModel: ObservableObject {
let measurement = PerformanceMeasurement(operation: "loadLabels", logger: logger) let measurement = PerformanceMeasurement(operation: "loadLabels", logger: logger)
logger.debug("Starting to load labels") logger.debug("Starting to load labels")
Task { Task {
// Check if server is reachable
let serverReachable = ServerConnectivity.isServerReachableSync() let serverReachable = ServerConnectivity.isServerReachableSync()
logger.debug("Server reachable for labels: \(serverReachable)") logger.debug("Server reachable for labels: \(serverReachable)")
if serverReachable { if serverReachable {
// Load from API
let loaded = await SimpleAPI.getBookmarkLabels { [weak self] message, error in let loaded = await SimpleAPI.getBookmarkLabels { [weak self] message, error in
self?.statusMessage = (message, error, error ? "" : "") self?.statusMessage = (message, error, error ? "" : "")
} ?? [] } ?? []
@ -119,7 +117,6 @@ class ShareBookmarkViewModel: ObservableObject {
measurement.end() measurement.end()
} }
} else { } else {
// Load from local database
let localTags = OfflineBookmarkManager.shared.getTags() let localTags = OfflineBookmarkManager.shared.getTags()
let localLabels = localTags.enumerated().map { index, tagName in let localLabels = localTags.enumerated().map { index, tagName in
BookmarkLabelDto(name: tagName, count: 0, href: "local://\(index)") BookmarkLabelDto(name: tagName, count: 0, href: "local://\(index)")

View File

@ -436,7 +436,7 @@
buildSettings = { buildSettings = {
CODE_SIGN_ENTITLEMENTS = URLShare/URLShare.entitlements; CODE_SIGN_ENTITLEMENTS = URLShare/URLShare.entitlements;
CODE_SIGN_STYLE = Automatic; CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 16; CURRENT_PROJECT_VERSION = 19;
DEVELOPMENT_TEAM = 8J69P655GN; DEVELOPMENT_TEAM = 8J69P655GN;
GENERATE_INFOPLIST_FILE = YES; GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_FILE = URLShare/Info.plist; INFOPLIST_FILE = URLShare/Info.plist;
@ -469,7 +469,7 @@
buildSettings = { buildSettings = {
CODE_SIGN_ENTITLEMENTS = URLShare/URLShare.entitlements; CODE_SIGN_ENTITLEMENTS = URLShare/URLShare.entitlements;
CODE_SIGN_STYLE = Automatic; CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 16; CURRENT_PROJECT_VERSION = 19;
DEVELOPMENT_TEAM = 8J69P655GN; DEVELOPMENT_TEAM = 8J69P655GN;
GENERATE_INFOPLIST_FILE = YES; GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_FILE = URLShare/Info.plist; INFOPLIST_FILE = URLShare/Info.plist;
@ -624,7 +624,7 @@
CODE_SIGN_ENTITLEMENTS = readeck/readeck.entitlements; CODE_SIGN_ENTITLEMENTS = readeck/readeck.entitlements;
CODE_SIGN_IDENTITY = "Apple Development"; CODE_SIGN_IDENTITY = "Apple Development";
CODE_SIGN_STYLE = Automatic; CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 16; CURRENT_PROJECT_VERSION = 19;
DEVELOPMENT_ASSET_PATHS = "\"readeck/Preview Content\""; DEVELOPMENT_ASSET_PATHS = "\"readeck/Preview Content\"";
DEVELOPMENT_TEAM = 8J69P655GN; DEVELOPMENT_TEAM = 8J69P655GN;
ENABLE_HARDENED_RUNTIME = YES; ENABLE_HARDENED_RUNTIME = YES;
@ -668,7 +668,7 @@
CODE_SIGN_ENTITLEMENTS = readeck/readeck.entitlements; CODE_SIGN_ENTITLEMENTS = readeck/readeck.entitlements;
CODE_SIGN_IDENTITY = "Apple Development"; CODE_SIGN_IDENTITY = "Apple Development";
CODE_SIGN_STYLE = Automatic; CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 16; CURRENT_PROJECT_VERSION = 19;
DEVELOPMENT_ASSET_PATHS = "\"readeck/Preview Content\""; DEVELOPMENT_ASSET_PATHS = "\"readeck/Preview Content\"";
DEVELOPMENT_TEAM = 8J69P655GN; DEVELOPMENT_TEAM = 8J69P655GN;
ENABLE_HARDENED_RUNTIME = YES; ENABLE_HARDENED_RUNTIME = YES;

View File

@ -29,6 +29,15 @@ class LabelsRepository: PLabelsRepository {
let fetchRequest: NSFetchRequest<TagEntity> = TagEntity.fetchRequest() let fetchRequest: NSFetchRequest<TagEntity> = TagEntity.fetchRequest()
fetchRequest.predicate = NSPredicate(format: "name == %@", name) 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
} }
} }

View File

@ -12,36 +12,40 @@ class BookmarkLabelsViewModel {
var showErrorAlert = false var showErrorAlert = false
var currentLabels: [String] = [] { var currentLabels: [String] = [] {
didSet { didSet {
if oldValue != currentLabels {
calculatePages() calculatePages()
} }
} }
}
var newLabelText = "" var newLabelText = ""
var searchText = "" { var searchText = "" {
didSet { didSet {
if oldValue != searchText {
calculatePages() calculatePages()
} }
} }
}
var allLabels: [BookmarkLabel] = [] { var allLabels: [BookmarkLabel] = [] {
didSet { didSet {
if oldValue != allLabels {
calculatePages() calculatePages()
} }
} }
}
var labelPages: [[BookmarkLabel]] = [] 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] { 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] { var filteredLabels: [BookmarkLabel] {
if searchText.isEmpty { return _filteredLabels
return availableLabels
} else {
return availableLabels.filter { $0.name.localizedCaseInsensitiveContains(searchText) }
}
} }
var availableLabelPages: [[BookmarkLabel]] = [] var availableLabelPages: [[BookmarkLabel]] = []
@ -76,8 +80,8 @@ class BookmarkLabelsViewModel {
errorMessage = nil errorMessage = nil
do { do {
currentLabels.append(contentsOf: labels) let uniqueLabels = Set(currentLabels + labels)
currentLabels = Array(Set(currentLabels)) // Remove duplicates currentLabels = currentLabels.filter { uniqueLabels.contains($0) } + labels.filter { !currentLabels.contains($0) }
try await addLabelsUseCase.execute(bookmarkId: bookmarkId, labels: labels) try await addLabelsUseCase.execute(bookmarkId: bookmarkId, labels: labels)
} catch let error as BookmarkUpdateError { } catch let error as BookmarkUpdateError {
@ -89,7 +93,6 @@ class BookmarkLabelsViewModel {
} }
isLoading = false isLoading = false
calculatePages()
} }
@MainActor @MainActor
@ -120,7 +123,6 @@ class BookmarkLabelsViewModel {
} }
isLoading = false isLoading = false
calculatePages()
} }
@MainActor @MainActor
@ -136,8 +138,6 @@ class BookmarkLabelsViewModel {
} else { } else {
await addLabel(to: bookmarkId, label: label) await addLabel(to: bookmarkId, label: label)
} }
calculatePages()
} }
func updateLabels(_ labels: [String]) { func updateLabels(_ labels: [String]) {
@ -147,24 +147,31 @@ class BookmarkLabelsViewModel {
private func calculatePages() { private func calculatePages() {
let pageSize = Constants.Labels.pageSize 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 // Calculate pages for all labels
if allLabels.count <= pageSize { if allLabels.count <= pageSize {
labelPages = [allLabels] labelPages = [allLabels]
} else { } else {
// Normal pagination for larger datasets
labelPages = stride(from: 0, to: allLabels.count, by: pageSize).map { labelPages = stride(from: 0, to: allLabels.count, by: pageSize).map {
Array(allLabels[$0..<min($0 + pageSize, allLabels.count)]) Array(allLabels[$0..<min($0 + pageSize, allLabels.count)])
} }
} }
// Calculate pages for filtered labels (search results or available labels) // Calculate pages for filtered labels
let labelsToShow = searchText.isEmpty ? availableLabels : filteredLabels if _filteredLabels.count <= pageSize {
if labelsToShow.count <= pageSize { availableLabelPages = [_filteredLabels]
availableLabelPages = [labelsToShow]
} else { } else {
// Normal pagination for larger datasets availableLabelPages = stride(from: 0, to: _filteredLabels.count, by: pageSize).map {
availableLabelPages = stride(from: 0, to: labelsToShow.count, by: pageSize).map { Array(_filteredLabels[$0..<min($0 + pageSize, _filteredLabels.count)])
Array(labelsToShow[$0..<min($0 + pageSize, labelsToShow.count)])
} }
} }
} }

View File

@ -39,124 +39,15 @@ struct BookmarksView: View {
var body: some View { var body: some View {
ZStack { ZStack {
if viewModel.isLoading && viewModel.bookmarks?.bookmarks.isEmpty == true { if shouldShowCenteredState {
VStack(spacing: 20) { centeredStateView
Spacer()
VStack(spacing: 16) {
ProgressView()
.scaleEffect(1.3)
.tint(.accentColor)
VStack(spacing: 8) {
Text("Loading \(state.displayName)")
.font(.headline)
.foregroundColor(.primary)
Text("Please wait while we fetch your bookmarks...")
.font(.subheadline)
.foregroundColor(.secondary)
.multilineTextAlignment(.center)
}
}
.padding(.horizontal, 40)
Spacer()
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background(Color(R.color.bookmark_list_bg))
} else { } else {
List { bookmarksList
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)
}
}
.listStyle(.plain)
.background(Color(R.color.bookmark_list_bg))
.scrollContentBackground(.hidden)
.refreshable {
await viewModel.refreshBookmarks()
}
.overlay {
if viewModel.bookmarks?.bookmarks.isEmpty == true && !viewModel.isLoading {
ContentUnavailableView(
"No bookmarks",
systemImage: "bookmark",
description: Text(
"No bookmarks found in \(state.displayName.lowercased())."
)
)
}
}
} }
// FAB Button - only show for "Unread" // FAB Button - only show for "Unread" and when not in error/loading state
if state == .unread || state == .all { if (state == .unread || state == .all) && !shouldShowCenteredState {
VStack { fabButton
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)
}
}
} }
} }
.navigationDestination( .navigationDestination(
@ -206,6 +97,189 @@ struct BookmarksView: View {
} }
} }
} }
// MARK: - Computed Properties
private var shouldShowCenteredState: Bool {
let isEmpty = viewModel.bookmarks?.bookmarks.isEmpty == true
return isEmpty && (viewModel.isLoading || viewModel.errorMessage != nil)
}
// MARK: - View Components
@ViewBuilder
private var centeredStateView: some View {
VStack(spacing: 20) {
Spacer()
if viewModel.isLoading {
loadingView
} else if let errorMessage = viewModel.errorMessage {
errorView(message: errorMessage)
}
Spacer()
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background(Color(R.color.bookmark_list_bg))
}
@ViewBuilder
private var loadingView: some View {
VStack(spacing: 16) {
ProgressView()
.scaleEffect(1.3)
.tint(.accentColor)
VStack(spacing: 8) {
Text("Loading \(state.displayName)")
.font(.headline)
.foregroundColor(.primary)
Text("Please wait while we fetch your bookmarks...")
.font(.subheadline)
.foregroundColor(.secondary)
.multilineTextAlignment(.center)
}
}
.padding(.horizontal, 40)
}
@ViewBuilder
private func errorView(message: String) -> 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 { #Preview {

View File

@ -191,7 +191,7 @@ struct TagManagementView: View {
.fontWeight(.medium) .fontWeight(.medium)
LazyVGrid(columns: [GridItem(.flexible()), GridItem(.flexible()), GridItem(.flexible()), GridItem(.flexible())], alignment: .leading, spacing: 8) { 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( UnifiedLabelChip(
label: label, label: label,
isSelected: false, isSelected: false,