From 20ce0713ad51cc6200600666daeafafd34043678 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Wed, 23 Aug 2023 15:20:07 +0200 Subject: [PATCH] Revert "Autofill Letter Icons (#1475)" This reverts commit f03f6ac1b15e3ea1f4e8097d0ddcced7185ad0f5. --- .../Common/View/SwiftUI/FaviconView.swift | 2 +- .../View/SwiftUI/LoginFaviconView.swift | 29 +++++----- .../PinnedTabs/View/PinnedTabView.swift | 2 +- .../PasswordManagementItemListModel.swift | 15 +---- .../Model/PasswordManagementLoginModel.swift | 19 ++---- .../View/PasswordManagementItemList.swift | 17 +----- .../PasswordManagementLoginItemView.swift | 12 +--- .../PasswordManagementViewController.swift | 10 +--- .../SwiftUIExtensions/ColorExtensions.swift | 4 +- .../SwiftUIExtensions/LetterIconView.swift | 58 ------------------- .../PasswordManagementItemModelTests.swift | 24 ++------ 11 files changed, 34 insertions(+), 158 deletions(-) delete mode 100644 LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/LetterIconView.swift diff --git a/DuckDuckGo/Common/View/SwiftUI/FaviconView.swift b/DuckDuckGo/Common/View/SwiftUI/FaviconView.swift index fb987011c6..0c7c4f8be8 100644 --- a/DuckDuckGo/Common/View/SwiftUI/FaviconView.swift +++ b/DuckDuckGo/Common/View/SwiftUI/FaviconView.swift @@ -73,7 +73,7 @@ struct FaviconView: View { ZStack { let eTLDplus1 = ContentBlocking.shared.tld.eTLDplus1(domain) ?? domain Rectangle() - .foregroundColor(Color.forString(eTLDplus1)) + .foregroundColor(Color.forDomain(eTLDplus1)) Text(String(eTLDplus1.capitalized.first ?? "?")) .font(.title) .foregroundColor(Color.white) diff --git a/DuckDuckGo/Common/View/SwiftUI/LoginFaviconView.swift b/DuckDuckGo/Common/View/SwiftUI/LoginFaviconView.swift index 7966ead67f..adb9621c81 100644 --- a/DuckDuckGo/Common/View/SwiftUI/LoginFaviconView.swift +++ b/DuckDuckGo/Common/View/SwiftUI/LoginFaviconView.swift @@ -17,30 +17,29 @@ // import SwiftUI -import BrowserServicesKit -import SwiftUIExtensions struct LoginFaviconView: View { - let domain: String - let generatedIconLetters: String + + let domain: String? + let faviconManagement: FaviconManagement = FaviconManager.shared var body: some View { - Group { - if let image = faviconManagement.getCachedFavicon(for: domain, sizeCategory: .small)?.image { - Image(nsImage: image) - .resizable() - .aspectRatio(contentMode: .fit) - .frame(width: 32) - .cornerRadius(4.0) - .padding(.leading, 6) - } else { - LetterIconView(title: generatedIconLetters) - } + + if let image = favicon { + Image(nsImage: image) + .resizable() + .aspectRatio(contentMode: .fit) + .frame(width: 32) + .cornerRadius(4.0) } + } var favicon: NSImage? { + guard let domain else { + return NSImage(named: "Login") + } return faviconManagement.getCachedFavicon(for: domain, sizeCategory: .small)?.image ?? NSImage(named: "Login") } diff --git a/DuckDuckGo/PinnedTabs/View/PinnedTabView.swift b/DuckDuckGo/PinnedTabs/View/PinnedTabView.swift index 873aac4cad..c7c3671c33 100644 --- a/DuckDuckGo/PinnedTabs/View/PinnedTabView.swift +++ b/DuckDuckGo/PinnedTabs/View/PinnedTabView.swift @@ -195,7 +195,7 @@ struct PinnedTabInnerView: View { } else if let domain = model.content.url?.host, let eTLDplus1 = ContentBlocking.shared.tld.eTLDplus1(domain), let firstLetter = eTLDplus1.capitalized.first.flatMap(String.init) { ZStack { Rectangle() - .foregroundColor(.forString(eTLDplus1)) + .foregroundColor(.forDomain(eTLDplus1)) Text(firstLetter) .font(.caption) .foregroundColor(.white) diff --git a/DuckDuckGo/SecureVault/Model/PasswordManagementItemListModel.swift b/DuckDuckGo/SecureVault/Model/PasswordManagementItemListModel.swift index 0a2963ee15..5adb5da5cc 100644 --- a/DuckDuckGo/SecureVault/Model/PasswordManagementItemListModel.swift +++ b/DuckDuckGo/SecureVault/Model/PasswordManagementItemListModel.swift @@ -274,18 +274,11 @@ final class PasswordManagementItemListModel: ObservableObject { @Published var canChangeCategory: Bool = true private var onItemSelected: (_ old: SecureVaultItem?, _ new: SecureVaultItem?) -> Void - private let tld: TLD - private let urlMatcher: AutofillDomainNameUrlMatcher - private static let randomColorsCount = 15 init(passwordManagerCoordinator: PasswordManagerCoordinating, - onItemSelected: @escaping (_ old: SecureVaultItem?, _ new: SecureVaultItem?) -> Void, - urlMatcher: AutofillDomainNameUrlMatcher = AutofillDomainNameUrlMatcher(), - tld: TLD = ContentBlocking.shared.tld) { + onItemSelected: @escaping (_ old: SecureVaultItem?, _ new: SecureVaultItem?) -> Void) { self.onItemSelected = onItemSelected self.passwordManagerCoordinator = passwordManagerCoordinator - self.urlMatcher = urlMatcher - self.tld = tld } func update(items: [SecureVaultItem]) { @@ -458,10 +451,4 @@ final class PasswordManagementItemListModel: ObservableObject { } } - func tldForAccount(_ account: SecureVaultModels.WebsiteAccount) -> String { - let name = account.name(tld: tld, autofillDomainNameUrlMatcher: urlMatcher) - let title = (account.title?.isEmpty == false) ? account.title! : "#" - return tld.eTLDplus1(name) ?? title - } - } diff --git a/DuckDuckGo/SecureVault/Model/PasswordManagementLoginModel.swift b/DuckDuckGo/SecureVault/Model/PasswordManagementLoginModel.swift index 050d33a83a..7e7cec66d1 100644 --- a/DuckDuckGo/SecureVault/Model/PasswordManagementLoginModel.swift +++ b/DuckDuckGo/SecureVault/Model/PasswordManagementLoginModel.swift @@ -18,7 +18,6 @@ import Combine import BrowserServicesKit -import Common final class PasswordManagementLoginModel: ObservableObject, PasswordManagementItemModel { @@ -33,7 +32,7 @@ final class PasswordManagementLoginModel: ObservableObject, PasswordManagementIt var onSaveRequested: (SecureVaultModels.WebsiteCredentials) -> Void var onDeleteRequested: (SecureVaultModels.WebsiteCredentials) -> Void - var urlMatcher: AutofillDomainNameUrlMatcher + var urlMatcher: AutofillUrlMatcher var emailManager: EmailManager var isEditingPublisher: Published.Publisher { @@ -53,7 +52,6 @@ final class PasswordManagementLoginModel: ObservableObject, PasswordManagementIt @Published var notes: String = "" @Published var isEditing = false @Published var isNew = false - @Published var domainTLD = "" var isDirty: Bool { title != "" || username != "" || password != "" || domain != "" || notes != "" @@ -123,23 +121,16 @@ final class PasswordManagementLoginModel: ObservableObject, PasswordManagementIt usernameIsPrivateEmail && privateEmailMessage != "" } - private let tld: TLD - private let urlSort: AutofillDomainNameUrlSort - private static let randomColorsCount = 15 - init(onSaveRequested: @escaping (SecureVaultModels.WebsiteCredentials) -> Void, onDeleteRequested: @escaping (SecureVaultModels.WebsiteCredentials) -> Void, - urlMatcher: AutofillDomainNameUrlMatcher, - emailManager: EmailManager, - tld: TLD = ContentBlocking.shared.tld, - urlSort: AutofillDomainNameUrlSort) { + urlMatcher: AutofillUrlMatcher = AutofillDomainNameUrlMatcher(), + emailManager: EmailManager = EmailManager()) { self.onSaveRequested = onSaveRequested self.onDeleteRequested = onDeleteRequested self.urlMatcher = urlMatcher self.emailManager = emailManager - self.tld = tld - self.urlSort = urlSort self.emailManager.requestDelegate = self + } func setSecureVaultModel(_ modelObject: Model) { @@ -210,8 +201,6 @@ final class PasswordManagementLoginModel: ObservableObject, PasswordManagementIt domain = urlMatcher.normalizeUrlForWeb(credentials?.account.domain ?? "") notes = credentials?.account.notes ?? "" isNew = credentials?.account.id == nil - let name = credentials?.account.name(tld: tld, autofillDomainNameUrlMatcher: urlMatcher) - domainTLD = tld.eTLDplus1(name) ?? credentials?.account.title ?? "#" // Determine Private Email Status when required usernameIsPrivateEmail = emailManager.isPrivateEmail(email: username) diff --git a/DuckDuckGo/SecureVault/View/PasswordManagementItemList.swift b/DuckDuckGo/SecureVault/View/PasswordManagementItemList.swift index a8d4a4a493..bb2fa850d1 100644 --- a/DuckDuckGo/SecureVault/View/PasswordManagementItemList.swift +++ b/DuckDuckGo/SecureVault/View/PasswordManagementItemList.swift @@ -20,7 +20,6 @@ import Foundation import SwiftUI import BrowserServicesKit import Combine -import SwiftUIExtensions struct ScrollOffsetKey: PreferenceKey { typealias Value = CGFloat @@ -268,13 +267,6 @@ private struct ItemView: View { let item: SecureVaultItem let action: () -> Void - func getIconLetters(account: SecureVaultModels.WebsiteAccount) -> String { - if let title = account.title, !title.isEmpty { - return title - } - return model.tldForAccount(account) - } - var body: some View { let selected = model.selected == item @@ -285,12 +277,9 @@ private struct ItemView: View { HStack(spacing: 2) { switch item { - case .account: - if let account = item.websiteAccount, let domain = account.domain { - LoginFaviconView(domain: domain, generatedIconLetters: getIconLetters(account: account)) - } else { - LetterIconView(title: "#") - } + case .account(let account): + LoginFaviconView(domain: account.domain) + .padding(.leading, 6) case .card: Image("Card") .frame(width: 32) diff --git a/DuckDuckGo/SecureVault/View/PasswordManagementLoginItemView.swift b/DuckDuckGo/SecureVault/View/PasswordManagementLoginItemView.swift index d9446010bd..6622c148aa 100644 --- a/DuckDuckGo/SecureVault/View/PasswordManagementLoginItemView.swift +++ b/DuckDuckGo/SecureVault/View/PasswordManagementLoginItemView.swift @@ -588,18 +588,12 @@ private struct HeaderView: View { @EnvironmentObject var model: PasswordManagementLoginModel - private func getIconLetters() -> String { - return !model.title.isEmpty ? model.title : - !model.domainTLD.isEmpty ? model.domainTLD : - "#" - } - var body: some View { HStack(alignment: .center, spacing: 0) { - LoginFaviconView(domain: model.domain, - generatedIconLetters: getIconLetters()) - .padding(.trailing, 10) + + LoginFaviconView(domain: model.domain) + .padding(.trailing, 10) if model.isNew || model.isEditing { diff --git a/DuckDuckGo/SecureVault/View/PasswordManagementViewController.swift b/DuckDuckGo/SecureVault/View/PasswordManagementViewController.swift index 97db87bad6..24fa8ba39a 100644 --- a/DuckDuckGo/SecureVault/View/PasswordManagementViewController.swift +++ b/DuckDuckGo/SecureVault/View/PasswordManagementViewController.swift @@ -149,11 +149,6 @@ final class PasswordManagementViewController: NSViewController { private let passwordManagerCoordinator: PasswordManagerCoordinating = PasswordManagerCoordinator.shared - private let emailManager = EmailManager() - private let urlMatcher = AutofillDomainNameUrlMatcher() - private let tld = ContentBlocking.shared.tld - private let urlSort = AutofillDomainNameUrlSort() - override func viewDidLoad() { super.viewDidLoad() createListView() @@ -400,10 +395,7 @@ final class PasswordManagementViewController: NSViewController { self?.doSaveCredentials(credentials) }, onDeleteRequested: { [weak self] credentials in self?.promptToDelete(credentials: credentials) - }, - urlMatcher: urlMatcher, - emailManager: emailManager, - urlSort: urlSort) + }) self.itemModel = itemModel diff --git a/LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/ColorExtensions.swift b/LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/ColorExtensions.swift index 332ee151d5..e8f8c55407 100644 --- a/LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/ColorExtensions.swift +++ b/LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/ColorExtensions.swift @@ -31,9 +31,9 @@ public extension Color { ) } - static func forString(_ string: String) -> Color { + static func forDomain(_ domain: String) -> Color { var consistentHash: Int { - return string.utf8 + return domain.utf8 .map { return $0 } .reduce(5381) { ($0 << 5) &+ $0 &+ Int($1) } } diff --git a/LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/LetterIconView.swift b/LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/LetterIconView.swift deleted file mode 100644 index d1dfab824d..0000000000 --- a/LocalPackages/SwiftUIExtensions/Sources/SwiftUIExtensions/LetterIconView.swift +++ /dev/null @@ -1,58 +0,0 @@ -// -// FaviconLetterView.swift -// -// Copyright © 2023 DuckDuckGo. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import SwiftUI - -public struct LetterIconView: View { - - public var title: String - public var size: CGFloat - public var prefferedFirstCharacters: String? - public var characterCount: Int - private var padding: CGFloat = 0.33 - - private var characters: String { - if let prefferedFirstCharacters = prefferedFirstCharacters, - prefferedFirstCharacters != "" { - return String(prefferedFirstCharacters.prefix(characterCount)) - } - return String(title.prefix(characterCount)) - } - - public init(title: String, size: CGFloat = 32, prefferedFirstCharacters: String? = nil, characterCount: Int = 2) { - self.title = title - self.size = size - self.prefferedFirstCharacters = prefferedFirstCharacters - self.characterCount = characterCount - } - - public var body: some View { - ZStack { - RoundedRectangle(cornerRadius: size * 0.125) - .foregroundColor(Color.forString(title)) - .frame(width: size, height: size) - - Text(characters.capitalized(with: .current)) - .frame(width: size - (size * padding), height: size - (size * padding)) - .foregroundColor(.white) - .minimumScaleFactor(0.01) - .font(.system(size: size, weight: .semibold)) - } - .padding(.leading, 8) - } -} diff --git a/UnitTests/SecureVault/PasswordManagementItemModelTests.swift b/UnitTests/SecureVault/PasswordManagementItemModelTests.swift index e5703c1473..f53762fcec 100644 --- a/UnitTests/SecureVault/PasswordManagementItemModelTests.swift +++ b/UnitTests/SecureVault/PasswordManagementItemModelTests.swift @@ -25,17 +25,10 @@ final class PasswordManagementItemModelTests: XCTestCase { var isDirty = false var savedCredentials: SecureVaultModels.WebsiteCredentials? var deletedCredentials: SecureVaultModels.WebsiteCredentials? - var urlMatcher = AutofillDomainNameUrlMatcher() - var emailManager = EmailManager() - var tld = ContentBlocking.shared.tld - var urlSort = AutofillDomainNameUrlSort() func testWhenCredentialsAreSavedThenSaveIsRequested() { let model = PasswordManagementLoginModel(onSaveRequested: onSaveRequested, - onDeleteRequested: onDeleteRequested, - urlMatcher: urlMatcher, - emailManager: emailManager, - urlSort: urlSort) + onDeleteRequested: onDeleteRequested) model.credentials = makeCredentials(id: "1") model.save() @@ -46,10 +39,7 @@ final class PasswordManagementItemModelTests: XCTestCase { func testWhenCredentialsAreDeletedThenDeleteIsRequested() { let model = PasswordManagementLoginModel(onSaveRequested: onSaveRequested, - onDeleteRequested: onDeleteRequested, - urlMatcher: urlMatcher, - emailManager: emailManager, - urlSort: urlSort) + onDeleteRequested: onDeleteRequested) model.credentials = makeCredentials(id: "1") model.requestDelete() @@ -60,10 +50,7 @@ final class PasswordManagementItemModelTests: XCTestCase { func testWhenCredentialsHasNoIdThenModelStateIsNew() { let model = PasswordManagementLoginModel(onSaveRequested: onSaveRequested, - onDeleteRequested: onDeleteRequested, - urlMatcher: urlMatcher, - emailManager: emailManager, - urlSort: urlSort) + onDeleteRequested: onDeleteRequested) model.createNew() @@ -75,10 +62,7 @@ final class PasswordManagementItemModelTests: XCTestCase { func testWhenModelIsEditedThenStateIsUpdated() { let model = PasswordManagementLoginModel(onSaveRequested: onSaveRequested, - onDeleteRequested: onDeleteRequested, - urlMatcher: urlMatcher, - emailManager: emailManager, - urlSort: urlSort) + onDeleteRequested: onDeleteRequested) model.credentials = makeCredentials(id: "1") XCTAssertEqual(model.domain, "domain")