From 8a4156542b3bd21241438271c83c319be05bcb71 Mon Sep 17 00:00:00 2001 From: Stuart Breckenridge Date: Fri, 4 Feb 2022 10:10:32 +0800 Subject: [PATCH 1/4] Notifications Manager Perf Improvements Adopts prefetch for smooth scrolling --- iOS/Settings/NotificationsTableViewCell.swift | 19 ++---------- .../NotificationsViewController.swift | 30 +++++++++++-------- iOS/Settings/Settings.storyboard | 4 +-- iOS/Settings/SettingsViewController.swift | 4 +-- 4 files changed, 25 insertions(+), 32 deletions(-) diff --git a/iOS/Settings/NotificationsTableViewCell.swift b/iOS/Settings/NotificationsTableViewCell.swift index 1058687ec..cf0c66beb 100644 --- a/iOS/Settings/NotificationsTableViewCell.swift +++ b/iOS/Settings/NotificationsTableViewCell.swift @@ -10,11 +10,8 @@ import UIKit import Account import UserNotifications -extension Notification.Name { - static let NotificationPreferencesDidUpdate = Notification.Name("NotificationPreferencesDidUpdate") -} -class NotificationsTableViewCell: UITableViewCell { +class NotificationsTableViewCell: VibrantBasicTableViewCell { @IBOutlet weak var notificationsSwitch: UISwitch! @IBOutlet weak var notificationsLabel: UILabel! @@ -33,7 +30,7 @@ class NotificationsTableViewCell: UITableViewCell { // Configure the view for the selected state } - func configure(_ webFeed: WebFeed, _ status: UNAuthorizationStatus) { + func configure(_ webFeed: WebFeed) { self.feed = webFeed var isOn = false if webFeed.isNotifyAboutNewArticles == nil { @@ -43,9 +40,8 @@ class NotificationsTableViewCell: UITableViewCell { } notificationsSwitch.isOn = isOn notificationsSwitch.addTarget(self, action: #selector(toggleWebFeedNotification(_:)), for: .touchUpInside) - if status == .denied { notificationsSwitch.isEnabled = false } notificationsLabel.text = webFeed.nameForDisplay - notificationsImageView.image = webFeed.smallIcon?.image + notificationsImageView.image = IconImageCache.shared.imageFor(webFeed.feedID!)?.image notificationsImageView.layer.cornerRadius = 4 } @@ -61,14 +57,5 @@ class NotificationsTableViewCell: UITableViewCell { feed.isNotifyAboutNewArticles!.toggle() } } - - @objc - private func requestNotificationPermissions(_ sender: Any) { - UNUserNotificationCenter.current().requestAuthorization(options: [.alert, .badge, .sound]) { success, error in - NotificationCenter.default.post(name: .NotificationPreferencesDidUpdate, object: nil) - } - } - - } diff --git a/iOS/Settings/NotificationsViewController.swift b/iOS/Settings/NotificationsViewController.swift index f72e76567..557ef9f1a 100644 --- a/iOS/Settings/NotificationsViewController.swift +++ b/iOS/Settings/NotificationsViewController.swift @@ -18,15 +18,10 @@ class NotificationsViewController: UIViewController { override func viewDidLoad() { super.viewDidLoad() + notificationsTableView.prefetchDataSource = self self.title = NSLocalizedString("New Article Notifications", comment: "Notifications") - notificationsTableView.sectionHeaderTopPadding = 25 - - NotificationCenter.default.addObserver(self, selector: #selector(reloadNotificationTableView(_:)), name: .FaviconDidBecomeAvailable, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(reloadNotificationTableView(_:)), name: .WebFeedIconDidBecomeAvailable, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(reloadNotificationTableView(_:)), name: .NotificationPreferencesDidUpdate, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(reloadNotificationTableView(_:)), name: UIScene.willEnterForegroundNotification, object: nil) - reloadNotificationTableView() + NotificationCenter.default.addObserver(self, selector: #selector(reloadNotificationTableView(_:)), name: UIScene.willEnterForegroundNotification, object: nil) } @objc @@ -62,16 +57,13 @@ extension NotificationsViewController: UITableViewDataSource { } func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { - - if indexPath.section == 0 { let openSettingsCell = tableView.dequeueReusableCell(withIdentifier: "OpenSettingsCell") as! VibrantBasicTableViewCell return openSettingsCell } else { let cell = tableView.dequeueReusableCell(withIdentifier: "NotificationsCell") as! NotificationsTableViewCell let account = AccountManager.shared.sortedActiveAccounts[indexPath.section - 1] - let feed = sortedWebFeedsForAccount(account)[indexPath.row] - cell.configure(feed, status) + cell.configure(sortedWebFeedsForAccount(account)[indexPath.row]) return cell } } @@ -96,8 +88,22 @@ extension NotificationsViewController: UITableViewDelegate { func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { tableView.deselectRow(at: indexPath, animated: true) - UIApplication.shared.open(URL(string: "\(UIApplication.openSettingsURLString)")!) + if indexPath.section == 0 { + UIApplication.shared.open(URL(string: "\(UIApplication.openSettingsURLString)")!) + } } +} + + +extension NotificationsViewController: UITableViewDataSourcePrefetching { + + func tableView(_ tableView: UITableView, prefetchRowsAt indexPaths: [IndexPath]) { + for path in indexPaths { + let account = AccountManager.shared.sortedActiveAccounts[path.section - 1] + let feed = sortedWebFeedsForAccount(account)[path.row] + let _ = IconImageCache.shared.imageFor(feed.feedID!) + } + } } diff --git a/iOS/Settings/Settings.storyboard b/iOS/Settings/Settings.storyboard index 385a825db..811612d5f 100644 --- a/iOS/Settings/Settings.storyboard +++ b/iOS/Settings/Settings.storyboard @@ -1110,7 +1110,7 @@ - + @@ -1194,7 +1194,7 @@ - + diff --git a/iOS/Settings/SettingsViewController.swift b/iOS/Settings/SettingsViewController.swift index ef845f19e..043f8505f 100644 --- a/iOS/Settings/SettingsViewController.swift +++ b/iOS/Settings/SettingsViewController.swift @@ -47,6 +47,8 @@ class SettingsViewController: UITableViewController { tableView.register(UINib(nibName: "SettingsComboTableViewCell", bundle: nil), forCellReuseIdentifier: "SettingsComboTableViewCell") tableView.register(UINib(nibName: "SettingsTableViewCell", bundle: nil), forCellReuseIdentifier: "SettingsTableViewCell") + refreshNotificationStatus() + tableView.rowHeight = UITableView.automaticDimension tableView.estimatedRowHeight = 44 } @@ -72,7 +74,6 @@ class SettingsViewController: UITableViewController { refreshClearsReadArticlesSwitch.isOn = false } - articleThemeDetailLabel.text = ArticleThemesManager.shared.currentTheme.name if AppDefaults.shared.confirmMarkAllAsRead { @@ -112,7 +113,6 @@ class SettingsViewController: UITableViewController { tableView.scrollToRow(at: IndexPath(row: 0, section: 4), at: .top, animated: true) scrollToArticlesSection = false } - refreshNotificationStatus() } @objc From 5b5414c7a8cd04a6df7026bf23e560353e053488 Mon Sep 17 00:00:00 2001 From: Stuart Breckenridge Date: Fri, 4 Feb 2022 13:20:59 +0800 Subject: [PATCH 2/4] Adds filtering and search to Notification Manager --- .../NotificationsViewController.swift | 150 ++++++++++++++++-- iOS/Settings/Settings.storyboard | 12 +- 2 files changed, 145 insertions(+), 17 deletions(-) diff --git a/iOS/Settings/NotificationsViewController.swift b/iOS/Settings/NotificationsViewController.swift index 557ef9f1a..f8b4e9949 100644 --- a/iOS/Settings/NotificationsViewController.swift +++ b/iOS/Settings/NotificationsViewController.swift @@ -11,16 +11,46 @@ import Account import UserNotifications class NotificationsViewController: UIViewController { - + @IBOutlet weak var notificationsTableView: UITableView! + + private lazy var searchController: UISearchController = { + let searchController = UISearchController(searchResultsController: nil) + searchController.searchBar.placeholder = NSLocalizedString("Find a feed", comment: "Find a feed") + searchController.searchBar.searchBarStyle = .minimal + searchController.delegate = self + searchController.searchBar.delegate = self + searchController.searchBar.sizeToFit() + searchController.obscuresBackgroundDuringPresentation = false + searchController.hidesNavigationBarDuringPresentation = false + self.definesPresentationContext = true + return searchController + }() private var status: UNAuthorizationStatus = .notDetermined + private var newArticleNotificationFilter: Bool = false { + didSet { + filterButton.menu = notificationFilterMenu() + } + } + private var filterButton: UIBarButtonItem! - - override func viewDidLoad() { - super.viewDidLoad() + override func viewDidLoad() { + super.viewDidLoad() + title = NSLocalizedString("New Article Notifications", comment: "Notifications") + notificationsTableView.prefetchDataSource = self - self.title = NSLocalizedString("New Article Notifications", comment: "Notifications") + navigationItem.searchController = searchController + + filterButton = UIBarButtonItem( + title: nil, + image: AppAssets.filterInactiveImage, + primaryAction: nil, + menu: notificationFilterMenu()) + + navigationItem.rightBarButtonItem = filterButton + reloadNotificationTableView() + NotificationCenter.default.addObserver(self, selector: #selector(reloadNotificationTableView(_:)), name: UIScene.willEnterForegroundNotification, object: nil) } @@ -29,18 +59,65 @@ class NotificationsViewController: UIViewController { UNUserNotificationCenter.current().getNotificationSettings { settings in DispatchQueue.main.async { self.status = settings.authorizationStatus + if self.status != .authorized { + self.filterButton.isEnabled = false + self.newArticleNotificationFilter = false + } self.notificationsTableView.reloadData() } } } + private func notificationFilterMenu() -> UIMenu { + + if let filterButton = filterButton { + if newArticleNotificationFilter == true { + filterButton.image = AppAssets.filterActiveImage + } else { + filterButton.image = AppAssets.filterInactiveImage + } + } + + let menu = UIMenu(title: "", + image: nil, + identifier: nil, + options: [.displayInline], + children: [ + UIAction( + title: NSLocalizedString("Show Feeds with Notifications Enabled", comment: "Feeds with Notifications"), + image: UIImage(systemName: "app.badge"), + identifier: nil, + discoverabilityTitle: nil, + attributes: [], + state: newArticleNotificationFilter ? .on : .off, + handler: { [weak self] _ in + self?.newArticleNotificationFilter.toggle() + self?.notificationsTableView.reloadData() + })]) + return menu + } + + // MARK: - Feed Filtering + private func sortedWebFeedsForAccount(_ account: Account) -> [WebFeed] { return Array(account.flattenedWebFeeds()).sorted(by: { $0.nameForDisplay.caseInsensitiveCompare($1.nameForDisplay) == .orderedAscending }) } - + + private func filteredWebFeeds(_ searchText: String? = "", account: Account) -> [WebFeed] { + sortedWebFeedsForAccount(account).filter { feed in + return feed.nameForDisplay.lowercased().contains(searchText!.lowercased()) + } + } + + private func feedsWithNotificationsEnabled(_ account: Account) -> [WebFeed] { + sortedWebFeedsForAccount(account).filter { feed in + return feed.isNotifyAboutNewArticles == true + } + } + } -// MARK: UITableViewDataSource +// MARK: - UITableViewDataSource extension NotificationsViewController: UITableViewDataSource { func numberOfSections(in tableView: UITableView) -> Int { @@ -53,7 +130,14 @@ extension NotificationsViewController: UITableViewDataSource { if status == .denied { return 1 } return 0 } - return AccountManager.shared.sortedActiveAccounts[section - 1].flattenedWebFeeds().count + if searchController.isActive { + return filteredWebFeeds(searchController.searchBar.text, account: AccountManager.shared.sortedActiveAccounts[section - 1]).count + } else if newArticleNotificationFilter == true { + return feedsWithNotificationsEnabled(AccountManager.shared.sortedActiveAccounts[section - 1]).count + } else { + return AccountManager.shared.sortedActiveAccounts[section - 1].flattenedWebFeeds().count + } + } func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { @@ -61,14 +145,25 @@ extension NotificationsViewController: UITableViewDataSource { let openSettingsCell = tableView.dequeueReusableCell(withIdentifier: "OpenSettingsCell") as! VibrantBasicTableViewCell return openSettingsCell } else { - let cell = tableView.dequeueReusableCell(withIdentifier: "NotificationsCell") as! NotificationsTableViewCell - let account = AccountManager.shared.sortedActiveAccounts[indexPath.section - 1] - cell.configure(sortedWebFeedsForAccount(account)[indexPath.row]) - return cell + if searchController.isActive { + let cell = tableView.dequeueReusableCell(withIdentifier: "NotificationsCell") as! NotificationsTableViewCell + let account = AccountManager.shared.sortedActiveAccounts[indexPath.section - 1] + cell.configure(filteredWebFeeds(searchController.searchBar.text, account: account)[indexPath.row]) + return cell + } else if newArticleNotificationFilter == true { + let cell = tableView.dequeueReusableCell(withIdentifier: "NotificationsCell") as! NotificationsTableViewCell + let account = AccountManager.shared.sortedActiveAccounts[indexPath.section - 1] + cell.configure(feedsWithNotificationsEnabled(account)[indexPath.row]) + return cell + } else { + let cell = tableView.dequeueReusableCell(withIdentifier: "NotificationsCell") as! NotificationsTableViewCell + let account = AccountManager.shared.sortedActiveAccounts[indexPath.section - 1] + cell.configure(sortedWebFeedsForAccount(account)[indexPath.row]) + return cell + } } } - func tableView(_ tableView: UITableView, titleForHeaderInSection section: Int) -> String? { if section == 0 { return nil } return AccountManager.shared.sortedActiveAccounts[section - 1].nameForDisplay @@ -84,6 +179,8 @@ extension NotificationsViewController: UITableViewDataSource { } } + +// MARK: - UITableViewDelegate extension NotificationsViewController: UITableViewDelegate { func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { @@ -107,3 +204,30 @@ extension NotificationsViewController: UITableViewDataSourcePrefetching { } } + + +// MARK: - UISearchControllerDelegate +extension NotificationsViewController: UISearchControllerDelegate { + + func didDismissSearchController(_ searchController: UISearchController) { + print(#function) + searchController.isActive = false + notificationsTableView.reloadData() + } + +} + +// MARK: - UISearchBarDelegate +extension NotificationsViewController: UISearchBarDelegate { + + func searchBarTextDidBeginEditing(_ searchBar: UISearchBar) { + searchController.isActive = true + newArticleNotificationFilter = false + notificationsTableView.reloadData() + } + + func searchBar(_ searchBar: UISearchBar, textDidChange searchText: String) { + notificationsTableView.reloadData() + } + +} diff --git a/iOS/Settings/Settings.storyboard b/iOS/Settings/Settings.storyboard index 811612d5f..1885b1634 100644 --- a/iOS/Settings/Settings.storyboard +++ b/iOS/Settings/Settings.storyboard @@ -1110,7 +1110,7 @@ - + @@ -1189,14 +1189,15 @@ - + - + - + + @@ -1371,6 +1372,9 @@ + + + From c71d06024cf9f32741501f08c8083104c2678d95 Mon Sep 17 00:00:00 2001 From: Stuart Breckenridge Date: Fri, 4 Feb 2022 13:27:59 +0800 Subject: [PATCH 3/4] Moves image to app assets --- iOS/AppAssets.swift | 4 ++++ iOS/Settings/NotificationsViewController.swift | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/iOS/AppAssets.swift b/iOS/AppAssets.swift index 160e5bf2e..26ee3be0f 100644 --- a/iOS/AppAssets.swift +++ b/iOS/AppAssets.swift @@ -54,6 +54,10 @@ struct AppAssets { static var accountTheOldReaderImage: UIImage = { return UIImage(named: "accountTheOldReader")! }() + + static var appBadgeImage: UIImage = { + return UIImage(systemName: "app.badge")! + }() static var articleExtractorError: UIImage = { return UIImage(named: "articleExtractorError")! diff --git a/iOS/Settings/NotificationsViewController.swift b/iOS/Settings/NotificationsViewController.swift index f8b4e9949..c7366ec22 100644 --- a/iOS/Settings/NotificationsViewController.swift +++ b/iOS/Settings/NotificationsViewController.swift @@ -85,7 +85,7 @@ class NotificationsViewController: UIViewController { children: [ UIAction( title: NSLocalizedString("Show Feeds with Notifications Enabled", comment: "Feeds with Notifications"), - image: UIImage(systemName: "app.badge"), + image: AppAssets.appBadgeImage, identifier: nil, discoverabilityTitle: nil, attributes: [], From a19154ac734bc133e451f217e451dec907ba85f0 Mon Sep 17 00:00:00 2001 From: Stuart Breckenridge Date: Sat, 5 Feb 2022 10:22:49 +0800 Subject: [PATCH 4/4] Adds account-wide to notifications manager --- iOS/AppAssets.swift | 4 ++ .../NotificationsViewController.swift | 65 ++++++++++++++++--- iOS/Settings/Settings.storyboard | 2 +- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/iOS/AppAssets.swift b/iOS/AppAssets.swift index 26ee3be0f..79b34c712 100644 --- a/iOS/AppAssets.swift +++ b/iOS/AppAssets.swift @@ -185,6 +185,10 @@ struct AppAssets { return UIImage(systemName: "ellipsis.circle")! }() + static var moreImageFill: UIImage = { + return UIImage(systemName: "ellipsis.circle.fill")! + }() + static var nextArticleImage: UIImage = { return UIImage(systemName: "chevron.down")! }() diff --git a/iOS/Settings/NotificationsViewController.swift b/iOS/Settings/NotificationsViewController.swift index c7366ec22..61b4ed398 100644 --- a/iOS/Settings/NotificationsViewController.swift +++ b/iOS/Settings/NotificationsViewController.swift @@ -43,7 +43,7 @@ class NotificationsViewController: UIViewController { filterButton = UIBarButtonItem( title: nil, - image: AppAssets.filterInactiveImage, + image: AppAssets.moreImage, primaryAction: nil, menu: notificationFilterMenu()) @@ -70,22 +70,23 @@ class NotificationsViewController: UIViewController { private func notificationFilterMenu() -> UIMenu { - if let filterButton = filterButton { - if newArticleNotificationFilter == true { - filterButton.image = AppAssets.filterActiveImage + if filterButton != nil { + if newArticleNotificationFilter { + filterButton.image = AppAssets.moreImageFill } else { - filterButton.image = AppAssets.filterInactiveImage + filterButton.image = AppAssets.moreImage } } - let menu = UIMenu(title: "", + + let filterMenu = UIMenu(title: "", image: nil, identifier: nil, options: [.displayInline], children: [ UIAction( title: NSLocalizedString("Show Feeds with Notifications Enabled", comment: "Feeds with Notifications"), - image: AppAssets.appBadgeImage, + image: nil, identifier: nil, discoverabilityTitle: nil, attributes: [], @@ -94,7 +95,55 @@ class NotificationsViewController: UIViewController { self?.newArticleNotificationFilter.toggle() self?.notificationsTableView.reloadData() })]) - return menu + + + var menus = [UIMenuElement]() + menus.append(filterMenu) + + for account in AccountManager.shared.sortedActiveAccounts { + let accountMenu = UIMenu(title: account.nameForDisplay, image: nil, identifier: nil, options: .singleSelection, children: [enableAllAction(for: account), disableAllAction(for: account)]) + menus.append(accountMenu) + } + + let combinedMenu = UIMenu(title: "", + image: nil, + identifier: nil, + options: .displayInline, + children: menus) + + return combinedMenu + } + + private func enableAllAction(for account: Account) -> UIAction { + let action = UIAction(title: NSLocalizedString("Enable All Notifications", comment: "Enable All"), + image: nil, + identifier: nil, + discoverabilityTitle: nil, + attributes: [], + state: .off) { [weak self] _ in + for feed in account.flattenedWebFeeds() { + feed.isNotifyAboutNewArticles = true + } + self?.notificationsTableView.reloadData() + self?.filterButton.menu = self?.notificationFilterMenu() + } + return action + } + + private func disableAllAction(for account: Account) -> UIAction { + let action = UIAction(title: NSLocalizedString("Disable All Notifications", comment: "Disable All"), + image: nil, + identifier: nil, + discoverabilityTitle: nil, + attributes: [], + state: .off) { [weak self] _ in + for feed in account.flattenedWebFeeds() { + feed.isNotifyAboutNewArticles = false + } + self?.notificationsTableView.reloadData() + self?.filterButton.menu = self?.notificationFilterMenu() + } + return action } // MARK: - Feed Filtering diff --git a/iOS/Settings/Settings.storyboard b/iOS/Settings/Settings.storyboard index 1885b1634..5d8a05ff4 100644 --- a/iOS/Settings/Settings.storyboard +++ b/iOS/Settings/Settings.storyboard @@ -43,7 +43,7 @@ -