review

Client/Extensions/UIViewExtensions.swift

@@ -103,9 +103,11 @@ extension UIView {
     /// Returns a line with height of 1pt. Used to imitate a separator line in custom views.
     static var separatorLine: UIView {
         let view = UIView().then {
-        $0.backgroundColor = UIColor(white: 0.0, alpha: 0.2)
-        $0.translatesAutoresizingMaskIntoConstraints = false
-        $0.addConstraint(NSLayoutConstraint(item: $0, attribute: .height, relatedBy: .equal, toItem: nil, attribute: .notAnAttribute, multiplier: 1, constant: 0.5))
+            $0.backgroundColor = UIColor(white: 0.0, alpha: 0.2)
+            $0.translatesAutoresizingMaskIntoConstraints = false
+            $0.addConstraint(NSLayoutConstraint(item: $0, attribute: .height, relatedBy: .equal,
+                                                toItem: nil, attribute: .notAnAttribute, multiplier: 1,
+                                                constant: 0.5))
         }
 
         return view

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift

@@ -35,10 +35,11 @@ class AddEditBookmarkTableViewController: UITableViewController {
     // MARK: - View setup
     
     private lazy var saveButton: UIBarButtonItem = {
-        let button = UIBarButtonItem()
-        button.target = self
-        button.action = #selector(save)
-        button.title = Strings.SaveButtonTitle
+        let button = UIBarButtonItem().then {
+            $0.target = self
+            $0.action = #selector(save)
+            $0.title = Strings.SaveButtonTitle
+        }
 
         return button
     }()

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditHeaderView.swift

@@ -25,6 +25,10 @@ class AddEditHeaderView: UIView {
 
         backgroundColor = .white
         addSubview(mainStackView)
+
+        mainStackView.snp.makeConstraints {
+            $0.edges.equalTo(self)
+        }
     }
 
     @available(*, unavailable)

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarkDetailsView.swift

@@ -20,7 +20,7 @@ class BookmarkDetailsView: AddEditHeaderView, BookmarkFormFieldsProtocol {
 
     let urlTextField: UITextField? = UITextField().then {
         $0.placeholder = Strings.BookmarkUrlPlaceholderText
-        $0.keyboardType = .webSearch
+        $0.keyboardType = .URL
         $0.autocorrectionType = .no
         $0.autocapitalizationType = .none
         $0.smartDashesType = .no
@@ -56,21 +56,18 @@ class BookmarkDetailsView: AddEditHeaderView, BookmarkFormFieldsProtocol {
 
         guard let urlTextField = urlTextField else { fatalError("Url text field must be set up") }
 
-        [UIView.separatorLine, contentStackView, UIView.separatorLine].forEach {
-            mainStackView.addArrangedSubview($0)
-        }
+        [UIView.separatorLine, contentStackView, UIView.separatorLine]
+            .forEach(mainStackView.addArrangedSubview)
 
-        [titleTextField, UIView.separatorLine, urlTextField].forEach {
-            textFieldsStackView.addArrangedSubview($0)
-        }
+        [titleTextField, UIView.separatorLine, urlTextField]
+            .forEach(textFieldsStackView.addArrangedSubview)
 
         // Adding spacer view with zero width, UIStackView's spacing will take care
         // about adding a left margin to the content stack view.
         let emptySpacer = UIView.spacer(.horizontal, amount: 0)
 
-        [emptySpacer, faviconImageView, textFieldsStackView].forEach {
-            contentStackView.addArrangedSubview($0)
-        }
+        [emptySpacer, faviconImageView, textFieldsStackView]
+            .forEach(contentStackView.addArrangedSubview)
 
         if let url = url, let favUrl = URL(string: url) {
             faviconImageView.setIcon(nil, forURL: favUrl)
@@ -79,10 +76,6 @@ class BookmarkDetailsView: AddEditHeaderView, BookmarkFormFieldsProtocol {
         titleTextField.text = title ?? Strings.NewBookmarkDefaultName
         urlTextField.text = url ?? Strings.NewFolderDefaultName
 
-        mainStackView.snp.makeConstraints {
-            $0.edges.equalTo(self)
-        }
-
         setupTextFieldTargets()
     }
 

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarkFormFieldsProtocol.swift

@@ -5,9 +5,8 @@
 import Foundation
 import Shared
 
-protocol BookmarkFormFieldsProtocol where Self: UIView {
+protocol BookmarkFormFieldsProtocol: UIView {
     var titleTextField: UITextField { get }
-    /// Nil by default
     var urlTextField: UITextField? { get }
 
     var delegate: BookmarkDetailsViewDelegate? { get set }
@@ -19,18 +18,20 @@ extension BookmarkFormFieldsProtocol {
     var urlTextField: UITextField? { return nil }
 
     func validateFields() -> Bool {
+        let title = titleTextField.text
+
         // Only title field is implemented
         if urlTextField == nil {
-            guard let titleText = titleTextField.text else { return false }
-            return validateTitle(titleText)
+            return validateTitle(title)
         }
 
-        guard let title = titleTextField.text, let url = urlTextField?.text else { return false }
+        guard let url = urlTextField?.text else { return false }
 
         return validateTitle(title) && validateUrl(url)
     }
 
-    private func validateTitle(_ title: String) -> Bool {
+    private func validateTitle(_ title: String?) -> Bool {
+        guard let title = title else { return false }
         return !title.isEmpty
     }
 

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/FolderDetailsView.swift

@@ -24,13 +24,12 @@ class FolderDetailsViewTableViewCell: AddEditHeaderView, BookmarkFormFieldsProto
     convenience init(title: String?, viewHeight: CGFloat) {
         self.init(frame: .zero)
 
-        [UIView.separatorLine, titleTextField, UIView.separatorLine].forEach {
-            mainStackView.addArrangedSubview($0)
-        }
+        [UIView.separatorLine, titleTextField, UIView.separatorLine]
+            .forEach(mainStackView.addArrangedSubview)
 
         titleTextField.text = title ?? Strings.NewFolderDefaultName
 
-        mainStackView.snp.makeConstraints {
+        mainStackView.snp.remakeConstraints {
             $0.edges.equalTo(self)
             $0.height.equalTo(viewHeight)
         }

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/IndentedImageTableViewCell.swift

@@ -6,12 +6,12 @@ import UIKit
 
 class IndentedImageTableViewCell: UITableViewCell {
 
-    let mainStackView = UIStackView().then {
+    private let mainStackView = UIStackView().then {
         $0.spacing = 8
         $0.alignment = .fill
     }
 
-    let folderNameStackView = UIStackView().then {
+    private let folderNameStackView = UIStackView().then {
         $0.axis = .vertical
         $0.distribution = .equalSpacing
     }
@@ -27,11 +27,12 @@ class IndentedImageTableViewCell: UITableViewCell {
         $0.textAlignment = .left
     }
 
-    var spacerLine: UIView {
-        let view = UIView()
-        view.backgroundColor = .lightGray
-        view.translatesAutoresizingMaskIntoConstraints = false
-        view.addConstraint(NSLayoutConstraint(item: view, attribute: .height, relatedBy: .equal, toItem: nil, attribute: .notAnAttribute, multiplier: 1, constant: 0.5))
+    private var spacerLine: UIView {
+        let view = UIView().then {
+        $0.backgroundColor = .lightGray
+        $0.translatesAutoresizingMaskIntoConstraints = false
+        $0.addConstraint(NSLayoutConstraint(item: $0, attribute: .height, relatedBy: .equal, toItem: nil, attribute: .notAnAttribute, multiplier: 1, constant: 0.5))
+        }
 
         return view
     }
@@ -50,9 +51,8 @@ class IndentedImageTableViewCell: UITableViewCell {
 
         let transparentLine = spacerLine
         transparentLine.backgroundColor = .clear
-        folderNameStackView.addArrangedSubview(transparentLine)
-        folderNameStackView.addArrangedSubview(folderName)
-        folderNameStackView.addArrangedSubview(spacerLine)
+
+        [transparentLine, folderName, spacerLine].forEach(folderNameStackView.addArrangedSubview)
 
         mainStackView.addArrangedSubview(folderNameStackView)
 

Client/Frontend/Browser/Toolbars/BottomToolbar/Menu/MenuViewController.swift

@@ -14,7 +14,7 @@ class MenuViewController: UITableViewController {
         static let topBottomInset: CGFloat = 5
     }
 
-    private enum MenuButtons: Int {
+    private enum MenuButtons: Int, CaseIterable {
         case bookmarks, history, settings, add, share
 
         var title: String {
@@ -36,9 +36,6 @@ class MenuViewController: UITableViewController {
             case .share: return #imageLiteral(resourceName: "nav-share")
             }
         }
-
-        // TODO: Remove when we can use Swift 4.2/`CaseIterable`
-        static let allCases: [MenuButtons] = [.bookmarks, .history, .settings, .add, .share]
     }
 
     private let bvc: BrowserViewController
@@ -104,15 +101,17 @@ class MenuViewController: UITableViewController {
 
         guard let cell = tableView.cellForRow(at: indexPath) else { return }
 
-        switch cell.tag {
-        case MenuButtons.bookmarks.rawValue: openBookmarks()
-        case MenuButtons.history.rawValue: openHistory()
-        case MenuButtons.settings.rawValue: openSettings()
-        case MenuButtons.add.rawValue: openAddBookmark()
-        case MenuButtons.share.rawValue: openShareSheet()
-
-        default:
+        guard let button = MenuButtons(rawValue: cell.tag) else {
             assertionFailure("No cell with \(cell.tag) tag.")
+            return
+        }
+
+        switch button {
+        case .bookmarks: openBookmarks()
+        case .history: openHistory()
+        case .settings: openSettings()
+        case .add: openAddBookmark()
+        case .share: openShareSheet()
         }
     }
 
@@ -148,7 +147,7 @@ class MenuViewController: UITableViewController {
         case .right: nav.navigationBar.topItem?.rightBarButtonItem = button
         }
 
-        dismiss(animated: true)
+        dismissView()
         bvc.present(nav, animated: true)
     }
 
@@ -182,7 +181,7 @@ class MenuViewController: UITableViewController {
     }
 
     private func openShareSheet() {
-        dismiss(animated: true)
+        dismissView()
         bvc.tabToolbarDidPressShare()
     }
 

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

@@ -8,7 +8,7 @@ import BraveShared
 import Data
 
 private struct URLBarViewUX {
-    static let LocationLeftPadding: CGFloat = 8
+    static let LocationPadding: CGFloat = 8
     static let Padding: CGFloat = 10
     static let LocationHeight: CGFloat = 34
     static let ButtonHeight: CGFloat = 44
@@ -279,8 +279,8 @@ class URLBarView: UIView {
         locationTextField.attributedPlaceholder = self.locationView.placeholder
         locationContainer.addSubview(locationTextField)
         locationTextField.snp.remakeConstraints { make in
-            let insets = UIEdgeInsets(top: 0, left: URLBarViewUX.LocationLeftPadding,
-                                      bottom: 0, right: URLBarViewUX.LocationLeftPadding)
+            let insets = UIEdgeInsets(top: 0, left: URLBarViewUX.LocationPadding,
+                                      bottom: 0, right: URLBarViewUX.LocationPadding)
             make.edges.equalTo(self.locationView).inset(insets)
         }
 

Data/models/Bookmark.swift

@@ -124,8 +124,6 @@ public final class Bookmark: NSManagedObject, WebsitePresentable, Syncable, CRUD
 
         fetchRequest.predicate = predicate
 
-        //"parentFolder == %@"
-        
         return NSFetchedResultsController(fetchRequest: fetchRequest, managedObjectContext: context,
                                           sectionNameKeyPath: nil, cacheName: nil)
     }
@@ -146,9 +144,7 @@ public final class Bookmark: NSManagedObject, WebsitePresentable, Syncable, CRUD
     // MARK: Update
     
     public func update(customTitle: String?, url: String?) {
-        // Title can't be empty, except when coming from Sync
-        let isTitleEmpty = customTitle?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == true
-        if customTitle == nil || isTitleEmpty { return }
+        if !containsCustomTitle { return }
         updateInternal(customTitle: customTitle, url: url)
     }
 
@@ -158,11 +154,16 @@ public final class Bookmark: NSManagedObject, WebsitePresentable, Syncable, CRUD
     }
 
     public func updateWithNewLocation(customTitle: String?, url: String?, location: Bookmark?) {
-        let isTitleEmpty = customTitle?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == true
-        if customTitle == nil || isTitleEmpty { return }
+        if !containsCustomTitle { return }
+
         updateInternal(customTitle: customTitle, url: url, location: .new(location: location))
     }
 
+    // Title can't be empty, except when coming from Sync
+    private var containsCustomTitle: Bool {
+        return customTitle?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false
+    }
+
     public class func migrateBookmarkOrders() {
         DataController.perform { context in
             migrateOrder(forFavorites: true, context: context)