Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alessandro/address bookmarks feedback bookmark logic #2231

Merged
merged 9 commits into from
Feb 26, 2024
Merged

Alessandro/address bookmarks feedback bookmark logic #2231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c8c5978
Initial logic
alessandroboron Feb 20, 2024
02146f6
Inject selected folder from BookmarkDialogView to AddBookmarkFolderDi…
alessandroboron Feb 20, 2024
06d1400
Fixed Previews
alessandroboron Feb 20, 2024
74bc8a9
Refactor logic to use CoordinatorViewModel
alessandroboron Feb 21, 2024
fe2dfbe
Add unit tests for AddEditBookmarkDialogViewModel
alessandroboron Feb 21, 2024
fbd26dc
Add unit tests for CoordinatorViewModel
alessandroboron Feb 21, 2024
c5056a7
Refactor view logic and extract AddEditBookmarkView to be re-used in …
alessandroboron Feb 21, 2024
00b1f99
Fix lint warning
alessandroboron Feb 21, 2024
9a5d181
Fix PR comments
alessandroboron Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions DuckDuckGo/Bookmarks/Services/BookmarkStoreMock.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public final class BookmarkStoreMock: BookmarkStore {
var capturedFolder: BookmarkFolder?
var capturedParentFolder: BookmarkFolder?
var capturedParentFolderType: ParentFolderType?
var capturedBookmark: Bookmark?

var loadAllCalled = false
var bookmarks: [BaseBookmarkEntity]?
Expand All @@ -64,6 +65,8 @@ public final class BookmarkStoreMock: BookmarkStore {
func save(bookmark: Bookmark, parent: BookmarkFolder?, index: Int?, completion: @escaping (Bool, Error?) -> Void) {
saveBookmarkCalled = true
bookmarks?.append(bookmark)
capturedParentFolder = parent
capturedBookmark = bookmark
completion(saveBookmarkSuccess, saveBookmarkError)
}

Expand Down Expand Up @@ -98,6 +101,7 @@ public final class BookmarkStoreMock: BookmarkStore {
}

updateBookmarkCalled = true
capturedBookmark = bookmark
}

var updateFolderCalled = false
Expand Down
53 changes: 16 additions & 37 deletions DuckDuckGo/Bookmarks/View/AddBookmarkPopoverView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,44 +38,22 @@ struct AddBookmarkPopoverView: View {

@MainActor
private var addBookmarkView: some View {
BookmarkDialogContainerView(
AddEditBookmarkView(
alessandroboron marked this conversation as resolved.
Show resolved Hide resolved
title: UserText.Bookmarks.Dialog.Title.addedBookmark,
middleSection: {
BookmarkDialogStackedContentView(
.init(
title: UserText.Bookmarks.Dialog.Field.name,
content: TextField("", text: $model.bookmarkTitle)
.focusedOnAppear()
.accessibilityIdentifier("bookmark.add.name.textfield")
.textFieldStyle(RoundedBorderTextFieldStyle())
.font(.system(size: 14))
),
.init(
title: UserText.Bookmarks.Dialog.Field.location,
content: BookmarkDialogFolderManagementView(
folders: model.folders,
selectedFolder: $model.selectedFolder,
onActionButton: model.addFolderButtonAction
)
)
)
BookmarkFavoriteView(isFavorite: $model.isBookmarkFavorite)
},
bottomSection: {
BookmarkDialogButtonsView(
viewState: .expanded,
otherButtonAction: .init(
title: UserText.remove,
action: model.removeButtonAction
),
defaultButtonAction: .init(
title: UserText.done,
keyboardShortCut: .defaultAction,
isDisabled: model.isDefaultActionButtonDisabled,
action: model.doneButtonAction
)
)
}
buttonsState: .compressed,
bookmarkName: $model.bookmarkTitle,
bookmarkURLPath: nil,
isBookmarkFavorite: $model.isBookmarkFavorite,
folders: model.folders,
selectedFolder: $model.selectedFolder,
isURLFieldHidden: true,
addFolderAction: model.addFolderButtonAction,
otherActionTitle: UserText.remove,
isOtherActionDisabled: false,
otherAction: model.removeButtonAction,
defaultActionTitle: UserText.done,
isDefaultActionDisabled: model.isDefaultActionButtonDisabled,
defaultAction: model.doneButtonAction
)
.padding(.vertical, 16.0)
.font(.system(size: 13))
Expand Down Expand Up @@ -112,6 +90,7 @@ struct AddBookmarkPopoverView: View {
let bkm = Bookmark(id: "n", url: URL.duckDuckGo.absoluteString, title: "DuckDuckGo", isFavorite: false, parentFolderUUID: "1")
let bkman = LocalBookmarkManager(bookmarkStore: BookmarkStoreMock(bookmarks: [
BookmarkFolder(id: "1", title: "Folder with a name that shouldn‘t fit into the picker", children: [])]))
bkman.loadBookmarks()

return AddBookmarkPopoverView(model: AddBookmarkPopoverViewModel(bookmark: bkm, bookmarkManager: bkman))
.preferredColorScheme(.dark)
Expand Down
201 changes: 79 additions & 122 deletions DuckDuckGo/Bookmarks/View/Dialog/AddEditBookmarkDialogView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,158 +17,115 @@
//

import SwiftUI
import Combine
import Common

@MainActor
final class AddEditBookmarkDialogViewModel: ObservableObject {

enum Mode {
case add
case edit
}

@Published var bookmarkName: String
@Published var bookmarkURLPath: String
@Published var isBookmarkFavorite: Bool
@Published var folders: [FolderViewModel] = []
@Published var selectedFolder: BookmarkFolder?

let title: String = ""
let defaultActionTitle = ""
var isDefaultActionButtonDisabled: Bool { false }

private let bookmark: Bookmark
private let mode: Mode
private let bookmarkManager: BookmarkManager

init(
mode: Mode,
bookmark: Bookmark,
bookmarkManager: LocalBookmarkManager = .shared
) {
self.bookmark = bookmark
self.mode = mode
self.bookmarkManager = bookmarkManager
bookmarkName = bookmark.title
bookmarkURLPath = bookmark.url
isBookmarkFavorite = bookmark.isFavorite
}

func cancelAction(dismiss: () -> Void) {}
func saveOrAddAction(dismiss: () -> Void) {}
func addFolderButtonAction() {}
}

struct AddEditBookmarkDialogView: ModalView {
@ObservedObject private var viewModel: AddEditBookmarkDialogViewModel
@ObservedObject private var viewModel: AddEditBookmarkDialogCoordinatorViewModel<AddEditBookmarkDialogViewModel, AddEditBookmarkFolderDialogViewModel>

init(viewModel: AddEditBookmarkDialogViewModel) {
init(viewModel: AddEditBookmarkDialogCoordinatorViewModel<AddEditBookmarkDialogViewModel, AddEditBookmarkFolderDialogViewModel>) {
self.viewModel = viewModel
}

var body: some View {
BookmarkDialogContainerView(
title: viewModel.title,
middleSection: {
BookmarkDialogStackedContentView(
.init(
title: UserText.Bookmarks.Dialog.Field.name,
content: TextField("", text: $viewModel.bookmarkName)
.focusedOnAppear()
.accessibilityIdentifier("bookmark.add.name.textfield")
.textFieldStyle(RoundedBorderTextFieldStyle())
.font(.system(size: 14))
),
.init(
title: UserText.Bookmarks.Dialog.Field.url,
content: TextField("", text: $viewModel.bookmarkURLPath)
.accessibilityIdentifier("bookmark.add.url.textfield")
.textFieldStyle(RoundedBorderTextFieldStyle())
.font(.system(size: 14))
),
.init(
title: UserText.Bookmarks.Dialog.Field.location,
content: BookmarkDialogFolderManagementView(
folders: viewModel.folders,
selectedFolder: $viewModel.selectedFolder,
onActionButton: viewModel.addFolderButtonAction
)
)
)
BookmarkFavoriteView(isFavorite: $viewModel.isBookmarkFavorite)
},
bottomSection: {
BookmarkDialogButtonsView(
viewState: .compressed,
otherButtonAction: .init(
title: UserText.cancel,
keyboardShortCut: .cancelAction,
action: viewModel.cancelAction
), defaultButtonAction: .init(
title: viewModel.defaultActionTitle,
keyboardShortCut: .defaultAction,
isDisabled: viewModel.isDefaultActionButtonDisabled,
action: viewModel.saveOrAddAction
)
)
Group {
switch viewModel.viewState {
case .bookmark:
addEditBookmarkView
case .folder:
addFolderView
}
)
}
.font(.system(size: 13))
.frame(width: 448, height: 288)
}
}

// MARK: - AddEditBookmarkDialogViewModel.Mode

private extension AddEditBookmarkDialogViewModel.Mode {

var title: String {
switch self {
case .add:
return UserText.Bookmarks.Dialog.Title.addBookmark
case .edit:
return UserText.Bookmarks.Dialog.Title.editBookmark
}
private var addEditBookmarkView: some View {
AddEditBookmarkView(
title: viewModel.bookmarkModel.title,
buttonsState: .compressed,
bookmarkName: $viewModel.bookmarkModel.bookmarkName,
bookmarkURLPath: $viewModel.bookmarkModel.bookmarkURLPath,
isBookmarkFavorite: $viewModel.bookmarkModel.isBookmarkFavorite,
folders: viewModel.bookmarkModel.folders,
selectedFolder: $viewModel.bookmarkModel.selectedFolder,
isURLFieldHidden: false,
addFolderAction: viewModel.addFolderAction,
otherActionTitle: viewModel.bookmarkModel.cancelActionTitle,
isOtherActionDisabled: viewModel.bookmarkModel.isOtherActionDisabled,
otherAction: viewModel.bookmarkModel.cancel,
defaultActionTitle: viewModel.bookmarkModel.defaultActionTitle,
isDefaultActionDisabled: viewModel.bookmarkModel.isDefaultActionDisabled,
defaultAction: viewModel.bookmarkModel.addOrSave
)
.frame(width: 448, height: 288)
}

var defaultActionTitle: String {
switch self {
case .add:
return UserText.Bookmarks.Dialog.Action.addBookmark
case .edit:
return UserText.save
}
private var addFolderView: some View {
AddEditBookmarkFolderView(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For both isn’t it better to pass the view model directly? I assume you will want to use the view from all the other places and don’t want to do the list every time

Copy link
Contributor Author

@alessandroboron alessandroboron Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get your point, the reason I’m not currently passing the VM is because the “Popover” we show from the address bar re-use the same views but have a different VM due to the live updates. Having said, that I should be able (🤞) to re-use the protocols I made for the new VMs and inject that one to the view. The thing is as soon as I pass the protocol I have to make the Views generic over the ViewModel due to any <viewModelProtocol> and not being able to use property wrappers.

title: viewModel.folderModel.title,
buttonsState: .compressed,
folders: viewModel.folderModel.folders,
folderName: $viewModel.folderModel.folderName,
selectedFolder: $viewModel.folderModel.selectedFolder,
cancelActionTitle: viewModel.folderModel.cancelActionTitle,
isCancelActionDisabled: viewModel.folderModel.isOtherActionDisabled,
cancelAction: { _ in
viewModel.dismissAction()
},
defaultActionTitle: viewModel.folderModel.defaultActionTitle,
isDefaultActionDisabled: viewModel.folderModel.isDefaultActionDisabled,
defaultAction: { _ in
viewModel.folderModel.addOrSave {
viewModel.dismissAction()
}
}
)
.frame(width: 448, height: 210)
}

}

// MARK: - Previews

#if DEBUG
#Preview("Add Bookmark - Light Mode") {
let bookmark = Bookmark(id: "1", url: "", title: "", isFavorite: false)
let viewModel = AddEditBookmarkDialogViewModel(mode: .add, bookmark: bookmark)
let bookmarkManager = LocalBookmarkManager(bookmarkStore: BookmarkStoreMock(bookmarks: []))
bookmarkManager.loadBookmarks()
let bookmarkViewModel = AddEditBookmarkDialogViewModel(mode: .add(), bookmarkManager: bookmarkManager)
let folderViewModel = AddEditBookmarkFolderDialogViewModel(mode: .add(parentFolder: nil), bookmarkManager: bookmarkManager)
let viewModel = AddEditBookmarkDialogCoordinatorViewModel(bookmarkModel: bookmarkViewModel, folderModel: folderViewModel)
return AddEditBookmarkDialogView(viewModel: viewModel)
.preferredColorScheme(.light)
}

#Preview("Edit Bookmark - Light Mode") {
let bookmark = Bookmark(id: "1", url: "www.duckduckgo.com", title: "DuckDuckGo", isFavorite: true)
let viewModel = AddEditBookmarkDialogViewModel(mode: .edit, bookmark: bookmark)
#Preview("Add Bookmark - Light Mode") {
let bookmarkManager = LocalBookmarkManager(bookmarkStore: BookmarkStoreMock(bookmarks: []))
bookmarkManager.loadBookmarks()
let bookmarkViewModel = AddEditBookmarkDialogViewModel(mode: .add(), bookmarkManager: bookmarkManager)
let folderViewModel = AddEditBookmarkFolderDialogViewModel(mode: .add(parentFolder: nil), bookmarkManager: bookmarkManager)
let viewModel = AddEditBookmarkDialogCoordinatorViewModel(bookmarkModel: bookmarkViewModel, folderModel: folderViewModel)
return AddEditBookmarkDialogView(viewModel: viewModel)
.preferredColorScheme(.light)
.preferredColorScheme(.dark)
}

#Preview("Add Bookmark - Dark Mode") {
let bookmark = Bookmark(id: "1", url: "", title: "", isFavorite: false)
let viewModel = AddEditBookmarkDialogViewModel(mode: .add, bookmark: bookmark)
#Preview("Edit Bookmark - Light Mode") {
let parentFolder = BookmarkFolder(id: "7", title: "DuckDuckGo")
let bookmark = Bookmark(id: "1", url: "www.duckduckgo.com", title: "DuckDuckGo", isFavorite: true, parentFolderUUID: "7")
let bookmarkManager = LocalBookmarkManager(bookmarkStore: BookmarkStoreMock(bookmarks: [bookmark, parentFolder]))
bookmarkManager.loadBookmarks()
let bookmarkViewModel = AddEditBookmarkDialogViewModel(mode: .edit(bookmark: bookmark), bookmarkManager: bookmarkManager)
let folderViewModel = AddEditBookmarkFolderDialogViewModel(mode: .add(parentFolder: nil), bookmarkManager: bookmarkManager)
let viewModel = AddEditBookmarkDialogCoordinatorViewModel(bookmarkModel: bookmarkViewModel, folderModel: folderViewModel)
return AddEditBookmarkDialogView(viewModel: viewModel)
.preferredColorScheme(.dark)
.preferredColorScheme(.light)
}

#Preview("Edit Bookmark - Dark Mode") {
let bookmark = Bookmark(id: "1", url: "www.duckduckgo.com", title: "DuckDuckGo", isFavorite: true)
let viewModel = AddEditBookmarkDialogViewModel(mode: .edit, bookmark: bookmark)
let parentFolder = BookmarkFolder(id: "7", title: "DuckDuckGo")
let bookmark = Bookmark(id: "1", url: "www.duckduckgo.com", title: "DuckDuckGo", isFavorite: true, parentFolderUUID: "7")
let bookmarkManager = LocalBookmarkManager(bookmarkStore: BookmarkStoreMock(bookmarks: [bookmark, parentFolder]))
bookmarkManager.loadBookmarks()
let bookmarkViewModel = AddEditBookmarkDialogViewModel(mode: .edit(bookmark: bookmark), bookmarkManager: bookmarkManager)
let folderViewModel = AddEditBookmarkFolderDialogViewModel(mode: .add(parentFolder: nil), bookmarkManager: bookmarkManager)
let viewModel = AddEditBookmarkDialogCoordinatorViewModel(bookmarkModel: bookmarkViewModel, folderModel: folderViewModel)
return AddEditBookmarkDialogView(viewModel: viewModel)
.preferredColorScheme(.dark)
}
#endif
4 changes: 2 additions & 2 deletions DuckDuckGo/Bookmarks/View/Dialog/AddEditBookmarkFolderDialogView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ struct AddEditBookmarkFolderDialogView: ModalView {
folderName: $viewModel.folderName,
selectedFolder: $viewModel.selectedFolder,
cancelActionTitle: viewModel.cancelActionTitle,
isCancelActionDisabled: viewModel.isCancelActionDisabled,
isCancelActionDisabled: viewModel.isOtherActionDisabled,
cancelAction: viewModel.cancel,
defaultActionTitle: viewModel.defaultActionTitle,
isDefaultActionDisabled: viewModel.isDefaultActionButtonDisabled,
isDefaultActionDisabled: viewModel.isDefaultActionDisabled,
defaultAction: viewModel.addOrSave
)
.font(.system(size: 13))
Expand Down
Loading
Loading