From c0936e3272bc0488b28176a49f471681bc87e3dc Mon Sep 17 00:00:00 2001 From: Ethan Wong Date: Sat, 12 Nov 2022 10:33:45 +0800 Subject: [PATCH 1/3] Prefer default button implementation for standard toolbar items. --- Mac/MainWindow/MainWindowController.swift | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Mac/MainWindow/MainWindowController.swift b/Mac/MainWindow/MainWindowController.swift index 64dd77e59..5a3afd4e1 100644 --- a/Mac/MainWindow/MainWindowController.swift +++ b/Mac/MainWindow/MainWindowController.swift @@ -1386,19 +1386,25 @@ private extension MainWindowController { } } - func buildToolbarButton(_ itemIdentifier: NSToolbarItem.Identifier, _ title: String, _ image: NSImage, _ selector: String) -> NSToolbarItem { + func buildToolbarButton(_ itemIdentifier: NSToolbarItem.Identifier, _ title: String, _ image: NSImage, _ selector: String, usesCustomButtonView: Bool = false) -> NSToolbarItem { let toolbarItem = RSToolbarItem(itemIdentifier: itemIdentifier) toolbarItem.autovalidates = true - let button = NSButton() - button.bezelStyle = .texturedRounded - button.image = image - button.imageScaling = .scaleProportionallyDown - button.action = Selector((selector)) - - toolbarItem.view = button toolbarItem.toolTip = title toolbarItem.label = title + + if usesCustomButtonView { + let button = NSButton() + button.bezelStyle = .texturedRounded + button.image = image + button.imageScaling = .scaleProportionallyDown + button.action = Selector((selector)) + toolbarItem.view = button + } else { + toolbarItem.image = image + toolbarItem.isBordered = true + toolbarItem.action = Selector((selector)) + } return toolbarItem } From d3ad6c5a0f5f37717ee8e18e0761e5d630409f28 Mon Sep 17 00:00:00 2001 From: Ethan Wong Date: Sat, 12 Nov 2022 10:34:24 +0800 Subject: [PATCH 2/3] Adding menuFormRepresentation to toolbars to properly handle text-only mode. --- Mac/MainWindow/MainWindowController.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Mac/MainWindow/MainWindowController.swift b/Mac/MainWindow/MainWindowController.swift index 5a3afd4e1..5a98acfea 100644 --- a/Mac/MainWindow/MainWindowController.swift +++ b/Mac/MainWindow/MainWindowController.swift @@ -894,6 +894,7 @@ extension MainWindowController: NSToolbarDelegate { button.action = #selector(toggleArticleExtractor(_:)) button.rightClickAction = #selector(showArticleExtractorMenu(_:)) toolbarItem.view = button + toolbarItem.menuFormRepresentation = NSMenuItem(title: description, action: #selector(toggleArticleExtractor(_:)), keyEquivalent: "") return toolbarItem case .share: @@ -1400,6 +1401,7 @@ private extension MainWindowController { button.imageScaling = .scaleProportionallyDown button.action = Selector((selector)) toolbarItem.view = button + toolbarItem.menuFormRepresentation = NSMenuItem(title: title, action: Selector((selector)), keyEquivalent: "") } else { toolbarItem.image = image toolbarItem.isBordered = true From 220b2287a0d28a9acfc02cafb62c7c8ed250615e Mon Sep 17 00:00:00 2001 From: Ethan Wong Date: Sat, 12 Nov 2022 10:35:22 +0800 Subject: [PATCH 3/3] Improve shareToolbarItem's behavior in text-only mode. --- Mac/MainWindow/MainWindowController.swift | 40 ++++++++++++++++--- .../Timeline/TimelineViewController.swift | 2 +- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Mac/MainWindow/MainWindowController.swift b/Mac/MainWindow/MainWindowController.swift index 5a98acfea..9a96dcfab 100644 --- a/Mac/MainWindow/MainWindowController.swift +++ b/Mac/MainWindow/MainWindowController.swift @@ -529,16 +529,17 @@ class MainWindowController : NSWindowController, NSUserInterfaceValidations { assertionFailure("Expected toolbarShowShareMenu to be called only by the Share item in the toolbar.") return } - guard let view = shareToolbarItem.view else { - // TODO: handle menu form representation - return - } let sortedArticles = selectedArticles.sortedByDate(.orderedAscending) let items = sortedArticles.map { ArticlePasteboardWriter(article: $0) } let sharingServicePicker = NSSharingServicePicker(items: items) sharingServicePicker.delegate = sharingServicePickerDelegate - sharingServicePicker.show(relativeTo: view.bounds, of: view, preferredEdge: .minY) + + if let view = shareToolbarItem.view, view.window != nil { + sharingServicePicker.show(relativeTo: view.bounds, of: view, preferredEdge: .minY) + } else if let view = window?.contentView { + sharingServicePicker.show(relativeTo: CGRect(x: view.frame.width / 2.0, y: view.frame.height - 4, width: 1, height: 1), of: view, preferredEdge: .minY) + } } @IBAction func moveFocusToSearchField(_ sender: Any?) { @@ -666,6 +667,9 @@ extension MainWindowController: TimelineContainerViewControllerDelegate { articleExtractor = nil isShowingExtractedArticle = false makeToolbarValidate() + if #available(macOS 13.0, *) { } else { + updateShareToolbarItemMenu() + } let detailState: DetailState if let articles = articles { @@ -899,7 +903,19 @@ extension MainWindowController: NSToolbarDelegate { case .share: let title = NSLocalizedString("Share", comment: "Share") - return buildToolbarButton(.share, title, AppAssets.shareImage, "toolbarShowShareMenu:") + let image = AppAssets.shareImage + // Should use NSSharingServicePickerToolbarItem here, but it has unexpected behavior on label-only mode. + if #available(macOS 13.0, *) { + // `item.view` is required for properly positioning the sharing picker. + return buildToolbarButton(.share, title, image, "toolbarShowShareMenu:", usesCustomButtonView: true) + } else { + let item = NSMenuToolbarItem(itemIdentifier: .share) + item.image = image + item.toolTip = title + item.label = title + item.showsIndicator = false + return item + } case .openInBrowser: let title = NSLocalizedString("Open in Browser", comment: "Open in Browser") @@ -1459,5 +1475,17 @@ private extension MainWindowController { articleThemePopUpButton?.menu = articleThemeMenu } + func updateShareToolbarItemMenu() { + guard let shareToolbarItem = shareToolbarItem as? NSMenuToolbarItem else { + return + } + if let shareMenu = shareMenu { + shareToolbarItem.isEnabled = true + shareToolbarItem.menu = shareMenu + } else { + shareToolbarItem.isEnabled = false + } + } + } diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index cb9c7e9d3..27b303339 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -65,7 +65,6 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr if !representedObjectArraysAreEqual(oldValue, representedObjects) { unreadCount = 0 - selectionDidChange(nil) if showsSearchResults { fetchAndReplaceArticlesAsync() } else { @@ -75,6 +74,7 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr } updateUnreadCount() } + selectionDidChange(nil) } } }