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

Option to add new notes or edit existing is disabled #446

Merged
merged 3 commits into from
Feb 25, 2022
Merged

Option to add new notes or edit existing is disabled #446

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f4f4736
Option to add new notes or edit them is disabled
tomasstrba Feb 24, 2022
8436440
Unit tests fixed
tomasstrba Feb 25, 2022
5391e7d
Merge branch 'develop' into tom/adding-notes-disabled
tomasstrba Feb 25, 2022
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
9 changes: 0 additions & 9 deletions DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ final class MoreOptionsMenu: NSMenu {
@objc func openLoginsWithCreditCards(_ sender: NSMenuItem) {
actionDelegate?.optionsButtonMenuRequestedLoginsPopover(self, selectedCategory: .cards)
}

@objc func openLoginsWithNotes(_ sender: NSMenuItem) {
actionDelegate?.optionsButtonMenuRequestedLoginsPopover(self, selectedCategory: .notes)
}

@objc func openPreferences(_ sender: NSMenuItem) {
WindowControllersManager.shared.showPreferencesTab()
Expand Down Expand Up @@ -384,11 +380,6 @@ final class LoginsSubMenu: NSMenu {
.targetting(target)
.withImage(NSImage(named: "CreditCardGlyph"))
.firingPixel(Pixel.Event.MoreResult.loginsMenuCreditCards)

addItem(withTitle: UserText.passwordManagementNotes, action: #selector(MoreOptionsMenu.openLoginsWithNotes), keyEquivalent: "")
.targetting(target)
.withImage(NSImage(named: "NoteGlyph"))
.firingPixel(Pixel.Event.MoreResult.loginsMenuNotes)
}

}
Expand Down
3 changes: 1 addition & 2 deletions DuckDuckGo/SecureVault/Model/PasswordManagementItemListModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ enum SecureVaultItem: Equatable, Identifiable, Comparable {
case .account: return .logins
case .card: return .cards
case .identity: return .identities
case .note: return .notes
case .note: return .allItems
}
}

Expand Down Expand Up @@ -419,7 +419,6 @@ final class PasswordManagementItemListModel: ObservableObject {
case .cards: emptyState = .creditCards
case .logins: emptyState = .logins
case .identities: emptyState = .identities
case .notes: emptyState = .notes
}
}

Expand Down
5 changes: 0 additions & 5 deletions DuckDuckGo/SecureVault/Model/SecureVaultSorting.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,13 @@ struct SecureVaultSorting: Equatable {
case logins
case identities
case cards
case notes

var title: String {
switch self {
case .allItems: return UserText.passwordManagementAllItems
case .logins: return UserText.passwordManagementLogins
case .identities: return UserText.passwordManagementIdentities
case .cards: return UserText.passwordManagementCreditCards
case .notes: return UserText.passwordManagementNotes
}
}

Expand All @@ -48,7 +46,6 @@ struct SecureVaultSorting: Equatable {
case .logins: return "LoginGlyph"
case .identities: return "IdentityGlyph"
case .cards: return "CreditCardGlyph"
case .notes: return "NoteGlyph"
}
}

Expand All @@ -58,7 +55,6 @@ struct SecureVaultSorting: Equatable {
case .logins: return NSColor(named: "LoginsColor")!
case .identities: return NSColor(named: "IdentitiesColor")!
case .cards: return NSColor(named: "CardsColor")!
case .notes: return NSColor(named: "NotesColor")!
}
}

Expand All @@ -68,7 +64,6 @@ struct SecureVaultSorting: Equatable {
case .logins: return .black
case .identities: return .black
case .cards: return .white
case .notes: return .black
}
}
}
Expand Down
22 changes: 16 additions & 6 deletions DuckDuckGo/SecureVault/View/PasswordManagementItemList.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,22 @@ private struct ItemView: View {
}

