From 2d2c14f9baca6c2a8c7b398d007b0b069dc90448 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Mon, 27 Jul 2020 13:54:34 -0400 Subject: [PATCH 01/12] Check if authorized for notifications in AppDelegate before registering --- Mac/AppDelegate.swift | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Mac/AppDelegate.swift b/Mac/AppDelegate.swift index 1937c8c3b..322b772f7 100644 --- a/Mac/AppDelegate.swift +++ b/Mac/AppDelegate.swift @@ -231,8 +231,13 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserInterfaceValidations, refreshTimer = AccountRefreshTimer() syncTimer = ArticleStatusSyncTimer() - UNUserNotificationCenter.current().requestAuthorization(options:[.badge, .sound, .alert]) { (granted, error) in } - NSApplication.shared.registerForRemoteNotifications() + UNUserNotificationCenter.current().getNotificationSettings { (settings) in + if settings.authorizationStatus == .authorized { + DispatchQueue.main.async { + NSApplication.shared.registerForRemoteNotifications() + } + } + } UNUserNotificationCenter.current().delegate = self userNotificationManager = UserNotificationManager() From 927db2c37507b26ebd3960257ac6e678598160ff Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sat, 1 Aug 2020 11:50:57 -0400 Subject: [PATCH 02/12] Import UserNotifications --- Mac/Inspector/WebFeedInspectorViewController.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 9db23b335..fdba11c40 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -9,6 +9,7 @@ import AppKit import Articles import Account +import UserNotifications final class WebFeedInspectorViewController: NSViewController, Inspector { @@ -27,6 +28,8 @@ final class WebFeedInspectorViewController: NSViewController, Inspector { } } + private var userNotificationSettings: UNNotificationSettings? + // MARK: Inspector let isFallbackInspector = false From 1123a0be865372f54c3573a1722a68b2b009c720 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sat, 1 Aug 2020 11:52:33 -0400 Subject: [PATCH 03/12] Try to register for remote notifications when feed inspector appears --- .../WebFeedInspectorViewController.swift | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index fdba11c40..2962b17c5 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -49,7 +49,11 @@ final class WebFeedInspectorViewController: NSViewController, Inspector { updateUI() NotificationCenter.default.addObserver(self, selector: #selector(imageDidBecomeAvailable(_:)), name: .ImageDidBecomeAvailable, object: nil) } - + + override func viewDidAppear() { + updateNotificationSettings() + } + // MARK: Actions @IBAction func isNotifyAboutNewArticlesChanged(_ sender: Any) { feed?.isNotifyAboutNewArticles = (isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false @@ -135,7 +139,18 @@ private extension WebFeedInspectorViewController { func updateFeedURL() { urlTextField?.stringValue = feed?.url ?? "" } - + + func updateNotificationSettings() { + UNUserNotificationCenter.current().getNotificationSettings { (settings) in + DispatchQueue.main.async { + self.userNotificationSettings = settings + if settings.authorizationStatus == .authorized { + NSApplication.shared.registerForRemoteNotifications() + } + } + } + } + func updateNotifyAboutNewArticles() { isNotifyAboutNewArticlesCheckBox?.state = (feed?.isNotifyAboutNewArticles ?? false) ? .on : .off } From 952d066199f29eea735a28444270a129f8ab4465 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sat, 1 Aug 2020 11:53:26 -0400 Subject: [PATCH 04/12] Apply same behavior on checkbox toggle as iOS app --- .../WebFeedInspectorViewController.swift | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 2962b17c5..92f7ab7e5 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -56,7 +56,40 @@ final class WebFeedInspectorViewController: NSViewController, Inspector { // MARK: Actions @IBAction func isNotifyAboutNewArticlesChanged(_ sender: Any) { - feed?.isNotifyAboutNewArticles = (isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false + guard let settings = userNotificationSettings else { + // Something went wront fetching the user notification settings, + // so toggle the checkbox back to its original state and return. + isNotifyAboutNewArticlesCheckBox.setNextState() + return + } + if settings.authorizationStatus == .denied { + // Notifications are not authorized, so toggle the checkbox back + // to its original state... + isNotifyAboutNewArticlesCheckBox.setNextState() + // ...and then alert the user to the issue + // TODO: present alert to user + } else if settings.authorizationStatus == .authorized { + // Notifications are authorized, so set the feed's isNotifyAboutNewArticles + // property to match the state of isNotifyAboutNewArticlesCheckbox. + feed?.isNotifyAboutNewArticles = (isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false + } else { + // We're not sure what the status may be but we've /probably/ not requested + // permission to send notifications, so do that: + UNUserNotificationCenter.current().requestAuthorization(options: [.badge, .sound, .alert]) { (granted, error) in + self.updateNotificationSettings() + if granted { + // We've been given permission, so set the feed's isNotifyAboutNewArticles + // property to match the state of isNotifyAboutNewArticlesCheckbox and then + // register for remote notifications. + self.feed?.isNotifyAboutNewArticles = (self.isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false + NSApplication.shared.registerForRemoteNotifications() + } else { + // We weren't given permission, so toggle the checkbox back to its + // original state. + self.isNotifyAboutNewArticlesCheckBox.setNextState() + } + } + } } @IBAction func isReaderViewAlwaysOnChanged(_ sender: Any) { From 42cfe2380a4324a3b948003783ab9013e60a37e1 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sat, 1 Aug 2020 13:02:24 -0400 Subject: [PATCH 05/12] Present an alert to the user if notifications are denied in System Prefs --- .../WebFeedInspectorViewController.swift | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 92f7ab7e5..04dccc4b4 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -66,8 +66,8 @@ final class WebFeedInspectorViewController: NSViewController, Inspector { // Notifications are not authorized, so toggle the checkbox back // to its original state... isNotifyAboutNewArticlesCheckBox.setNextState() - // ...and then alert the user to the issue - // TODO: present alert to user + // ...then alert the user to the issue... + showNotificationsDeniedError() } else if settings.authorizationStatus == .authorized { // Notifications are authorized, so set the feed's isNotifyAboutNewArticles // property to match the state of isNotifyAboutNewArticlesCheckbox. @@ -184,6 +184,19 @@ private extension WebFeedInspectorViewController { } } + func showNotificationsDeniedError() { + let updateAlert = NSAlert() + updateAlert.alertStyle = .informational + updateAlert.messageText = NSLocalizedString("Enable Notifications", comment: "Notifications") + updateAlert.informativeText = NSLocalizedString("Notifications need to be enabled in System Preferences > Notifications.", comment: "Notifications need to be enabled in System Preferences > Notifications.") + updateAlert.addButton(withTitle: NSLocalizedString("Open System Preferences", comment: "Open System Preferences")) + updateAlert.addButton(withTitle: NSLocalizedString("Close", comment: "Close")) + let modalResponse = updateAlert.runModal() + if modalResponse == .alertFirstButtonReturn { + NSWorkspace.shared.open(URL(fileURLWithPath: "x-apple.systempreferences:com.apple.preference")) + } + } + func updateNotifyAboutNewArticles() { isNotifyAboutNewArticlesCheckBox?.state = (feed?.isNotifyAboutNewArticles ?? false) ? .on : .off } From 619ddc6ea2aa16647b6bd5c7b6355ff31bb7e347 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sat, 1 Aug 2020 13:09:33 -0400 Subject: [PATCH 06/12] Remove comments, and move post-request actions to the main queue --- .../WebFeedInspectorViewController.swift | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 04dccc4b4..711ac1472 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -57,36 +57,26 @@ final class WebFeedInspectorViewController: NSViewController, Inspector { // MARK: Actions @IBAction func isNotifyAboutNewArticlesChanged(_ sender: Any) { guard let settings = userNotificationSettings else { - // Something went wront fetching the user notification settings, - // so toggle the checkbox back to its original state and return. isNotifyAboutNewArticlesCheckBox.setNextState() return } if settings.authorizationStatus == .denied { - // Notifications are not authorized, so toggle the checkbox back - // to its original state... isNotifyAboutNewArticlesCheckBox.setNextState() - // ...then alert the user to the issue... showNotificationsDeniedError() } else if settings.authorizationStatus == .authorized { - // Notifications are authorized, so set the feed's isNotifyAboutNewArticles - // property to match the state of isNotifyAboutNewArticlesCheckbox. feed?.isNotifyAboutNewArticles = (isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false } else { - // We're not sure what the status may be but we've /probably/ not requested - // permission to send notifications, so do that: UNUserNotificationCenter.current().requestAuthorization(options: [.badge, .sound, .alert]) { (granted, error) in self.updateNotificationSettings() if granted { - // We've been given permission, so set the feed's isNotifyAboutNewArticles - // property to match the state of isNotifyAboutNewArticlesCheckbox and then - // register for remote notifications. - self.feed?.isNotifyAboutNewArticles = (self.isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false - NSApplication.shared.registerForRemoteNotifications() + DispatchQueue.main.async { + self.feed?.isNotifyAboutNewArticles = (self.isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false + NSApplication.shared.registerForRemoteNotifications() + } } else { - // We weren't given permission, so toggle the checkbox back to its - // original state. - self.isNotifyAboutNewArticlesCheckBox.setNextState() + DispatchQueue.main.async { + self.isNotifyAboutNewArticlesCheckBox.setNextState() + } } } } From 288a5b18db134857d8c5eaf0dfcfdcaed2d59939 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sun, 2 Aug 2020 08:41:48 -0400 Subject: [PATCH 07/12] Improve the open-notifications-prefPane alert's message --- Mac/Inspector/WebFeedInspectorViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 711ac1472..9a8336cb0 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -178,7 +178,7 @@ private extension WebFeedInspectorViewController { let updateAlert = NSAlert() updateAlert.alertStyle = .informational updateAlert.messageText = NSLocalizedString("Enable Notifications", comment: "Notifications") - updateAlert.informativeText = NSLocalizedString("Notifications need to be enabled in System Preferences > Notifications.", comment: "Notifications need to be enabled in System Preferences > Notifications.") + updateAlert.informativeText = NSLocalizedString("To enable notifications, open Notifications in System Preferences, then find NetNewsWire in the list.", comment: "To enable notifications, open Notifications in System Preferences, then find NetNewsWire in the list.") updateAlert.addButton(withTitle: NSLocalizedString("Open System Preferences", comment: "Open System Preferences")) updateAlert.addButton(withTitle: NSLocalizedString("Close", comment: "Close")) let modalResponse = updateAlert.runModal() From 3cb47afe65e7d4975b93732496c0292530726e4a Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sun, 2 Aug 2020 08:43:24 -0400 Subject: [PATCH 08/12] Group all update-UI functions together in file --- Mac/Inspector/WebFeedInspectorViewController.swift | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 9a8336cb0..5e35e19a7 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -163,6 +163,14 @@ private extension WebFeedInspectorViewController { urlTextField?.stringValue = feed?.url ?? "" } + func updateNotifyAboutNewArticles() { + isNotifyAboutNewArticlesCheckBox?.state = (feed?.isNotifyAboutNewArticles ?? false) ? .on : .off + } + + func updateIsReaderViewAlwaysOn() { + isReaderViewAlwaysOnCheckBox?.state = (feed?.isArticleExtractorAlwaysOn ?? false) ? .on : .off + } + func updateNotificationSettings() { UNUserNotificationCenter.current().getNotificationSettings { (settings) in DispatchQueue.main.async { @@ -186,12 +194,6 @@ private extension WebFeedInspectorViewController { NSWorkspace.shared.open(URL(fileURLWithPath: "x-apple.systempreferences:com.apple.preference")) } } - - func updateNotifyAboutNewArticles() { - isNotifyAboutNewArticlesCheckBox?.state = (feed?.isNotifyAboutNewArticles ?? false) ? .on : .off } - func updateIsReaderViewAlwaysOn() { - isReaderViewAlwaysOnCheckBox?.state = (feed?.isArticleExtractorAlwaysOn ?? false) ? .on : .off - } } From 193aab2ceff02298f40c87fd2a20c905fb0d1ce0 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sun, 2 Aug 2020 08:43:55 -0400 Subject: [PATCH 09/12] Open the Notifications prefPane directly in System Preferences --- Mac/Inspector/WebFeedInspectorViewController.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 5e35e19a7..91e146a8e 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -191,9 +191,8 @@ private extension WebFeedInspectorViewController { updateAlert.addButton(withTitle: NSLocalizedString("Close", comment: "Close")) let modalResponse = updateAlert.runModal() if modalResponse == .alertFirstButtonReturn { - NSWorkspace.shared.open(URL(fileURLWithPath: "x-apple.systempreferences:com.apple.preference")) + NSWorkspace.shared.open(URL(fileURLWithPath: "x-apple.systempreferences:com.apple.preference.notifications")) } } - } } From 88ac27ef502e0c6e4c6ab20cf33c8334a5ab31ab Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sun, 2 Aug 2020 08:45:36 -0400 Subject: [PATCH 10/12] Only dispatch UI code for async execution --- Mac/Inspector/WebFeedInspectorViewController.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 91e146a8e..43f8bfed6 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -173,9 +173,9 @@ private extension WebFeedInspectorViewController { func updateNotificationSettings() { UNUserNotificationCenter.current().getNotificationSettings { (settings) in - DispatchQueue.main.async { - self.userNotificationSettings = settings - if settings.authorizationStatus == .authorized { + self.userNotificationSettings = settings + if settings.authorizationStatus == .authorized { + DispatchQueue.main.async { NSApplication.shared.registerForRemoteNotifications() } } From a0fea768bf2133ffde593bd4ed51ca1ecde91d10 Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sun, 2 Aug 2020 08:47:14 -0400 Subject: [PATCH 11/12] Fix race condition between getting and checking notification settings --- .../WebFeedInspectorViewController.swift | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 43f8bfed6..5f2a13a65 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -60,22 +60,31 @@ final class WebFeedInspectorViewController: NSViewController, Inspector { isNotifyAboutNewArticlesCheckBox.setNextState() return } - if settings.authorizationStatus == .denied { - isNotifyAboutNewArticlesCheckBox.setNextState() - showNotificationsDeniedError() - } else if settings.authorizationStatus == .authorized { - feed?.isNotifyAboutNewArticles = (isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false - } else { - UNUserNotificationCenter.current().requestAuthorization(options: [.badge, .sound, .alert]) { (granted, error) in - self.updateNotificationSettings() - if granted { - DispatchQueue.main.async { - self.feed?.isNotifyAboutNewArticles = (self.isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false - NSApplication.shared.registerForRemoteNotifications() - } - } else { - DispatchQueue.main.async { - self.isNotifyAboutNewArticlesCheckBox.setNextState() + + UNUserNotificationCenter.current().getNotificationSettings { (settings) in + self.updateNotificationSettings() + + if settings.authorizationStatus == .denied { + DispatchQueue.main.async { + self.isNotifyAboutNewArticlesCheckBox.setNextState() + self.showNotificationsDeniedError() + } + } else if settings.authorizationStatus == .authorized { + DispatchQueue.main.async { + self.feed?.isNotifyAboutNewArticles = (self.isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false + } + } else { + UNUserNotificationCenter.current().requestAuthorization(options: [.badge, .sound, .alert]) { (granted, error) in + self.updateNotificationSettings() + if granted { + DispatchQueue.main.async { + self.feed?.isNotifyAboutNewArticles = (self.isNotifyAboutNewArticlesCheckBox?.state ?? .off) == .on ? true : false + NSApplication.shared.registerForRemoteNotifications() + } + } else { + DispatchQueue.main.async { + self.isNotifyAboutNewArticlesCheckBox.setNextState() + } } } } From be8fedbf1186e5f1d8f1a23d5f066e72d4d16b4f Mon Sep 17 00:00:00 2001 From: Angelo Stavrow Date: Sun, 2 Aug 2020 08:49:37 -0400 Subject: [PATCH 12/12] Use conditional in guard statement instead of setting an unused variable --- Mac/Inspector/WebFeedInspectorViewController.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index 5f2a13a65..ce0b608c0 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -56,8 +56,10 @@ final class WebFeedInspectorViewController: NSViewController, Inspector { // MARK: Actions @IBAction func isNotifyAboutNewArticlesChanged(_ sender: Any) { - guard let settings = userNotificationSettings else { - isNotifyAboutNewArticlesCheckBox.setNextState() + guard userNotificationSettings != nil else { + DispatchQueue.main.async { + self.isNotifyAboutNewArticlesCheckBox.setNextState() + } return }