Add FXIOS-9189 ⁃ Display url in address toolbar in Client (#20556)

* Remove duplicated font assigment * Display search engine icon in address toolbar * Fix build errors in sample browser * Position search engine icon * Add delegate for searching * Display url in address toolbar * Fix build error in sample browser * Use url instead of string * Update text field leading constraint correctly * Resign editing when location view is getting newly configured * Display correct icon in container * Fix Swiftlint warnings * Fix build in sample browser * Add search term * Add unit tests * Fix sample browser build * Fix Swiftlint warning
mozilla-mobile · Jun 6, 2024 · a558083 · a558083
1 parent e166887
commit a558083
Show file tree

Hide file tree

Showing 21 changed files with 244 additions and 89 deletions.
diff --git a/BrowserKit/Sources/ToolbarKit/AddressToolbarDelegate.swift b/BrowserKit/Sources/ToolbarKit/AddressToolbarDelegate.swift
@@ -8,5 +8,4 @@ public protocol AddressToolbarDelegate: AnyObject {
     func searchSuggestions(searchTerm: String)
     func openBrowser(searchTerm: String)
     func openSuggestions(searchTerm: String)
-    func shouldDisplayTextForURL(_ url: URL?) -> String?
 }
diff --git a/BrowserKit/Sources/ToolbarKit/BrowserAddressToolbar.swift b/BrowserKit/Sources/ToolbarKit/BrowserAddressToolbar.swift
@@ -238,10 +238,6 @@ public class BrowserAddressToolbar: UIView, AddressToolbar, ThemeApplicable, Loc
         toolbarDelegate?.openBrowser(searchTerm: text.lowercased())
     }
 
