Notifications Refresh (Phase 1)

Modules/Package.swift

@@ -20,6 +20,10 @@ let package = Package(
             name: "\(jetpackStatsWidgetsCoreName)Tests",
             dependencies: [.target(name: jetpackStatsWidgetsCoreName)]
         ),
+        .testTarget(
+            name: "\(designSystemName)Tests",
+            dependencies: [.target(name: designSystemName)]
+        ),
         .target(name: designSystemName)
     ]
 )

Modules/Sources/DesignSystem/Foundation/IconName.swift

@@ -0,0 +1,32 @@
+import UIKit
+import SwiftUI
+
+/// `Icon` provides a namespace for icon identifiers.
+///
+/// The naming convention follows the SF Symbols guidelines, enhancing consistency and readability.
+/// Each icon name is a dot-syntax representation of the icon's hierarchy and style.
+///
+/// For example, `ellipsis.vertical` represents a vertical ellipsis icon.
+public enum IconName: String, CaseIterable {
+    case ellipsisHorizontal = "ellipsis.horizontal"
+    case checkmark
+    case gearshapeFill = "gearshape.fill"
+}
+
+// MARK: - Load Image
+
+public extension UIImage {
+    enum DS {
+        public static func icon(named name: IconName, with configuration: UIImage.Configuration? = nil) -> UIImage? {
+            return UIImage(named: name.rawValue, in: .module, with: configuration)
+        }
+    }
+}
+
+public extension Image {
+    enum DS {
+        public static func icon(named name: IconName) -> Image {
+            return Image(name.rawValue, bundle: .module)
+        }
+    }
+}

Modules/Sources/DesignSystem/Foundation/Icons.xcassets/Contents.json

@@ -0,0 +1,6 @@
+{
+  "info" : {
+    "author" : "xcode",
+    "version" : 1
+  }
+}

Modules/Sources/DesignSystem/Foundation/Icons.xcassets/checkmark.imageset/Contents.json

@@ -0,0 +1,16 @@
+{
+  "images" : [
+    {
+      "filename" : "checkmark.pdf",
+      "idiom" : "universal"
+    }
+  ],
+  "info" : {
+    "author" : "xcode",
+    "version" : 1
+  },
+  "properties" : {
+    "preserves-vector-representation" : true,
+    "template-rendering-intent" : "template"
+  }
+}

Modules/Sources/DesignSystem/Foundation/Icons.xcassets/ellipsis.horizontal.imageset/Contents.json

@@ -0,0 +1,16 @@
+{
+  "images" : [
+    {
+      "filename" : "more-horizontal-mobile.svg",
+      "idiom" : "universal"
+    }
+  ],
+  "info" : {
+    "author" : "xcode",
+    "version" : 1
+  },
+  "properties" : {
+    "preserves-vector-representation" : true,
+    "template-rendering-intent" : "template"
+  }
+}

Modules/Sources/DesignSystem/Foundation/Icons.xcassets/gearshape.fill.imageset/Contents.json

@@ -0,0 +1,16 @@
+{
+  "images" : [
+    {
+      "filename" : "cog.pdf",
+      "idiom" : "universal"
+    }
+  ],
+  "info" : {
+    "author" : "xcode",
+    "version" : 1
+  },
+  "properties" : {
+    "preserves-vector-representation" : true,
+    "template-rendering-intent" : "template"
+  }
+}

Modules/Tests/DesignSystemTests/IconTests.swift

@@ -0,0 +1,12 @@
+import XCTest
+import DesignSystem
+import SwiftUI
+
+final class IconTests: XCTestCase {
+
+    func testCanLoadAllIconsAsUIImage() throws {
+        for icon in IconName.allCases {
+            let _ = try XCTUnwrap(UIImage.DS.icon(named: icon))
+        }
+    }
+}

WordPress/Classes/Services/NotificationSyncMediator.swift

