fix: Address code review findings for NotificationsView (FRE-4737)
- P0: Add default param to protocol list(params:) for compile fix - P1: Fix onDelete async closure, implement deletion logic - P2: Remove redundant objectWillChange.send() (Published handles it) - P2: Make RelativeDateTimeFormatter static singleton (per-row perf) - P3: Replace deprecated NavigationView with NavigationStack
This commit is contained in:
@@ -3,7 +3,7 @@ import Foundation
|
|||||||
// MARK: - Service Protocol
|
// MARK: - Service Protocol
|
||||||
|
|
||||||
protocol NotificationsServiceProtocol: Sendable {
|
protocol NotificationsServiceProtocol: Sendable {
|
||||||
func list(params: NotificationListParams) async throws -> [NotificationItem]
|
func list(params: NotificationListParams = NotificationListParams()) async throws -> [NotificationItem]
|
||||||
func markAsRead(id: String) async throws
|
func markAsRead(id: String) async throws
|
||||||
func markAllAsRead() async throws
|
func markAllAsRead() async throws
|
||||||
func getUnreadCount() async throws -> Int
|
func getUnreadCount() async throws -> Int
|
||||||
|
|||||||
@@ -54,7 +54,6 @@ class NotificationsViewModel: ObservableObject {
|
|||||||
try await notificationsService.markAsRead(id: id)
|
try await notificationsService.markAsRead(id: id)
|
||||||
notifications[index].isRead = true
|
notifications[index].isRead = true
|
||||||
badgeCount = max(0, badgeCount - 1)
|
badgeCount = max(0, badgeCount - 1)
|
||||||
objectWillChange.send()
|
|
||||||
} catch {
|
} catch {
|
||||||
print("Failed to mark notification as read: \(error)")
|
print("Failed to mark notification as read: \(error)")
|
||||||
}
|
}
|
||||||
@@ -70,7 +69,6 @@ class NotificationsViewModel: ObservableObject {
|
|||||||
notifications[index].isRead = true
|
notifications[index].isRead = true
|
||||||
}
|
}
|
||||||
badgeCount = 0
|
badgeCount = 0
|
||||||
objectWillChange.send()
|
|
||||||
} catch {
|
} catch {
|
||||||
print("Failed to mark all as read: \(error)")
|
print("Failed to mark all as read: \(error)")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -43,10 +43,14 @@ struct NotificationRowView: View {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private func formatTimestamp(_ date: Date) -> String {
|
private func formatTimestamp(_ date: Date) -> String {
|
||||||
|
Self.formatter.localizedString(for: date, relativeTo: Date())
|
||||||
|
}
|
||||||
|
|
||||||
|
private static let formatter: RelativeDateTimeFormatter = {
|
||||||
let formatter = RelativeDateTimeFormatter()
|
let formatter = RelativeDateTimeFormatter()
|
||||||
formatter.unitsStyle = .abbreviated
|
formatter.unitsStyle = .abbreviated
|
||||||
return formatter.localizedString(for: date, relativeTo: Date())
|
return formatter
|
||||||
}
|
}()
|
||||||
}
|
}
|
||||||
|
|
||||||
#Preview {
|
#Preview {
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ struct NotificationsView: View {
|
|||||||
}
|
}
|
||||||
|
|
||||||
var body: some View {
|
var body: some View {
|
||||||
NavigationView {
|
NavigationStack {
|
||||||
Group {
|
Group {
|
||||||
if viewModel.notifications.isEmpty && !viewModel.isLoading {
|
if viewModel.notifications.isEmpty && !viewModel.isLoading {
|
||||||
emptyStateView
|
emptyStateView
|
||||||
@@ -18,7 +18,6 @@ struct NotificationsView: View {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
.navigationTitle("Notifications")
|
.navigationTitle("Notifications")
|
||||||
.navigationBarTitleDisplayMode(.inline)
|
|
||||||
.toolbar {
|
.toolbar {
|
||||||
if !viewModel.notifications.isEmpty {
|
if !viewModel.notifications.isEmpty {
|
||||||
ToolbarItem(placement: .navigationBarTrailing) {
|
ToolbarItem(placement: .navigationBarTrailing) {
|
||||||
@@ -57,7 +56,15 @@ struct NotificationsView: View {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
.onDelete(perform: deleteNotifications)
|
.onDelete { offsets in
|
||||||
|
Task {
|
||||||
|
for index in offsets {
|
||||||
|
let notification = viewModel.notifications[index]
|
||||||
|
await viewModel.markAsRead(id: notification.id)
|
||||||
|
}
|
||||||
|
viewModel.notifications.remove(atOffsets: offsets)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
.listStyle(.insetGrouped)
|
.listStyle(.insetGrouped)
|
||||||
.refreshable {
|
.refreshable {
|
||||||
@@ -84,15 +91,6 @@ struct NotificationsView: View {
|
|||||||
}
|
}
|
||||||
.padding(.vertical, 60)
|
.padding(.vertical, 60)
|
||||||
}
|
}
|
||||||
|
|
||||||
private func deleteNotifications(at offsets: IndexSet) async {
|
|
||||||
// TODO: Implement notification deletion logic
|
|
||||||
// This would typically call a delete API endpoint
|
|
||||||
for index in offsets {
|
|
||||||
let notification = viewModel.notifications[index]
|
|
||||||
// await notificationsService.delete(id: notification.id)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#Preview {
|
#Preview {
|
||||||
|
|||||||
Reference in New Issue
Block a user