-    func locationViewDisplayTextForURL(_ url: URL?) -> String? {
-        toolbarDelegate?.shouldDisplayTextForURL(url)
-    }
-
     // MARK: - ThemeApplicable
     public func applyTheme(theme: Theme) {
         backgroundColor = theme.colors.layer1

diff --git a/BrowserKit/Sources/ToolbarKit/LocationView/LocationView.swift b/BrowserKit/Sources/ToolbarKit/LocationView/LocationView.swift
@@ -17,6 +17,7 @@ class LocationView: UIView, UITextFieldDelegate, ThemeApplicable {
     }
 
     private var urlAbsolutePath: String?
+    private var searchTerm: String?
     private var notifyTextChanged: (() -> Void)?
     private var onTapLockIcon: (() -> Void)?
     private var locationViewDelegate: LocationViewDelegate?
@@ -35,6 +36,13 @@ class LocationView: UIView, UITextFieldDelegate, ThemeApplicable {
         return urlTextFieldWidth >= locationViewWidth
     }
 
+    private var dotWidth: CGFloat {
+        guard let font = urlTextField.font else { return 0 }
+        let fontAttributes = [NSAttributedString.Key.font: font]
+        let width = "...".size(withAttributes: fontAttributes).width
+        return CGFloat(width)
+    }
+
     private lazy var urlTextFieldSubdomainColor: UIColor = .clear
     private lazy var gradientLayer = CAGradientLayer()
     private lazy var gradientView: UIView = .build()
@@ -104,7 +112,9 @@ class LocationView: UIView, UITextFieldDelegate, ThemeApplicable {
         configureURLTextField(state)
         configureA11y(state)
         formatAndTruncateURLTextField()
+        updateIconContainer()
         locationViewDelegate = delegate
+        searchTerm = state.searchTerm
     }
 
     // MARK: - Layout
@@ -126,6 +136,10 @@ class LocationView: UIView, UITextFieldDelegate, ThemeApplicable {
         searchEngineContentView.addSubview(searchEngineImageView)
         iconContainerStackView.addArrangedSubview(searchEngineContentView)
 
+        urlTextFieldLeadingConstraint = urlTextField.leadingAnchor.constraint(
+            equalTo: iconContainerStackView.trailingAnchor)
+        urlTextFieldLeadingConstraint?.isActive = true
+
         NSLayoutConstraint.activate([
             gradientView.topAnchor.constraint(equalTo: urlTextField.topAnchor),
             gradientView.bottomAnchor.constraint(equalTo: urlTextField.bottomAnchor),
@@ -174,35 +188,40 @@ class LocationView: UIView, UITextFieldDelegate, ThemeApplicable {
 
         // hide the leading "..." by moving them behind the lock icon
         if shouldAdjustForOverflow {
-            updateURLTextFieldLeadingConstraint(equalTo: iconContainerStackView.leadingAnchor)
+            updateURLTextFieldLeadingConstraint(constant: -dotWidth)
         } else if shouldAdjustForNonEmpty {
-            updateURLTextFieldLeadingConstraint(equalTo: iconContainerStackView.trailingAnchor)
+            updateURLTextFieldLeadingConstraint()
         } else {
-            updateURLTextFieldLeadingConstraint(equalTo: iconContainerStackView.trailingAnchor,
-                                                constant: UX.horizontalSpace)
+            updateURLTextFieldLeadingConstraint(constant: UX.horizontalSpace)
         }
     }
 
-    private func updateURLTextFieldLeadingConstraint(equalTo anchor: NSLayoutXAxisAnchor, constant: CGFloat = 0) {
-        urlTextFieldLeadingConstraint?.isActive = false
-        urlTextFieldLeadingConstraint = urlTextField.leadingAnchor.constraint(equalTo: anchor, constant: constant)
-        urlTextFieldLeadingConstraint?.isActive = true
+    private func updateURLTextFieldLeadingConstraint(constant: CGFloat = 0) {
+        urlTextFieldLeadingConstraint?.constant = constant
     }
 
     private func removeContainerIcons() {
         iconContainerStackView.removeAllArrangedViews()
     }
 
+    private func updateIconContainer() {
+        guard !urlTextField.isEditing else {
+            updateUIForSearchEngineDisplay()
+            return
+        }
+
+        if isURLTextFieldEmpty {
+            updateUIForSearchEngineDisplay()
+        } else {
+            updateUIForLockIconDisplay()
+        }
+    }
+
     private func updateUIForSearchEngineDisplay() {
         removeContainerIcons()
         iconContainerStackView.addArrangedSubview(searchEngineContentView)
         urlTextFieldLeadingConstraint?.constant = UX.horizontalSpace
-
-        updateURLTextFieldLeadingConstraint(
-            equalTo: iconContainerStackView.trailingAnchor,
-            constant: UX.horizontalSpace
-        )
-
+        updateURLTextFieldLeadingConstraint(constant: UX.horizontalSpace)
         updateGradient()
     }
 
@@ -215,7 +234,8 @@ class LocationView: UIView, UITextFieldDelegate, ThemeApplicable {
 
     // MARK: - `urlTextField` Configuration
     private func configureURLTextField(_ state: LocationViewState) {
-        urlTextField.text = state.url
+        urlTextField.resignFirstResponder()
+        urlTextField.text = state.url?.absoluteString
         urlTextField.placeholder = state.urlTextFieldPlaceholder
         urlAbsolutePath = urlTextField.text
     }
@@ -286,13 +306,10 @@ class LocationView: UIView, UITextFieldDelegate, ThemeApplicable {
             animateURLText(textField, options: .transitionFlipFromBottom, textAlignment: .natural)
         }
 
-        let url = URL(string: textField.text ?? "")
-        let queryText = locationViewDelegate?.locationViewDisplayTextForURL(url)
-
         DispatchQueue.main.async { [self] in
             // `attributedText` property is set to nil to remove all formatting and truncation set before.
             textField.attributedText = nil
-            textField.text = (queryText != nil) ? queryText : urlAbsolutePath
+            textField.text = searchTerm != nil ? searchTerm : urlAbsolutePath
             textField.selectAll(nil)
         }
         locationViewDelegate?.locationViewDidBeginEditing(textField.text ?? "")

diff --git a/BrowserKit/Sources/ToolbarKit/LocationView/LocationViewDelegate.swift b/BrowserKit/Sources/ToolbarKit/LocationView/LocationViewDelegate.swift
@@ -11,19 +11,14 @@ protocol LocationViewDelegate: AnyObject {
     ///
     /// - Parameter text: The text that was entered.
     func locationViewDidEnterText(_ text: String)
+
     /// Called when the user begins editing text in the location view.
     ///
     /// - Parameter text: The initial text in the location view when the user began editing.
     func locationViewDidBeginEditing(_ text: String)
+
     /// Called when the location view should perform a search based on the entered text.
     ///
     /// - Parameter text: The text for which the location view should search.
     func locationViewShouldSearchFor(_ text: String)
-
-    /// Called to determine the display text for a given URL in the location view.
-    ///
-    /// - Parameter url: The URL for which to determine the display text.
-    /// - Returns: The display text as an optional String. If the URL is nil
-    ///  or cannot be converted to a display text, returns nil.
-    func locationViewDisplayTextForURL(_ url: URL?) -> String?
 }
diff --git a/BrowserKit/Sources/ToolbarKit/LocationView/LocationViewState.swift b/BrowserKit/Sources/ToolbarKit/LocationView/LocationViewState.swift
@@ -17,7 +17,8 @@ public struct LocationViewState {
 
     public let searchEngineImage: UIImage?
     public let lockIconImageName: String
-    public let url: String?
+    public let url: URL?
+    public let searchTerm: String?
     public var onTapLockIcon: (() -> Void)?
 
     public init(
@@ -30,7 +31,8 @@ public struct LocationViewState {
         urlTextFieldA11yLabel: String,
         searchEngineImage: UIImage?,
         lockIconImageName: String,
-        url: String?,
+        url: URL?,
+        searchTerm: String?,
         onTapLockIcon: (() -> Void)? = nil
     ) {
         self.searchEngineImageViewA11yId = searchEngineImageViewA11yId
@@ -43,6 +45,7 @@ public struct LocationViewState {
         self.searchEngineImage = searchEngineImage
         self.lockIconImageName = lockIconImageName
         self.url = url
+        self.searchTerm = searchTerm
         self.onTapLockIcon = onTapLockIcon
     }
 }
diff --git a/BrowserKit/Tests/ToolbarKitTests/ToolbarManagerTests.swift b/BrowserKit/Tests/ToolbarKitTests/ToolbarManagerTests.swift
@@ -69,6 +69,7 @@ final class ToolbarManagerTests: XCTestCase {
         let subject = createSubject()
         XCTAssertTrue(subject.shouldDisplayNavigationBorder(toolbarPosition: .top))
     }
+
     func testDisplayNavigationToolbarBorderWhenBottomPlacementThenShouldNotDisplay() {
         let subject = createSubject()
         XCTAssertFalse(subject.shouldDisplayNavigationBorder(toolbarPosition: .bottom))

diff --git a/SampleBrowser/SampleBrowser/Model/RootViewControllerModel.swift b/SampleBrowser/SampleBrowser/Model/RootViewControllerModel.swift
@@ -71,7 +71,7 @@ class RootViewControllerModel {
     }
 
     // MARK: - Address toolbar
-    func addressToolbarContainerModel(url: String?) -> AddressToolbarContainerModel {
+    func addressToolbarContainerModel(url: URL?) -> AddressToolbarContainerModel {
         let pageActions = [ToolbarElement(
             iconName: StandardImageIdentifiers.Large.qrCode,
             isEnabled: true,
@@ -98,7 +98,8 @@ class RootViewControllerModel {
             urlTextFieldA11yLabel: "Address Bar",
             searchEngineImage: UIImage(named: "bingSearchEngine"),
             lockIconImageName: StandardImageIdentifiers.Medium.lock,
-            url: url
+            url: url,
+            searchTerm: nil
         )
 
         // FXIOS-8947: Use scroll position

diff --git a/SampleBrowser/SampleBrowser/UI/RootViewController.swift b/SampleBrowser/SampleBrowser/UI/RootViewController.swift
@@ -112,7 +112,7 @@ class RootViewController: UIViewController,
         _ = addressToolbarContainer.becomeFirstResponder()
     }
 
-    private func updateAddressToolbar(url: String?) {
+    private func updateAddressToolbar(url: URL?) {
         let model = model.addressToolbarContainerModel(url: url)
         addressToolbarContainer.configure(model, toolbarDelegate: self)
     }
@@ -195,7 +195,7 @@ class RootViewController: UIViewController,
     }
 
     func onURLChange(url: String) {
-        updateAddressToolbar(url: url)
+        updateAddressToolbar(url: URL(string: url))
     }
 
     func onFindInPage(selected: String) {
@@ -239,7 +239,6 @@ class RootViewController: UIViewController,
     // MARK: - SearchViewDelegate
 
     func tapOnSuggestion(term: String) {
-        updateAddressToolbar(url: term)
         browse(to: term)
     }
 

diff --git a/firefox-ios/Client.xcodeproj/project.pbxproj b/firefox-ios/Client.xcodeproj/project.pbxproj
@@ -1463,6 +1463,7 @@
 		E1463D042982D0240074E16E /* NotificationManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1463D032982D0240074E16E /* NotificationManagerTests.swift */; };
 		E1463D0629830E4F0074E16E /* MockUserNotificationCenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1463D0529830E4F0074E16E /* MockUserNotificationCenter.swift */; };
 		E14BF33E2950B1230039758D /* MailProvidersTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E14BF33D2950B1230039758D /* MailProvidersTests.swift */; };
+		E14C78962C105488002AD3C7 /* AddressToolbarContainerModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E14C78952C105488002AD3C7 /* AddressToolbarContainerModelTests.swift */; };
 		E14F7DF2288F3F9F00E3722C /* ThemedTableSectionHeaderFooterView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E14F7DF1288F3F9F00E3722C /* ThemedTableSectionHeaderFooterView.swift */; };
 		E1516A3E2A7BC07E007819A4 /* ReliabilityGrade.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1516A3D2A7BC07E007819A4 /* ReliabilityGrade.swift */; };
 		E15DE7C0293A670700B32667 /* PhotonActionSheetSeparator.swift in Sources */ = {isa = PBXBuildFile; fileRef = E15DE7BF293A670700B32667 /* PhotonActionSheetSeparator.swift */; };
@@ -7627,6 +7628,7 @@
 		E1463D032982D0240074E16E /* NotificationManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationManagerTests.swift; sourceTree = "<group>"; };
 		E1463D0529830E4F0074E16E /* MockUserNotificationCenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockUserNotificationCenter.swift; sourceTree = "<group>"; };
 		E14BF33D2950B1230039758D /* MailProvidersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MailProvidersTests.swift; sourceTree = "<group>"; };
+		E14C78952C105488002AD3C7 /* AddressToolbarContainerModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddressToolbarContainerModelTests.swift; sourceTree = "<group>"; };
 		E14F7DF1288F3F9F00E3722C /* ThemedTableSectionHeaderFooterView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThemedTableSectionHeaderFooterView.swift; sourceTree = "<group>"; };
 		E1516A3D2A7BC07E007819A4 /* ReliabilityGrade.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReliabilityGrade.swift; sourceTree = "<group>"; };
 		E15C49D1BE35740E81701F21 /* dsb */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = dsb; path = dsb.lproj/Menu.strings; sourceTree = "<group>"; };
@@ -12049,6 +12051,7 @@
 				8A8482ED2BE15FEF00F9007B /* Microsurvey */,
 				8AC225632B6D3F9600CDA7FD /* Telemetry */,
 				8A171A6029F82AD90085770E /* Application */,
+				E14C78952C105488002AD3C7 /* AddressToolbarContainerModelTests.swift */,
 				CA7BD564248185B500A0A61B /* BreachAlertsTests.swift */,
 				BC003F5D2B59F44500929ECB /* BrowserViewControllerTests.swift */,
 				8A28C627291028870078A81A /* CanRemoveQuickActionBookmarkTests.swift */,
@@ -14853,6 +14856,7 @@
 				DFD1046D2B23539600938418 /* ProductAdsCacheTests.swift in Sources */,
 				E1390FB628B040E900C9EF3E /* WallpaperSelectorViewModelTests.swift in Sources */,
 				39AF317429DAE37E00F8E6F7 /* NimbusMessagingMessageTests.swift in Sources */,
+				E14C78962C105488002AD3C7 /* AddressToolbarContainerModelTests.swift in Sources */,
 				C889D7D52858CD8800121E1D /* HistoryHighlightsTestEntryProvider.swift in Sources */,
 				C2A72A6B2A77AC10002ACCE2 /* ReadingListCoordinatorTests.swift in Sources */,
 				E19B38B128A3E69300D8C541 /* WallpaperCollectionAvailabilityTests.swift in Sources */,

diff --git a/...ntend/Browser/BrowserViewController/Extensions/BrowserViewController+URLBarDelegate.swift b/...ntend/Browser/BrowserViewController/Extensions/BrowserViewController+URLBarDelegate.swift
@@ -340,40 +340,7 @@ extension BrowserViewController: URLBarDelegate {
     }
 
     func urlBar(_ urlBar: URLBarView, didSubmitText text: String) {
-        guard let currentTab = tabManager.selectedTab else { return }
-
-        if let fixupURL = URIFixup.getURL(text) {
-            // The user entered a URL, so use it.
-            finishEditingAndSubmit(fixupURL, visitType: VisitType.typed, forTab: currentTab)
-            return
-        }
-
-        // We couldn't build a URL, so check for a matching search keyword.
-        let trimmedText = text.trimmingCharacters(in: .whitespaces)
-        guard let possibleKeywordQuerySeparatorSpace = trimmedText.firstIndex(of: " ") else {
-            submitSearchText(text, forTab: currentTab)
-            return
-        }
-
-        let possibleKeyword = String(trimmedText[..<possibleKeywordQuerySeparatorSpace])
-        let possibleQuery = String(trimmedText[trimmedText.index(after: possibleKeywordQuerySeparatorSpace)...])
-
-        profile.places.getBookmarkURLForKeyword(keyword: possibleKeyword).uponQueue(.main) { result in
-            if var urlString = result.successValue ?? "",
-               let escapedQuery = possibleQuery.addingPercentEncoding(
-                withAllowedCharacters: NSCharacterSet.urlQueryAllowed
-               ),
-               let range = urlString.range(of: "%s") {
-                urlString.replaceSubrange(range, with: escapedQuery)
-
-                if let url = URL(string: urlString, invalidCharacters: false) {
-                    self.finishEditingAndSubmit(url, visitType: VisitType.typed, forTab: currentTab)
-                    return
-                }
-            }
-
-            self.submitSearchText(text, forTab: currentTab)
-        }
+        didSubmitSearchText(text)
     }
 
     func submitSearchText(_ text: String, forTab tab: Tab) {

diff --git a/...-ios/Client/Frontend/Browser/BrowserViewController/State/BrowserViewControllerState.swift b/...-ios/Client/Frontend/Browser/BrowserViewController/State/BrowserViewControllerState.swift
@@ -267,6 +267,7 @@ struct BrowserViewControllerState: ScreenState, Equatable {
         switch action.actionType {
         case ToolbarActionType.didLoadToolbars,
             ToolbarActionType.numberOfTabsChanged,
+            ToolbarActionType.urlDidChange,
             ToolbarActionType.backButtonStateChanged,
             ToolbarActionType.forwardButtonStateChanged:
             return BrowserViewControllerState(