@@ -150,43 +150,59 @@ final class NotificationSyncMediator: NotificationSyncMediatorProtocol {
 
     /// Marks a Notification as Read.
     ///
-    /// - Note: This method should only be used on the main thread.
+    /// - Note: This method is called on the main thread.
     ///
     /// - Parameters:
     ///     - notification: The notification that was just read.
     ///     - completion: Callback to be executed on completion.
     ///
     func markAsRead(_ notification: Notification, completion: ((Error?)-> Void)? = nil) {
-        mark([notification], asRead: true, completion: completion)
+        Task { @MainActor in
+            mark([notification], asRead: true, completion: completion)
+        }
     }
 
     /// Marks an array of notifications as Read.
     ///
-    /// - Note: This method should only be used on the main thread.
+    /// - Note: This method is called on the main thread.
     ///
     /// - Parameters:
     ///     - notifications: Notifications that were marked as read.
     ///     - completion: Callback to be executed on completion.
     ///
     func markAsRead(_ notifications: [Notification], completion: ((Error?)-> Void)? = nil) {
-        mark(notifications, asRead: true, completion: completion)
+        Task { @MainActor in
+            mark(notifications, asRead: true, completion: completion)
+        }
     }
 
     /// Marks a Notification as Unead.
     ///
-    /// - Note: This method should only be used on the main thread.
+    /// - Note: This method is called on the main thread.
     ///
     /// - Parameters:
     ///     - notification: The notification that should be marked unread.
     ///     - completion: Callback to be executed on completion.
     ///
     func markAsUnread(_ notification: Notification, completion: ((Error?)-> Void)? = nil) {
-        mark([notification], asRead: false, completion: completion)
+        markAsUnread([notification], completion: completion)
     }
 
-    private func mark(_ notifications: [Notification], asRead read: Bool = true, completion: ((Error?)-> Void)? = nil) {
-        assert(Thread.isMainThread)
+    /// Marks a Notification as Unread.
+    ///
+    /// - Note: This method is called on the main thread.
+    ///
+    /// - Parameters:
+    ///     - notifications: The notifications that should be marked unread.
+    ///     - completion: Callback to be executed on completion.
+    ///
+    func markAsUnread(_ notifications: [Notification], completion: ((Error?)-> Void)? = nil) {
+        Task { @MainActor in
+            mark(notifications, asRead: false, completion: completion)
+        }
+    }
 
+    @MainActor private func mark(_ notifications: [Notification], asRead read: Bool = true, completion: ((Error?)-> Void)? = nil) {
         let noteIDs = notifications.map {
             $0.notificationId
         }

WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift

@@ -331,6 +331,7 @@ import Foundation
     case notificationsMarkAllReadTapped
     case notificationMarkAsReadTapped
     case notificationMarkAsUnreadTapped
+    case notificationMenuTapped
 
     // Sharing Buttons
     case sharingButtonsEditSharingButtonsToggled
@@ -1142,6 +1143,8 @@ import Foundation
             return "notification_mark_as_read_tapped"
         case .notificationMarkAsUnreadTapped:
             return "notification_mark_as_unread_tapped"
+        case .notificationMenuTapped:
+            return "notification_menu_tapped"
 
         // Sharing
         case .sharingButtonsEditSharingButtonsToggled:

WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController/NotificationsViewController+Strings.swift

@@ -0,0 +1,24 @@
+import Foundation
+
+extension NotificationsViewController {
+
+    enum Strings {
+        enum NavigationBar {
+            static let notificationSettingsActionTitle = NSLocalizedString(
+                "notificationsViewController.navigationBar.action.settings",
+                value: "Notification Settings",
+                comment: "Link to Notification Settings section"
+            )
+            static let markAllAsReadActionTitle = NSLocalizedString(
+                "notificationsViewController.navigationBar.action.markAllAsRead",
+                value: "Mark All As Read",
+                comment: "Marks all notifications under the filter as read"
+            )
+            static let menuButtonAccessibilityLabel = NSLocalizedString(
+                "notificationsViewController.navigationBar.menu.accessibilityLabel",
+                value: "Navigation Bar Menu Button",
+                comment: "Accessibility label for the navigation bar menu button"
+            )
+        }
+    }
+}

WordPress/Classes/ViewRelated/Notifications/Controllers/NotificationsViewController/NotificationsViewController.swift

@@ -103,36 +103,6 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
         return indicator
     }()
 
-    /// Notification Settings button
-    private lazy var settingsBarButtonItem: UIBarButtonItem = {
-        let settingsButton = UIBarButtonItem(
-            image: .gridicon(.cog),
-            style: .plain,
-            target: self,
-            action: #selector(showNotificationSettings)
-        )
-        settingsButton.accessibilityLabel = NSLocalizedString(
-            "Notification Settings",
-            comment: "Link to Notification Settings section"
-        )
-        return settingsButton
-    }()
-
-    /// Mark All As Read button
-    private lazy var markAllAsReadBarButtonItem: UIBarButtonItem = {
-        let markButton = UIBarButtonItem(
-            image: .gridicon(.checkmark),
-            style: .plain,
-            target: self,
-            action: #selector(showMarkAllAsReadConfirmation)
-        )
-        markButton.accessibilityLabel = NSLocalizedString(
-            "Mark All As Read",
-            comment: "Marks all notifications under the filter as read"
-        )
-        return markButton
-    }()
-
     /// Used by JPScrollViewDelegate to send scroll position
     internal let scrollViewTranslationPublisher = PassthroughSubject<Bool, Never>()
 
@@ -199,7 +169,6 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
 
         // Refresh the UI
         reloadResultsControllerIfNeeded()
-        updateMarkAllAsReadButton()
 
         if !splitViewControllerIsHorizontallyCompact {
             reloadTableViewPreservingSelection()
@@ -528,6 +497,7 @@ class NotificationsViewController: UIViewController, UIViewControllerRestoration
 // MARK: - User Interface Initialization
 //
 private extension NotificationsViewController {
+
     func setupNavigationBar() {
         navigationController?.navigationBar.prefersLargeTitles = false
         navigationItem.largeTitleDisplayMode = .never
@@ -540,15 +510,57 @@ private extension NotificationsViewController {
     }
 
     func updateNavigationItems() {
-        var barItems: [UIBarButtonItem] = []
+        let moreMenuItems = UIDeferredMenuElement.uncached { [weak self] completion in
+            guard let self else {
+                completion([])
+                return
+            }
+            WPAnalytics.track(.notificationMenuTapped)
+            completion(self.makeMoreMenuElements())
+        }
+        self.navigationItem.rightBarButtonItem = {
+            let menu = UIMenu(children: [moreMenuItems])
+            let button = UIBarButtonItem(
+                image: UIImage.DS.icon(named: .ellipsisHorizontal),
+                menu: menu
+            )
+            button.accessibilityLabel = Strings.NavigationBar.menuButtonAccessibilityLabel
+            return button
+        }()
+    }
 
-        if shouldDisplaySettingsButton {
-            barItems.append(settingsBarButtonItem)
-        }
+    func makeMoreMenuElements() -> [UIAction] {
+        // Mark All As Read
+        let markAllAsRead: UIAction? = { () -> UIAction? in
+            guard let notes = tableViewHandler.resultsController?.fetchedObjects as? [Notification] else {
+                return nil
+            }
+            let isEnabled = notes.first { !$0.read } != nil
+            let attributes = isEnabled ? UIAction.Attributes(rawValue: 0) : .disabled
+            return UIAction(
+                title: Strings.NavigationBar.markAllAsReadActionTitle,
+                image: .DS.icon(named: .checkmark),
+                attributes: attributes
+            ) { [weak self] _ in
+                self?.showMarkAllAsReadConfirmation()
+            }
+        }()
 
-        barItems.append(markAllAsReadBarButtonItem)
+        // Notifications Settings
+        let settings: UIAction? = { () -> UIAction? in
+            guard shouldDisplaySettingsButton else {
+                return nil
+            }
+            return UIAction(
+                title: Strings.NavigationBar.notificationSettingsActionTitle,
+                image: .DS.icon(named: .gearshapeFill)
+            ) { [weak self] _ in
+                self?.showNotificationSettings()
+            }
+        }()
 
-        navigationItem.setRightBarButtonItems(barItems, animated: false)
+        // Return
+        return [markAllAsRead, settings].compactMap { $0 }
     }
 
     @objc func closeNotificationSettings() {
@@ -1074,12 +1086,11 @@ private extension NotificationsViewController {
             !$0.read
         }
 
-        NotificationSyncMediator()?.markAsRead(unreadNotifications, completion: { [weak self] error in
+        NotificationSyncMediator()?.markAsRead(unreadNotifications, completion: { error in
             let notice = Notice(
                 title: error != nil ? Localization.markAllAsReadNoticeFailure : Localization.markAllAsReadNoticeSuccess
             )
             ActionDispatcherFacade().dispatch(NoticeAction.post(notice))
-            self?.updateMarkAllAsReadButton()
         })
     }
 
@@ -1132,7 +1143,6 @@ private extension NotificationsViewController {
         }
 
         NotificationSyncMediator()?.markAsUnread(note)
-        updateMarkAllAsReadButton()
     }
 
     func markWelcomeNotificationAsSeenIfNeeded() {
@@ -1142,17 +1152,6 @@ private extension NotificationsViewController {
             resetApplicationBadge()
         }
     }
-
-    func updateMarkAllAsReadButton() {
-        guard let notes = tableViewHandler.resultsController?.fetchedObjects as? [Notification] else {
-            return
-        }
-
-        let isEnabled = notes.first { !$0.read } != nil
-
-        markAllAsReadBarButtonItem.tintColor = isEnabled ? .primary : .textTertiary
-        markAllAsReadBarButtonItem.isEnabled = isEnabled
-    }
 }
 
 // MARK: - Unread notifications caching
@@ -1221,7 +1220,6 @@ private extension NotificationsViewController {
         // Don't overwork!
         lastReloadDate = Date()
         needsReloadResults = false
-        updateMarkAllAsReadButton()
     }
 
     func reloadRowForNotificationWithID(_ noteObjectID: NSManagedObjectID) {