VStack(alignment: .leading, spacing: 4) {
Text(item.displayTitle)
.foregroundColor(textColor)
.font(font)
Text(item.displaySubtitle)
.foregroundColor(textColor.opacity(0.8))
.font(font)
switch item {
case .note:
Text(item.displayTitle)
.foregroundColor(textColor.opacity(0.7))
.font(font)
Text(item.displaySubtitle)
.foregroundColor(textColor.opacity(0.5))
.font(font)
default:
Text(item.displayTitle)
.foregroundColor(textColor)
.font(font)
Text(item.displaySubtitle)
.foregroundColor(textColor.opacity(0.8))
.font(font)
}
}
.padding(.leading, 4)
}
Expand Down
76 changes: 12 additions & 64 deletions DuckDuckGo/SecureVault/View/PasswordManagementNoteItemView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,16 @@ struct PasswordManagementNoteItemView: View {

if model.note != nil {

let editMode = model.isEditing || model.isNew

ZStack(alignment: .top) {
Spacer()

if editMode {

RoundedRectangle(cornerRadius: 8)
.foregroundColor(Color(NSColor.editingPanelColor))
.shadow(radius: 6)

}

VStack(alignment: .leading, spacing: 0) {

HeaderView()
.padding(.bottom, editMode ? 20 : 30)
.padding(.bottom, 30)

TextView()

Spacer(minLength: 0)

Buttons()
Expand All @@ -78,38 +68,16 @@ private struct Buttons: View {
var body: some View {
HStack {

if model.isEditing && !model.isNew {
Button(UserText.pmDelete) {
model.requestDelete()
}
.buttonStyle(StandardButtonStyle())
}
Text("⚠️ Notes are deprecated.")
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can skip UserText here, since it's temporary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I basically wanted to leave a simple information to users what is happening. I expected something to change during the design review but seems we are moving forward with this and there will be follow-up tasks

.font(.body)
.foregroundColor(Color.secondary)

Spacer()

if model.isEditing || model.isNew {
Button(UserText.pmCancel) {
model.cancel()
}
.buttonStyle(StandardButtonStyle())
Button(UserText.pmSave) {
model.save()
}
.disabled(!model.isDirty)
.buttonStyle(DefaultActionButtonStyle(enabled: model.isDirty))

} else {
Button(UserText.pmDelete) {
model.requestDelete()
}
.buttonStyle(StandardButtonStyle())

Button(UserText.pmEdit) {
model.edit()
}
.buttonStyle(StandardButtonStyle())

Button(UserText.pmDelete) {
model.requestDelete()
}
.buttonStyle(StandardButtonStyle())

}
}
Expand Down Expand Up @@ -143,21 +111,10 @@ private struct TextView: View {
var body: some View {
VStack(alignment: .leading, spacing: 0) {

if model.isEditing || model.isNew {

EditableTextView(text: $model.text)
.overlay(
RoundedRectangle(cornerRadius: 4.0)
.stroke(Color.init(NSColor.tertiaryLabelColor), lineWidth: 1)
)
.padding(.bottom, interItemSpacing)

} else {

HStack {
if #available(macOS 12, *) {
Text(model.text)
.foregroundColor(Color.primary)
.foregroundColor(Color.secondary)
.textSelection(.enabled)
} else {
Text(model.text)
Expand All @@ -166,7 +123,6 @@ private struct TextView: View {
Spacer()
}
.padding(.bottom, interItemSpacing)
}

}
}
Expand All @@ -184,17 +140,9 @@ private struct HeaderView: View {
Image("Note")
.padding(.trailing, 10)

if model.isNew || model.isEditing {

TextField("", text: $model.title)
.font(.title)

} else {

Text(model.title)
.font(.title)

}
Text(model.title)
.font(.title)
.foregroundColor(Color.secondary)

}

Expand Down
41 changes: 1 addition & 40 deletions DuckDuckGo/SecureVault/View/PasswordManagementViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,7 @@ final class PasswordManagementViewController: NSViewController {
menu.items = [
createMenuItem(title: UserText.pmNewCard, action: #selector(createNewCreditCard), imageName: "CreditCardGlyph"),
createMenuItem(title: UserText.pmNewLogin, action: #selector(createNewLogin), imageName: "LoginGlyph"),
createMenuItem(title: UserText.pmNewIdentity, action: #selector(createNewIdentity), imageName: "IdentityGlyph"),
createMenuItem(title: UserText.pmNewNote, action: #selector(createNewNote), imageName: "NoteGlyph")
createMenuItem(title: UserText.pmNewIdentity, action: #selector(createNewIdentity), imageName: "IdentityGlyph")
]

return menu
Expand Down Expand Up @@ -610,9 +609,6 @@ final class PasswordManagementViewController: NSViewController {
case .cards:
let cards = (try? self.secureVault?.creditCards()) ?? []
items = cards.map(SecureVaultItem.card)
case .notes:
let notes = (try? self.secureVault?.notes()) ?? []
items = notes.map(SecureVaultItem.note)
}

DispatchQueue.main.async {
Expand Down Expand Up @@ -723,40 +719,6 @@ final class PasswordManagementViewController: NSViewController {
createNew()
}
}

@objc
private func createNewNote() {
guard let window = view.window else { return }

func createNew() {
createNoteItemView()

listModel?.clearSelection()
itemModel?.createNew()
}

if isDirty {
let alert = NSAlert.passwordManagerSaveChangesToLogin()
alert.beginSheetModal(for: window) { response in

switch response {
case .alertFirstButtonReturn: // Save
self.itemModel?.save()
createNew()

case .alertSecondButtonReturn: // Discard
self.itemModel?.cancel()
createNew()

default: // Cancel
break // just do nothing
}

}
} else {
createNew()
}
}

// MARK: - Empty State

Expand All @@ -778,7 +740,6 @@ final class PasswordManagementViewController: NSViewController {
case .logins: showEmptyState(imageName: "LoginsEmpty", title: UserText.pmEmptyStateLoginsTitle)
case .identities: showEmptyState(imageName: "IdentitiesEmpty", title: UserText.pmEmptyStateIdentitiesTitle)
case .cards: showEmptyState(imageName: "CreditCardsEmpty", title: UserText.pmEmptyStateCardsTitle)
case .notes: showEmptyState(imageName: "NotesEmpty", title: UserText.pmEmptyStateNotesTitle)
}
}

Expand Down