Fix #1391: Added Setting to Enable Bookmark Button on Top Toolbar (#1456

)

BraveShared/BraveStrings.swift

@@ -395,6 +395,7 @@ extension Strings {
     public static let Privacy = NSLocalizedString("Privacy", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Privacy", comment: "Settings privacy section title")
     public static let Security = NSLocalizedString("Security", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Security", comment: "Settings security section title")
     public static let Save_Logins = NSLocalizedString("SaveLogins", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Save Logins", comment: "Setting to enable the built-in password manager")
+    public static let Show_Bookmark_Button_In_Top_Toolbar = NSLocalizedString("ShowBookmarkButtonInTopToolbar", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Show Bookmarks Shortcut", comment: "Setting to show a bookmark button on the top level menu that triggers a panel of the user's bookmarks.")
     public static let AlwaysRequestDesktopSite = NSLocalizedString("AlwaysRequestDesktopSite", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Always Request Desktop Site", comment: "Setting to always request the desktop version of a website.")
     public static let ShieldsAdStats = NSLocalizedString("AdsrBlocked", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Ads \nBlocked", comment: "Shields Ads Stat")
     public static let ShieldsAdAndTrackerStats = NSLocalizedString("AdsAndTrackersrBlocked", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Ads & Trackers \nBlocked", comment: "Shields Ads Stat")

Client/Application/ClientPreferences.swift

@@ -41,6 +41,8 @@ extension Preferences {
         static let blockPopups = Option<Bool>(key: "general.block-popups", default: true)
         /// Controls how the tab bar should be shown (or not shown)
         static let tabBarVisibility = Option<Int>(key: "general.tab-bar-visiblity", default: TabBarVisibility.always.rawValue)
+        /// Specifies whether the bookmark button is present on toolbar
+        static let showBookmarkToolbarShortcut = Option<Bool>(key: "general.show-bookmark-toolbar-shortcut", default: UIDevice.isIpad)
         /// Sets Desktop UA for iPad by default (iOS 13+ & iPad only)
         static let alwaysRequestDesktopSite = Option<Bool>(key: "general.always-request-desktop-site", default: UIDevice.isIpad)
 

Client/Frontend/Browser/BrowserViewController.swift

@@ -1619,6 +1619,21 @@ extension BrowserViewController: TopToolbarDelegate {
         popover.present(from: topToolbar.locationView.shieldsButton, on: self)
     }
 
+    // TODO: This logic should be fully abstracted away and share logic from current MenuViewController
+    // See: https://github.com/brave/brave-ios/issues/1452
+    func topToolbarDidTapBookmarkButton(_ topToolbar: TopToolbarView) {
+        let vc = BookmarksViewController(folder: nil, isPrivateBrowsing: PrivateBrowsingManager.shared.isPrivateBrowsing)
+        vc.toolbarUrlActionsDelegate = self
+
+        let nav = SettingsNavigationController(rootViewController: vc)
+        nav.modalPresentationStyle = .formSheet
+
+        let button = UIBarButtonItem(barButtonSystemItem: .done, target: nav, action: #selector(SettingsNavigationController.done))
+        nav.navigationBar.topItem?.rightBarButtonItem = button
+
+        present(nav, animated: true)
+    }
+
     func topToolbarDidTapBraveRewardsButton(_ topToolbar: TopToolbarView) {
         showBraveRewardsPanel()
     }

Client/Frontend/Browser/Toolbars/UrlBar/TopToolbarView.swift

@@ -37,6 +37,7 @@ protocol TopToolbarDelegate: class {
     // Returns either (search query, true) or (url, false).
     func topToolbarDisplayTextForURL(_ url: URL?) -> (String?, Bool)
     func topToolbarDidBeginDragInteraction(_ topToolbar: TopToolbarView)
+    func topToolbarDidTapBookmarkButton(_ topToolbar: TopToolbarView)
     func topToolbarDidTapBraveShieldsButton(_ topToolbar: TopToolbarView)
     func topToolbarDidTapBraveRewardsButton(_ topToolbar: TopToolbarView)
     func topToolbarDidTapMenuButton(_ topToolbar: TopToolbarView)
@@ -139,15 +140,17 @@ class TopToolbarView: UIView, ToolbarProtocol {
         return button
     }()
 
-    var bookmarkButton = ToolbarButton()
+    lazy var bookmarkButton = ToolbarButton().then {
+        $0.setImage(#imageLiteral(resourceName: "menu_bookmarks").template, for: .normal)
+        $0.accessibilityLabel = Strings.BookmarksMenuItem
+        $0.addTarget(self, action: #selector(didClickBookmarkButton), for: .touchUpInside)
+    }
+
     var forwardButton = ToolbarButton()
     var shareButton = ToolbarButton()
     var addTabButton = ToolbarButton()
     lazy var menuButton = ToolbarButton().then {
         $0.contentMode = .center
-        $0.setImage(#imageLiteral(resourceName: "nav-menu").template, for: .normal)
-        $0.accessibilityLabel = Strings.AppMenuButtonAccessibilityLabel
-        $0.addTarget(self, action: #selector(didClickMenu), for: .touchUpInside)
         $0.accessibilityIdentifier = "topToolbarView-menuButton"
     }
 
@@ -158,7 +161,7 @@ class TopToolbarView: UIView, ToolbarProtocol {
     }()
 
     lazy var actionButtons: [Themeable & UIButton] =
-        [self.shareButton, self.tabsButton,
+        [self.shareButton, self.tabsButton, self.bookmarkButton,
          self.forwardButton, self.backButton, self.menuButton].compactMap { $0 }
 
     /// Update the shields icon based on whether or not shields are enabled for this site
@@ -206,6 +209,8 @@ class TopToolbarView: UIView, ToolbarProtocol {
         helper = ToolbarHelper(toolbar: self)
         setupConstraints()
 
+        Preferences.General.showBookmarkToolbarShortcut.observe(from: self)
+
         // Make sure we hide any views that shouldn't be showing in non-overlay mode.
         updateViewsForOverlayModeAndToolbarChanges()
     }
@@ -227,11 +232,11 @@ class TopToolbarView: UIView, ToolbarProtocol {
         navigationStackView.addArrangedSubview(backButton)
         navigationStackView.addArrangedSubview(forwardButton)
 
-        [backButton, forwardButton, menuButton, tabsButton].forEach {
+        [backButton, forwardButton, bookmarkButton, tabsButton, menuButton].forEach {
             $0.contentEdgeInsets = UIEdgeInsets(top: 4, left: 8, bottom: 4, right: 8)
         }
 
-        [navigationStackView, locationContainer, tabsButton, menuButton, cancelButton].forEach {
+        [navigationStackView, bookmarkButton, locationContainer, tabsButton, menuButton, cancelButton].forEach {
             mainStackView.addArrangedSubview($0)
         }
 
@@ -421,6 +426,9 @@ class TopToolbarView: UIView, ToolbarProtocol {
         menuButton.isHidden = !toolbarIsShowing || inOverlayMode
         tabsButton.isHidden = !toolbarIsShowing || inOverlayMode
         locationView.contentView.isHidden = inOverlayMode
+
+        let showBookmarkPref = Preferences.General.showBookmarkToolbarShortcut.value
+        bookmarkButton.isHidden = showBookmarkPref ? inOverlayMode : true
     }
 
     private func animateToOverlayState(overlayMode overlay: Bool, didCancel cancel: Bool = false) {
@@ -431,7 +439,7 @@ class TopToolbarView: UIView, ToolbarProtocol {
         }
 
         if inOverlayMode {
-            [progressBar, navigationStackView, menuButton, tabsButton, locationView.contentView].forEach {
+            [progressBar, navigationStackView, bookmarkButton, menuButton, tabsButton, locationView.contentView].forEach {
                 $0?.isHidden = true
             }
 
@@ -458,6 +466,10 @@ class TopToolbarView: UIView, ToolbarProtocol {
         delegate?.topToolbarDidPressScrollToTop(self)
     }
 
+    @objc func didClickBookmarkButton() {
+        delegate?.topToolbarDidTapBookmarkButton(self)
+    }
+
     @objc func didClickMenu() {
         delegate?.topToolbarDidTapMenuButton(self)
     }
@@ -467,6 +479,14 @@ class TopToolbarView: UIView, ToolbarProtocol {
     }
 }
 
+// MARK: - PreferencesObserver
+
+extension TopToolbarView: PreferencesObserver {
+    func preferencesDidChange(for key: String) {
+        updateViewsForOverlayModeAndToolbarChanges()
+    }
+}
+
 // MARK: - TabLocationViewDelegate
 
 extension TopToolbarView: TabLocationViewDelegate {

Client/Frontend/Settings/SettingsViewController.swift

@@ -167,6 +167,10 @@ class SettingsViewController: TableViewController {
             general.rows.append(row)
         }
 
+        general.rows.append(
+            BoolRow(title: Strings.Show_Bookmark_Button_In_Top_Toolbar, option: Preferences.General.showBookmarkToolbarShortcut)
+        )
+
         if #available(iOS 13.0, *), UIDevice.isIpad {
             general.rows.append(BoolRow(title: Strings.AlwaysRequestDesktopSite,
             option: Preferences.General.alwaysRequestDesktopSite))