Skip to content

Commit 92b52be

Browse files
opficdevopficdev
authored
[#392] 리스트형 삭제/복구 시 로딩 UX를 개선한다 (#401)
* refactor: 웹페이지 제거 시 LoadingView 제거 * fix: 웹페이지 삭제 복구 후 탭 전환 로딩 문제 수정 * fix: Todo 삭제 복구 시 LoadingView가 뜨고 과도하게 로딩에 오래 걸려보이는 현상 해결 * fix: PushNotification 삭제 복구 시 LoadingView가 뜨고 과도하게 로딩이 오래 걸려보이는 현상 해결 * style: 변수이름 수정 * fix: 혹시라도 남아있을 isHidden 요소를 모두 제거하도록 수정 * feat: 웹페이지 제거, undo 실패 시 문구를 띄워 탭해서 리프레시가 가능하도록 구현 * style: Delete 단어 제거 * fix: 혹시라도 남아있을 isHidden 요소를 모두 제거하도록 수정 * refactor: 숨김 알림 병합 로직의 평균 시간복잡도를 O(N*M)에서 O(N+M)으로 최적화
1 parent 3c0371a commit 92b52be

10 files changed

Lines changed: 185 additions & 124 deletions

File tree

‎DevLog/Presentation/Structure/PushNotificationItem.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import Foundation
99

1010
struct PushNotificationItem: Identifiable, Hashable {
1111
let id: String
12+
var isHidden = false
1213
let title: String
1314
let body: String
1415
let receivedAt: Date

‎DevLog/Presentation/Structure/Todo/TodoListItem.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import Foundation
99

1010
struct TodoListItem: Identifiable, Hashable {
1111
let id: String
12+
var isHidden = false
1213
let number: Int
1314
let title: String
1415
let tags: [String]

‎DevLog/Presentation/Structure/WebPageItem.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import SwiftUI
99

1010
struct WebPageItem: Identifiable, Hashable {
1111
private let metadata: WebPage
12+
var isHidden = false
1213

1314
init(from metadata: WebPage) {
1415
self.metadata = metadata

‎DevLog/Presentation/ViewModel/HomeViewModel.swift‎

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ final class HomeViewModel: Store {
1414
var preferences: [TodoCategoryItem] = []
1515
var recentTodos: [RecentTodoItem] = []
1616
var webPages: [WebPageItem] = []
17+
var needsWebPageRefresh = false
1718
var isNetworkConnected: Bool = true
1819
var showContentPicker: Bool = false
1920
var showTodoEditor: Bool = false
@@ -40,7 +41,10 @@ final class HomeViewModel: Store {
4041
case setPresentation(Presentation, Bool)
4142
case setAlert(isPresented: Bool, type: AlertType? = nil)
4243
case setToast(isPresented: Bool, type: ToastType? = nil)
44+
case refreshWebPages
4345
case setLoading(LoadingTarget, Bool)
46+
case setWebPageHidden(URL, Bool)
47+
case handleWebPageDeleteFailure(URL)
4448
case tapTodoCategory(TodoCategory)
4549
case orderTodoCategory([TodoCategoryItem])
4650
case setTodoCategory([TodoCategoryItem])
@@ -51,13 +55,12 @@ final class HomeViewModel: Store {
5155
case deleteWebPage(WebPageItem)
5256
case undoDeleteWebPage
5357
case updateWebPages([WebPageItem])
54-
case restoreWebPage(WebPageItem, Int)
5558
}
5659

5760
enum SideEffect {
5861
case addTodo(Todo)
5962
case addWebPage(String)
60-
case deleteWebPage(WebPageItem, Int)
63+
case deleteWebPage(WebPageItem)
6164
case undoDeleteWebPage(String)
6265
case fetchTodoCategoryPreferences
6366
case updateTodoCategoryPreferences([TodoCategoryItem])
@@ -140,13 +143,13 @@ final class HomeViewModel: Store {
140143
switch action {
141144
case .networkStatusChanged(let isConnected):
142145
state.isNetworkConnected = isConnected
143-
case .onAppear, .setPresentation, .setAlert, .setToast, .tapTodoCategory,
144-
.orderTodoCategory, .addTodo, .updateWebPageURLInput,
146+
case .onAppear, .setPresentation, .setAlert, .setToast, .refreshWebPages,
147+
.tapTodoCategory, .orderTodoCategory, .addTodo, .updateWebPageURLInput,
145148
.addWebPage, .deleteWebPage, .undoDeleteWebPage:
146149
effects = reduceByView(action, state: &state)
147150

148-
case .setLoading, .setTodoCategory, .updateRecentTodos,
149-
.updateWebPages, .restoreWebPage:
151+
case .setLoading, .setWebPageHidden, .handleWebPageDeleteFailure, .setTodoCategory,
152+
.updateRecentTodos, .updateWebPages:
150153
effects = reduceByRun(action, state: &state)
151154
}
152155

@@ -218,38 +221,24 @@ final class HomeViewModel: Store {
218221
send(.setAlert(isPresented: true, type: .error))
219222
}
220223
}
221-
case .deleteWebPage(let page, let index):
222-
beginLoading(for: .webPage, mode: .delayed)
224+
case .deleteWebPage(let page):
223225
Task {
224226
do {
225-
defer { endLoading(for: .webPage, mode: .delayed) }
226227
try await deleteWebPageUseCase.execute(page.url.absoluteString)
227228
} catch {
228-
send(.restoreWebPage(page, index))
229+
send(.handleWebPageDeleteFailure(page.id))
229230
send(.setAlert(isPresented: true, type: .error))
230231
}
231232
}
232233
case .undoDeleteWebPage(let urlString):
233-
beginLoading(for: .webPage, mode: .delayed)
234234
Task {
235-
defer { endLoading(for: .webPage, mode: .delayed) }
236-
237-
var shouldPresentError = false
238-
239235
do {
240236
try await undoDeleteWebPageUseCase.execute(urlString)
237+
try await addWebPageUseCase.execute(urlString)
241238
} catch {
242-
shouldPresentError = true
243-
}
244-
245-
do {
246-
let pages = try await fetchWebPagesUseCase.execute("")
247-
send(.updateWebPages(pages.map { WebPageItem(from: $0) }))
248-
} catch {
249-
shouldPresentError = true
250-
}
251-
252-
if shouldPresentError {
239+
if let webPageURL = URL(string: urlString) {
240+
send(.setWebPageHidden(webPageURL, true))
241+
}
253242
send(.setAlert(isPresented: true, type: .error))
254243
}
255244
}
@@ -285,6 +274,8 @@ private extension HomeViewModel {
285274
switch action {
286275
case .onAppear:
287276
return [.fetchTodoCategoryPreferences, .fetchRecentTodos, .fetchWebPages]
277+
case .refreshWebPages:
278+
return [.fetchWebPages]
288279
case .setPresentation(let presentation, let isPresented):
289280
setPresentation(&state, presentation: presentation, isPresented: isPresented)
290281
case .setAlert(let presented, let type):
@@ -296,6 +287,7 @@ private extension HomeViewModel {
296287
case .setToast(let isPresented, let type):
297288
setToast(&state, isPresented: isPresented, for: type)
298289
if !isPresented {
290+
state.webPages.removeAll { $0.isHidden }
299291
deletedWebPageURLString = nil
300292
}
301293
case .tapTodoCategory(let category):
@@ -320,12 +312,17 @@ private extension HomeViewModel {
320312
case .deleteWebPage(let page):
321313
if let index = state.webPages.firstIndex(where: { $0.id == page.id }) {
322314
deletedWebPageURLString = page.url.absoluteString
323-
state.webPages.remove(at: index)
315+
state.webPages[index].isHidden = true
324316
setToast(&state, isPresented: true, for: .deleteWebPage)
325-
return [.deleteWebPage(page, index)]
317+
return [.deleteWebPage(page)]
326318
}
327319
case .undoDeleteWebPage:
328320
guard let deletedWebPageURLString else { return [] }
321+
if let index = state.webPages.firstIndex(where: {
322+
$0.url.absoluteString == deletedWebPageURLString
323+
}) {
324+
state.webPages[index].isHidden = false
325+
}
329326
self.deletedWebPageURLString = nil
330327
return [.undoDeleteWebPage(deletedWebPageURLString)]
331328
default:
@@ -339,23 +336,24 @@ private extension HomeViewModel {
339336
switch action {
340337
case .setLoading(let loadingTarget, let isLoading):
341338
setLoading(&state, loadingTarget: loadingTarget, isLoading: isLoading)
339+
case .setWebPageHidden(let webPageURL, let isHidden):
340+
if let index = state.webPages.firstIndex(where: { $0.id == webPageURL }) {
341+
state.webPages[index].isHidden = isHidden
342+
}
343+
case .handleWebPageDeleteFailure(let webPageURL):
344+
if let index = state.webPages.firstIndex(where: { $0.id == webPageURL }) {
345+
state.webPages[index].isHidden = false
346+
} else {
347+
state.needsWebPageRefresh = true
348+
}
342349
case .setTodoCategory(let preferences):
343350
state.preferences = preferences
344351
state.recentTodos = syncRecentTodos(state.recentTodos, preferences: preferences)
345352
case .updateRecentTodos(let todos):
346353
state.recentTodos = todos
347354
case .updateWebPages(let pages):
348355
state.webPages = pages
349-
case .restoreWebPage(let page, let index):
350-
if state.webPages.contains(where: { $0.id == page.id }) { break }
351-
if index <= state.webPages.count {
352-
state.webPages.insert(page, at: index)
353-
} else {
354-
state.webPages.append(page)
355-
}
356-
if deletedWebPageURLString == page.url.absoluteString {
357-
deletedWebPageURLString = nil
358-
}
356+
state.needsWebPageRefresh = false
359357
default:
360358
break
361359
}

‎DevLog/Presentation/ViewModel/PushNotificationListViewModel.swift‎

Lines changed: 59 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ final class PushNotificationListViewModel: Store {
3737
case resetPagination
3838
case setHasMore(Bool)
3939
case syncNotifications([PushNotificationItem], nextCursor: PushNotificationCursor?, hasMore: Bool)
40-
case restoreNotification(PushNotificationItem, Int)
40+
case setNotificationHidden(String, Bool)
4141
case toggleSortOption
4242
case setTimeFilter(PushNotificationQuery.TimeFilter)
4343
case toggleUnreadOnly
@@ -48,7 +48,7 @@ final class PushNotificationListViewModel: Store {
4848

4949
enum SideEffect {
5050
case fetchNotifications(PushNotificationQuery, cursor: PushNotificationCursor?)
51-
case delete(PushNotificationItem, Int)
51+
case delete(PushNotificationItem)
5252
case undoDelete(String)
5353
case toggleRead(String)
5454
}
@@ -61,7 +61,7 @@ final class PushNotificationListViewModel: Store {
6161
private let fetchQueryUseCase: FetchPushNotificationQueryUseCase
6262
private let updateQueryUseCase: UpdatePushNotificationQueryUseCase
6363
private let loadingState = LoadingState()
64-
private var undoDeleteNotificationId: String?
64+
private var undoNotificationId: String?
6565
private var cancellable: AnyCancellable?
6666

6767
init(
@@ -104,7 +104,7 @@ final class PushNotificationListViewModel: Store {
104104
effects = reduceByView(action, state: &state)
105105

106106
case .setLoading, .appendNotifications, .resetPagination, .setHasMore,
107-
.syncNotifications, .restoreNotification:
107+
.syncNotifications, .setNotificationHidden:
108108
effects = reduceByRun(action, state: &state)
109109
}
110110

@@ -145,31 +145,23 @@ final class PushNotificationListViewModel: Store {
145145
}
146146

147147
}
148-
case .delete(let item, let index):
149-
beginLoading(.delayed)
148+
case .delete(let item):
150149
Task {
151150
do {
152-
defer { endLoading(.delayed) }
153151
try await deleteUseCase.execute(item.id)
154152
} catch {
155-
send(.restoreNotification(item, index))
153+
send(.setNotificationHidden(item.id, false))
156154
send(.setAlert(isPresented: true))
157155
}
158156
}
159157
case .undoDelete(let notificationId):
160-
beginLoading(.delayed)
161158
Task {
162-
// endLoading(.delayed)를 defer로 두지 않는 이유
163-
// send(.fetchNotifications)가 같은 턴에서 beginLoading(.delayed)를 먼저 올린 뒤
164-
// delayed 로딩을 내려야 같은 isLoading이 끊기지 않기 때문
165159
do {
166160
try await undoDeleteUseCase.execute(notificationId)
167161
} catch {
162+
send(.setNotificationHidden(notificationId, true))
168163
send(.setAlert(isPresented: true))
169164
}
170-
171-
send(.fetchNotifications)
172-
endLoading(.delayed)
173165
}
174166
case .toggleRead(let todoId):
175167
beginLoading(.delayed)
@@ -190,11 +182,11 @@ private extension PushNotificationListViewModel {
190182
func reduceByUser(_ action: Action, state: inout State) -> [SideEffect] {
191183
switch action {
192184
case .deleteNotification(let item):
193-
if let index = state.notifications.firstIndex(where: { $0.id == item.id }) {
194-
undoDeleteNotificationId = item.id
195-
state.notifications.remove(at: index)
185+
if state.notifications.contains(where: { $0.id == item.id }) {
186+
self.undoNotificationId = item.id
187+
setNotificationHidden(&state, notificationId: item.id, isHidden: true)
196188
setToast(&state, isPresented: true)
197-
return [.delete(item, index)]
189+
return [.delete(item)]
198190
}
199191
return []
200192
case .toggleRead(let item):
@@ -203,9 +195,10 @@ private extension PushNotificationListViewModel {
203195
return [.toggleRead(item.todoId)]
204196
}
205197
case .undoDelete:
206-
guard let undoDeleteNotificationId else { return [] }
207-
self.undoDeleteNotificationId = nil
208-
return [.undoDelete(undoDeleteNotificationId)]
198+
guard let undoNotificationId else { return [] }
199+
setNotificationHidden(&state, notificationId: undoNotificationId, isHidden: false)
200+
self.undoNotificationId = nil
201+
return [.undoDelete(undoNotificationId)]
209202
case .setAlert(let isPresented):
210203
setAlert(&state, isPresented: isPresented)
211204
case .toggleSortOption:
@@ -251,7 +244,8 @@ private extension PushNotificationListViewModel {
251244
case .setToast(let isPresented):
252245
setToast(&state, isPresented: isPresented)
253246
if !isPresented {
254-
undoDeleteNotificationId = nil
247+
state.notifications.removeAll { $0.isHidden }
248+
self.undoNotificationId = nil
255249
}
256250
case .setSelectedTodoId(let todoId):
257251
state.selectedTodoId = todoId
@@ -271,24 +265,20 @@ private extension PushNotificationListViewModel {
271265
state.notifications = []
272266
state.nextCursor = nil
273267
case .appendNotifications(let notifications, let nextCursor):
274-
state.notifications.append(contentsOf: notifications)
268+
state.notifications.append(contentsOf: mergedHiddenNotifications(
269+
currentNotifications: state.notifications,
270+
incomingNotifications: notifications
271+
))
275272
state.nextCursor = nextCursor
276273
case .syncNotifications(let notifications, let nextCursor, let hasMore):
277-
state.notifications = notifications
274+
state.notifications = mergedHiddenNotifications(
275+
currentNotifications: state.notifications,
276+
incomingNotifications: notifications
277+
)
278278
state.nextCursor = nextCursor
279279
state.hasMore = hasMore
280-
case .restoreNotification(let notification, let index):
281-
if state.notifications.contains(where: { $0.id == notification.id }) { break }
282-
283-
if index <= state.notifications.count {
284-
state.notifications.insert(notification, at: index)
285-
} else {
286-
state.notifications.append(notification)
287-
}
288-
289-
if undoDeleteNotificationId == notification.id {
290-
undoDeleteNotificationId = nil
291-
}
280+
case .setNotificationHidden(let notificationId, let isHidden):
281+
setNotificationHidden(&state, notificationId: notificationId, isHidden: isHidden)
292282
default:
293283
break
294284
}
@@ -314,6 +304,38 @@ private extension PushNotificationListViewModel {
314304
state.showToast = isPresented
315305
}
316306

307+
func setNotificationHidden(
308+
_ state: inout State,
309+
notificationId: String,
310+
isHidden: Bool
311+
) {
312+
if let notificationIndex = state.notifications.firstIndex(where: {
313+
$0.id == notificationId
314+
}) {
315+
state.notifications[notificationIndex].isHidden = isHidden
316+
}
317+
}
318+
319+
func mergedHiddenNotifications(
320+
currentNotifications: [PushNotificationItem],
321+
incomingNotifications: [PushNotificationItem]
322+
) -> [PushNotificationItem] {
323+
let hiddenNotificationIds = Set(currentNotifications
324+
.filter(\.isHidden)
325+
.map(\.id)
326+
)
327+
328+
return incomingNotifications.map { incomingNotification in
329+
guard hiddenNotificationIds.contains(incomingNotification.id) else {
330+
return incomingNotification
331+
}
332+
333+
var hiddenNotification = incomingNotification
334+
hiddenNotification.isHidden = true
335+
return hiddenNotification
336+
}
337+
}
338+
317339
func startObservingNotifications(
318340
query: PushNotificationQuery,
319341
limit: Int

0 commit comments

Comments
 (0)