Popovers positioning (#1574)

Task/Issue URL:
https://app.asana.com/0/1177771139624306/1202217488822824/f
Figma:
https://www.figma.com/file/pJbEb2nC4jwXlToaV0xwGD/Time-boxed-UI-polish-round-(2022-09)?type=design&node-id=48-15266&mode=design&viewport=-2903%2C2125%2C1.15&t=VC4yzgdLvv3RaUhD-0

Author: mallexxx

DuckDuckGo/Bookmarks/View/BookmarkPopover.swift

@@ -22,10 +22,24 @@ final class BookmarkPopover: NSPopover {
 
     var isNew = false
 
+    private weak var addressBar: NSView?
+
+    /// prefferred bounding box for the popover positioning
+    override var boundingFrame: NSRect {
+        guard let addressBar,
+              let window = addressBar.window else { return .infinite }
+        var frame = window.convertToScreen(addressBar.convert(addressBar.bounds, to: nil))
+
+        frame = frame.insetBy(dx: -36, dy: -window.frame.size.height)
+
+        return frame
+    }
+
     override init() {
         super.init()
 
-        behavior = .transient
+        self.animates = false
+        self.behavior = .transient
         setupContentController()
     }
 
@@ -46,6 +60,11 @@ final class BookmarkPopover: NSPopover {
     }
     // swiftlint:enable force_cast
 
+    override func show(relativeTo positioningRect: NSRect, of positioningView: NSView, preferredEdge: NSRectEdge) {
+        self.addressBar = positioningView.superview
+        super.show(relativeTo: positioningRect, of: positioningView, preferredEdge: preferredEdge)
+    }
+
 }
 
 extension BookmarkPopover: BookmarkPopoverViewControllerDelegate {

DuckDuckGo/Common/Extensions/NSPopoverExtension.swift

@@ -20,22 +20,77 @@ import AppKit
 
 extension NSPopover {
 
-    /// Shows the popover below the specified button with the popover's pin positioned in the middle of the button, and a specified y-offset for the pin.
-    ///
-    /// - Parameters:
-    ///   - view: The button below which the popover should appear.
-    ///   - yOffset: The y-offset for the popover's pin position relative to the bottom of the button. Default is 5.0 points.
-    func showBelow(_ view: NSView) {
-        // Set the preferred edge to be the bottom edge of the button
-        let preferredEdge: NSRectEdge = .maxY
-
-        // Calculate the positioning rect
-        let viewFrame = view.bounds
-        let pinPositionX = viewFrame.midX
-        let positioningRect = NSRect(x: pinPositionX, y: 0, width: 0, height: 0)
-
-        // Show the popover
-        self.show(relativeTo: positioningRect, of: view, preferredEdge: preferredEdge)
+    static let defaultMainWindowMargin = 6.0
+
+    /// maximum margin from window edge
+    @objc var mainWindowMargin: CGFloat {
+        Self.defaultMainWindowMargin
+    }
+
+    /// temporary value used to get popover‘s owner window while it‘s not yet set
+    @TaskLocal
+    private static var mainWindow: NSWindow?
+
+    var mainWindow: NSWindow? {
+        self.contentViewController?.view.window?.parent ?? Self.mainWindow
+    }
+
+    /// prefferred bounding box for the popover positioning
+    @objc var boundingFrame: NSRect {
+        guard let mainWindow else { return .infinite }
+
+        return mainWindow.frame.insetBy(dx: mainWindowMargin, dy: 0)
+            .intersection(mainWindow.screen?.visibleFrame ?? .infinite)
+    }
+
+    @objc func adjustFrame(_ frame: NSRect) -> NSRect {
+        var frame = frame
+        let boundingFrame = self.boundingFrame
+        if !boundingFrame.isInfinite, boundingFrame.width > frame.width {
+            frame.origin.x = min(max(frame.minX, boundingFrame.minX), boundingFrame.maxX - frame.width)
+        }
+        return frame
+    }
+
+    /// Shows the popover below the specified rect inside the view bounds with the popover's pin positioned in the middle of the rect
+    public func show(positionedBelow positioningRect: NSRect, in positioningView: NSView) {
+        assert(!positioningView.isHidden && positioningView.alphaValue > 0)
+
+        // We tap into `_currentFrameOnScreenWithContentSize:outAnchorEdge:` to adjust popover position
+        // inside bounds of its owner Main Window.
+        // https://app.asana.com/0/1177771139624306/1202217488822824/f
+        _=Self.swizzleCurrentFrameOnScreenOnce
+
+        // position popover at the middle of the positioningView
+        let positioningRect = NSRect(x: positioningRect.midX - 1, y: positioningRect.origin.y, width: 2, height: positioningRect.height)
+        let preferredEdge: NSRectEdge = positioningView.isFlipped ? .maxY : .minY
+
+        Self.$mainWindow.withValue(positioningView.window) {
+            self.show(relativeTo: positioningRect, of: positioningView, preferredEdge: preferredEdge)
+        }
+    }
+
+    /// Shows the popover below the specified view with the popover's pin positioned in the middle of the view
+    func show(positionedBelow view: NSView) {
+        self.show(positionedBelow: view.bounds, in: view)
+    }
+
+    static let currentFrameOnScreenWithContentSizeSelector = NSSelectorFromString("_currentFrameOnScreenWithContentSize:outAnchorEdge:")
+
+    private static let swizzleCurrentFrameOnScreenOnce: () = {
+        guard let originalMethod = class_getInstanceMethod(NSPopover.self, currentFrameOnScreenWithContentSizeSelector),
+              let swizzledMethod = class_getInstanceMethod(NSPopover.self, #selector(currentFrameOnScreenWithContentSize)) else {
+            assertionFailure("Methods not available")
+            return
+        }
+
+        method_exchangeImplementations(originalMethod, swizzledMethod)
+    }()
+
+    // place popover inside bounds of its owner Main Window
+    @objc(swizzled_currentFrameOnScreenWithContentSize:outAnchorEdge:)
+    private dynamic func currentFrameOnScreenWithContentSize(size: NSSize, outAnchorEdge: UnsafeRawPointer?) -> NSRect {
+        self.adjustFrame(currentFrameOnScreenWithContentSize(size: size, outAnchorEdge: outAnchorEdge))
     }
 
 }

DuckDuckGo/Common/Extensions/NSRectExtension.swift

@@ -37,8 +37,10 @@ extension NSRect {
     }
 
     // Apply an offset so that we don't get caught by the "Line of Death" https://textslashplain.com/2017/01/14/the-line-of-death/
-    func insetFromLineOfDeath() -> NSRect {
-        return insetBy(dx: 0, dy: -15)
+    func insetFromLineOfDeath(flipped: Bool) -> NSRect {
+        let offset = 3.0
+        assert(height > offset * 2)
+        return insetBy(dx: 0, dy: offset)
     }
 
 }

DuckDuckGo/FileDownload/View/DownloadsPopover.swift

@@ -23,6 +23,7 @@ final class DownloadsPopover: NSPopover {
     override init() {
         super.init()
 
+        self.animates = false
         self.behavior = .semitransient
 
         setupContentController()

DuckDuckGo/Fire/View/FireCoordinator.swift

@@ -56,10 +56,12 @@ final class FireCoordinator {
     }
 
     static func showFirePopover(relativeTo positioningView: NSView, tabCollectionViewModel: TabCollectionViewModel) {
-        if !(firePopover?.isShown ?? false) {
-            firePopover = FirePopover(fireViewModel: fireViewModel, tabCollectionViewModel: tabCollectionViewModel)
-            firePopover?.showBelow(positioningView)
+        guard !(firePopover?.isShown ?? false) else {
+            firePopover?.close()
+            return
         }
+        firePopover = FirePopover(fireViewModel: fireViewModel, tabCollectionViewModel: tabCollectionViewModel)
+        firePopover?.show(positionedBelow: positioningView.bounds.insetBy(dx: 0, dy: 3), in: positioningView)
     }
 
 }

DuckDuckGo/Fire/View/FirePopover.swift

@@ -20,10 +20,36 @@ import AppKit
 
 final class FirePopover: NSPopover {
 
+    override var mainWindowMargin: CGFloat { -14 }
+
+    private static let minScreenEdgeMargin = 10.0
+    private static let defaultScreenEdgeCorrection = 12.0
+
+    // always position the Fire popover by the right edge
+    override func adjustFrame(_ frame: NSRect) -> NSRect {
+        let boundingFrame = self.boundingFrame
+        guard !boundingFrame.isInfinite else { return frame }
+        guard let popoverWindow = self.contentViewController?.view.window else {
+            assertionFailure("no popover window")
+            return frame
+        }
+
+        var frame = frame
+        frame.origin.x = boundingFrame.maxX - popoverWindow.frame.width
+        if let mainWindow = popoverWindow.parent,
+           let screen = mainWindow.screen,
+           mainWindow.frame.maxX > screen.visibleFrame.maxX - Self.defaultScreenEdgeCorrection {
+            // close to the screen edge the Popover gets shifted to the left
+            frame.origin.x += Self.defaultScreenEdgeCorrection - (screen.visibleFrame.maxX - mainWindow.frame.maxX)
+        }
+        return frame
+    }
+
     init(fireViewModel: FireViewModel, tabCollectionViewModel: TabCollectionViewModel) {
         super.init()
 
-        self.behavior = .transient
+        self.animates = false
+        self.behavior = .semitransient
 
         setupContentController(fireViewModel: fireViewModel, tabCollectionViewModel: tabCollectionViewModel)
     }
@@ -38,10 +64,9 @@ final class FirePopover: NSPopover {
 
     private func setupContentController(fireViewModel: FireViewModel, tabCollectionViewModel: TabCollectionViewModel) {
         let storyboard = NSStoryboard(name: "Fire", bundle: nil)
-        let controller = storyboard.instantiateController(
-            identifier: "FirePopoverWrapperViewController") { coder -> FirePopoverWrapperViewController? in
-                return FirePopoverWrapperViewController(coder: coder, fireViewModel: fireViewModel, tabCollectionViewModel: tabCollectionViewModel)
-            }
+        let controller = storyboard.instantiateController(identifier: "FirePopoverWrapperViewController") { coder -> FirePopoverWrapperViewController? in
+            return FirePopoverWrapperViewController(coder: coder, fireViewModel: fireViewModel, tabCollectionViewModel: tabCollectionViewModel)
+        }
         contentViewController = controller
     }
 

DuckDuckGo/Menus/MainMenuActions.swift

@@ -240,7 +240,7 @@ extension AppDelegate {
             assertionFailure("No reference to main window controller")
             return
         }
-    windowController.mainViewController.browserTabViewController.openNewTab(with: .url(URL.duckDuckGoEmailLogin))
+        windowController.mainViewController.browserTabViewController.openNewTab(with: .url(URL.duckDuckGoEmailLogin))
     }
 
 }

DuckDuckGo/NavigationBar/View/AddressBarButtonsViewController.swift

@@ -292,7 +292,7 @@ final class AddressBarButtonsViewController: NSViewController {
             bookmarkButton.isHidden = false
             bookmarkPopover.isNew = result.isNew
             bookmarkPopover.viewController.bookmark = bookmark
-            bookmarkPopover.show(relativeTo: bookmarkButton.bounds, of: bookmarkButton, preferredEdge: .maxY)
+            bookmarkPopover.show(positionedBelow: bookmarkButton)
         } else {
             updateBookmarkButtonVisibility()
             bookmarkPopover.close()
@@ -356,7 +356,7 @@ final class AddressBarButtonsViewController: NSViewController {
 
         let positioningViewInWindow = privacyDashboardPositioningView.convert(privacyDashboardPositioningView.bounds, to: view.window?.contentView)
         privacyDashboardPopover.setPreferredMaxHeight(positioningViewInWindow.origin.y)
-        privacyDashboardPopover.show(relativeTo: privacyDashboardPositioningView.bounds, of: privacyDashboardPositioningView, preferredEdge: .maxY)
+        privacyDashboardPopover.show(positionedBelow: privacyDashboardPositioningView)
 
         privacyEntryPointButton.state = .on
 

DuckDuckGo/NavigationBar/View/NavigationBarPopovers.swift

@@ -97,8 +97,7 @@ final class NavigationBarPopovers {
     @available(macOS 11.4, *)
     func toggleNetworkProtectionPopover(usingView view: NSView, withDelegate delegate: NSPopoverDelegate) {
 #if NETWORK_PROTECTION
-        if let networkProtectionPopover = networkProtectionPopover,
-           networkProtectionPopover.isShown {
+        if let networkProtectionPopover, networkProtectionPopover.isShown {
             networkProtectionPopover.close()
         } else {
             showNetworkProtectionPopover(usingView: view, withDelegate: delegate)
@@ -121,7 +120,7 @@ final class NavigationBarPopovers {
         popover.viewController.delegate = downloadsDelegate
         downloadsPopover = popover
 
-        show(popover: popover, usingView: view, preferredEdge: .maxY)
+        show(popover, positionedBelow: view)
     }
 
     private var downloadsPopoverTimer: Timer?
@@ -161,6 +160,12 @@ final class NavigationBarPopovers {
             downloadsPopover?.close()
         }
 
+#if NETWORK_PROTECTION
+        if networkProtectionPopover?.isShown ?? false {
+            networkProtectionPopover?.close()
+        }
+#endif
+
         return true
     }
 
@@ -176,7 +181,7 @@ final class NavigationBarPopovers {
         }
 
         LocalBookmarkManager.shared.requestSync()
-        show(popover: popover, usingView: view)
+        show(popover, positionedBelow: view)
     }
 
     func showPasswordManagementPopover(selectedCategory: SecureVaultSorting.Category?, usingView view: NSView, withDelegate delegate: NSPopoverDelegate) {
@@ -186,15 +191,15 @@ final class NavigationBarPopovers {
         passwordManagementPopover = popover
         popover.viewController.domain = passwordManagementDomain
         popover.delegate = delegate
-        show(popover: popover, usingView: view)
+        show(popover, positionedBelow: view)
         popover.select(category: selectedCategory)
     }
 
     func showPasswordManagerPopover(selectedWebsiteAccount: SecureVaultModels.WebsiteAccount, usingView view: NSView, withDelegate delegate: NSPopoverDelegate) {
         let popover = passwordManagementPopover ?? PasswordManagementPopover()
         passwordManagementPopover = popover
         popover.delegate = delegate
-        show(popover: popover, usingView: view)
+        show(popover, positionedBelow: view)
         popover.select(websiteAccount: selectedWebsiteAccount)
     }
 
@@ -251,27 +256,27 @@ final class NavigationBarPopovers {
         let popover = SaveCredentialsPopover()
         popover.delegate = delegate
         saveCredentialsPopover = popover
-        show(popover: popover, usingView: view)
+        show(popover, positionedBelow: view)
     }
 
     private func showSavePaymentMethodPopover(usingView view: NSView, withDelegate delegate: NSPopoverDelegate) {
         let popover = SavePaymentMethodPopover()
         popover.delegate = delegate
         savePaymentMethodPopover = popover
-        show(popover: popover, usingView: view)
+        show(popover, positionedBelow: view)
     }
 
     private func showSaveIdentityPopover(usingView view: NSView, withDelegate delegate: NSPopoverDelegate) {
         let popover = SaveIdentityPopover()
         popover.delegate = delegate
         saveIdentityPopover = popover
-        show(popover: popover, usingView: view)
+        show(popover, positionedBelow: view)
     }
 
-    private func show(popover: NSPopover, usingView view: NSView, preferredEdge edge: NSRectEdge = .minY) {
+    private func show(_ popover: NSPopover, positionedBelow view: NSView) {
         view.isHidden = false
 
-        popover.show(relativeTo: view.bounds.insetFromLineOfDeath(), of: view, preferredEdge: edge)
+        popover.show(positionedBelow: view.bounds.insetFromLineOfDeath(flipped: view.isFlipped), in: view)
     }
 
     // MARK: - Network Protection
@@ -312,7 +317,7 @@ final class NavigationBarPopovers {
             networkProtectionPopover = popover
             return popover
         }()
-        show(popover: popover, usingView: view, preferredEdge: .maxY)
+        show(popover, positionedBelow: view)
     }
 #endif
 }

DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift

@@ -256,7 +256,8 @@ final class NavigationBarViewController: NSViewController {
                                    passwordManagerCoordinator: PasswordManagerCoordinator.shared,
                                    internalUserDecider: internalUserDecider)
         menu.actionDelegate = self
-        menu.popUp(positioning: nil, at: NSPoint(x: 0, y: sender.bounds.height + 4), in: sender)
+        let location = NSPoint(x: -menu.size.width + sender.bounds.width, y: sender.bounds.height + 4)
+        menu.popUp(positioning: nil, at: location, in: sender)
     }
     // swiftlint:enable force_cast