From e727a560e0bee05ee2f655432fdf5b350e5c76e8 Mon Sep 17 00:00:00 2001 From: Carlos Enumo Date: Sun, 6 Nov 2022 16:09:27 +0000 Subject: [PATCH] Create new menu for settings and calendar picker --- Calendr/Assets/Accessibility.swift | 1 - .../Assets/Generated/Strings.generated.swift | 2 + Calendr/Assets/Icons.swift | 1 - Calendr/Assets/de.lproj/Localizable.strings | 1 + Calendr/Assets/en.lproj/Localizable.strings | 1 + Calendr/Assets/es.lproj/Localizable.strings | 1 + Calendr/Assets/fr.lproj/Localizable.strings | 1 + Calendr/Assets/pt.lproj/Localizable.strings | 1 + Calendr/Main/MainViewController.swift | 69 +++++++-------- .../CalendarPickerViewController.swift | 84 ++++++++----------- .../Settings/CalendarPickerViewModel.swift | 7 +- Calendr/Settings/SettingsViewController.swift | 4 +- .../CalendarPickerViewModelTests.swift | 3 +- CalendrUITests/MainViewTests.swift | 17 +++- CalendrUITests/UITestCase+Queries.swift | 11 ++- 15 files changed, 103 insertions(+), 101 deletions(-) diff --git a/Calendr/Assets/Accessibility.swift b/Calendr/Assets/Accessibility.swift index b86bf36e..137cc2aa 100644 --- a/Calendr/Assets/Accessibility.swift +++ b/Calendr/Assets/Accessibility.swift @@ -19,7 +19,6 @@ enum Accessibility { static let calendarBtn = "main_calendar_button" static let settingsBtn = "main_settings_button" static let pinBtn = "main_pin_button" - static let pickerBtn = "main_picker_button" } enum MenuBar { diff --git a/Calendr/Assets/Generated/Strings.generated.swift b/Calendr/Assets/Generated/Strings.generated.swift index 02c9f3b3..815c8e64 100644 --- a/Calendr/Assets/Generated/Strings.generated.swift +++ b/Calendr/Assets/Generated/Strings.generated.swift @@ -84,6 +84,8 @@ internal enum Strings { internal static let events = Strings.tr("Localizable", "settings.events", fallback: "Events") /// Menu Bar internal static let menuBar = Strings.tr("Localizable", "settings.menu_bar", fallback: "Menu Bar") + /// Preferences + internal static let title = Strings.tr("Localizable", "settings.title", fallback: "Preferences") /// Transparency internal static let transparency = Strings.tr("Localizable", "settings.transparency", fallback: "Transparency") internal enum Calendar { diff --git a/Calendr/Assets/Icons.swift b/Calendr/Assets/Icons.swift index 6ebab7ba..8e7dcaa9 100644 --- a/Calendr/Assets/Icons.swift +++ b/Calendr/Assets/Icons.swift @@ -17,7 +17,6 @@ enum Icons { static let prev = NSImage(systemName: "chevron.backward") static let reset = NSImage(systemName: "circle") static let next = NSImage(systemName: "chevron.forward") - static let picker = NSImage(systemName: "checkmark.square") static let reminders = NSImage(systemName: "list.bullet") static let calendar = NSImage(systemName: "calendar") static let settings = NSImage(systemName: "ellipsis.circle") diff --git a/Calendr/Assets/de.lproj/Localizable.strings b/Calendr/Assets/de.lproj/Localizable.strings index bf3e68d2..0e90f422 100644 --- a/Calendr/Assets/de.lproj/Localizable.strings +++ b/Calendr/Assets/de.lproj/Localizable.strings @@ -8,6 +8,7 @@ "quit" = "Programm beenden"; +"settings.title" = "Einstellungen"; "settings.tab.general" = "Allgemein"; "settings.tab.calendars" = "Kalender"; "settings.tab.about" = "Über"; diff --git a/Calendr/Assets/en.lproj/Localizable.strings b/Calendr/Assets/en.lproj/Localizable.strings index 576a4dd1..7a04ba75 100644 --- a/Calendr/Assets/en.lproj/Localizable.strings +++ b/Calendr/Assets/en.lproj/Localizable.strings @@ -8,6 +8,7 @@ "quit" = "Quit"; +"settings.title" = "Preferences"; "settings.tab.general" = "General"; "settings.tab.calendars" = "Calendars"; "settings.tab.about" = "About"; diff --git a/Calendr/Assets/es.lproj/Localizable.strings b/Calendr/Assets/es.lproj/Localizable.strings index f68dc1db..e1c7680a 100644 --- a/Calendr/Assets/es.lproj/Localizable.strings +++ b/Calendr/Assets/es.lproj/Localizable.strings @@ -8,6 +8,7 @@ "quit" = "Salir"; +"settings.title" = "Preferencias"; "settings.tab.general" = "General"; "settings.tab.calendars" = "Calendarios"; "settings.tab.about" = "Acerca de"; diff --git a/Calendr/Assets/fr.lproj/Localizable.strings b/Calendr/Assets/fr.lproj/Localizable.strings index 40ee51e3..9a9bff02 100644 --- a/Calendr/Assets/fr.lproj/Localizable.strings +++ b/Calendr/Assets/fr.lproj/Localizable.strings @@ -8,6 +8,7 @@ "quit" = "Quitter"; +"settings.title" = "Préférences"; "settings.tab.general" = "Général"; "settings.tab.calendars" = "Calendriers"; "settings.tab.about" = "À propos"; diff --git a/Calendr/Assets/pt.lproj/Localizable.strings b/Calendr/Assets/pt.lproj/Localizable.strings index cfc3d32e..cedf2475 100644 --- a/Calendr/Assets/pt.lproj/Localizable.strings +++ b/Calendr/Assets/pt.lproj/Localizable.strings @@ -8,6 +8,7 @@ "quit" = "Sair"; +"settings.title" = "Preferências"; "settings.tab.general" = "Geral"; "settings.tab.calendars" = "Calendários"; "settings.tab.about" = "Sobre"; diff --git a/Calendr/Main/MainViewController.swift b/Calendr/Main/MainViewController.swift index 5ecb9705..d2ec3582 100644 --- a/Calendr/Main/MainViewController.swift +++ b/Calendr/Main/MainViewController.swift @@ -27,7 +27,6 @@ class MainViewController: NSViewController { private let pinBtn = NSButton() private let remindersBtn = NSButton() private let calendarBtn = NSButton() - private let pickerBtn = NSButton() private let settingsBtn = NSButton() // ViewModels @@ -36,7 +35,6 @@ class MainViewController: NSViewController { private let statusItemViewModel: StatusItemViewModel private let nextEventViewModel: NextEventViewModel private let calendarPickerViewModel: CalendarPickerViewModel - private let calendarsViewModel: CalendarPickerViewModel private let eventListViewModel: EventListViewModel // Reactive @@ -96,19 +94,12 @@ class MainViewController: NSViewController { calendarPickerViewModel = CalendarPickerViewModel( calendarService: calendarService, - userDefaults: userDefaults, - popoverSettings: settingsViewModel - ) - - calendarsViewModel = CalendarPickerViewModel( - calendarService: calendarService, - userDefaults: userDefaults, - popoverSettings: nil + userDefaults: userDefaults ) settingsViewController = SettingsViewController( settingsViewModel: settingsViewModel, - calendarsViewModel: calendarsViewModel, + calendarsViewModel: calendarPickerViewModel, notificationCenter: notificationCenter ) @@ -176,6 +167,8 @@ class MainViewController: NSViewController { setUpPopover() + setUpSettings() + setUpMainStatusItem() setUpEventStatusItem() @@ -276,7 +269,6 @@ class MainViewController: NSViewController { pinBtn.setAccessibilityIdentifier(Accessibility.Main.pinBtn) remindersBtn.setAccessibilityIdentifier(Accessibility.Main.remindersBtn) calendarBtn.setAccessibilityIdentifier(Accessibility.Main.calendarBtn) - pickerBtn.setAccessibilityIdentifier(Accessibility.Main.pickerBtn) settingsBtn.setAccessibilityIdentifier(Accessibility.Main.settingsBtn) } @@ -318,31 +310,41 @@ class MainViewController: NSViewController { } } .disposed(by: disposeBag) + } - pickerBtn.rx.tap - .flatMapFirst { [pickerBtn, calendarPickerViewModel] _ -> Observable in - let vc = CalendarPickerViewController(viewModel: calendarPickerViewModel) - let popover = NSPopover() - popover.behavior = .transient - popover.contentViewController = vc - popover.delegate = vc - popover.show(relativeTo: .zero, of: pickerBtn, preferredEdge: .maxX) - return popover.rx.deallocated - } - .bind { [weak self] in - // 🔨 Allow clicking outside to dismiss the main view after dismissing the calendar picker - if NSApp.keyWindow == nil { - self?.view.window?.makeKey() - } - } - .disposed(by: disposeBag) + private func setUpSettings() { + + let settingsMenu = NSMenu() + + settingsMenu.addItem(withTitle: Strings.Settings.title, action: #selector(openSettings), keyEquivalent: "") + + settingsMenu.addItem(.separator()) + + let pickerMenuItem = settingsMenu.addItem(withTitle: Strings.Settings.Tab.calendars, action: nil, keyEquivalent: "") + let pickerSubmenu = NSMenu() + let pickerSubmenuItem = NSMenuItem() + pickerSubmenu.addItem(pickerSubmenuItem) + pickerMenuItem.submenu = pickerSubmenu - settingsBtn.rx.tap.bind { [weak self, settingsViewController] in - self?.presentAsModalWindow(settingsViewController) + let pickerViewController = CalendarPickerViewController(viewModel: calendarPickerViewModel, configuration: .picker) + pickerSubmenuItem.view = pickerViewController.view.forAutoLayout() + addChild(pickerViewController) + + settingsMenu.addItem(.separator()) + + settingsMenu.addItem(withTitle: Strings.quit, action: #selector(NSApp.terminate), keyEquivalent: "q") + + settingsBtn.rx.tap.bind { [settingsBtn] in + settingsMenu.popUp(positioning: nil, at: .init(x: 0, y: settingsBtn.frame.height), in: settingsBtn) } .disposed(by: disposeBag) } + @objc private func openSettings() { + + presentAsModalWindow(settingsViewController) + } + private func setUpPopover() { popover.rx.observe(\.isShown) @@ -499,7 +501,7 @@ class MainViewController: NSViewController { private func makeToolBar() -> NSView { - [pinBtn, remindersBtn, calendarBtn, pickerBtn, settingsBtn].forEach(styleButton) + [pinBtn, remindersBtn, calendarBtn, settingsBtn].forEach(styleButton) pinBtn.setButtonType(.toggle) pinBtn.image = Icons.Calendar.unpinned @@ -507,10 +509,9 @@ class MainViewController: NSViewController { remindersBtn.image = Icons.Calendar.reminders.with(scale: .large) calendarBtn.image = Icons.Calendar.calendar.with(scale: .large) - pickerBtn.image = Icons.Calendar.picker.with(scale: .large) settingsBtn.image = Icons.Calendar.settings.with(scale: .large) - return NSStackView(views: [pinBtn, .spacer, remindersBtn, calendarBtn, pickerBtn, settingsBtn]) + return NSStackView(views: [pinBtn, .spacer, remindersBtn, calendarBtn, settingsBtn]) } private func makeDateSelector() -> DateSelector { diff --git a/Calendr/Settings/CalendarPickerViewController.swift b/Calendr/Settings/CalendarPickerViewController.swift index 90508c76..d841c721 100644 --- a/Calendr/Settings/CalendarPickerViewController.swift +++ b/Calendr/Settings/CalendarPickerViewController.swift @@ -8,7 +8,12 @@ import Cocoa import RxSwift -class CalendarPickerViewController: NSViewController, NSPopoverDelegate { +enum CalendarPickerConfiguration { + case settings + case picker +} + +class CalendarPickerViewController: NSViewController { private let disposeBag = DisposeBag() @@ -16,9 +21,12 @@ class CalendarPickerViewController: NSViewController, NSPopoverDelegate { private let contentStackView = NSStackView(.vertical) - init(viewModel: CalendarPickerViewModel) { + private let configuration: CalendarPickerConfiguration + + init(viewModel: CalendarPickerViewModel, configuration: CalendarPickerConfiguration) { self.viewModel = viewModel + self.configuration = configuration super.init(nibName: nil, bundle: nil) @@ -32,11 +40,7 @@ class CalendarPickerViewController: NSViewController, NSPopoverDelegate { guard BuildConfig.isUITesting else { return } view.setAccessibilityElement(true) - view.setAccessibilityIdentifier( - viewModel.isPopover - ? Accessibility.CalendarPicker.view - : Accessibility.Settings.Calendars.view - ) + view.setAccessibilityIdentifier(configuration.accessibilityIdentifier) } override func loadView() { @@ -47,9 +51,7 @@ class CalendarPickerViewController: NSViewController, NSPopoverDelegate { view.addSubview(scrollView) - let insets: NSEdgeInsets = viewModel.isPopover ? .init(top: 16, left: 16, bottom: 16, right: 20) : .init() - - scrollView.edges(to: view, insets: insets) + scrollView.edges(to: view, insets: configuration.insets) scrollView.drawsBackground = false scrollView.documentView = contentStackView.forAutoLayout() @@ -58,29 +60,18 @@ class CalendarPickerViewController: NSViewController, NSPopoverDelegate { scrollView.contentView.top(equalTo: contentStackView) scrollView.contentView.leading(equalTo: contentStackView) scrollView.contentView.trailing(equalTo: contentStackView) - scrollView.contentView.height(equalTo: contentStackView).priority = .dragThatCanResizeWindow - scrollView.contentView.heightAnchor.constraint( - lessThanOrEqualToConstant: viewModel.isPopover ? 0.8 * NSScreen.main!.visibleFrame.height : 600 - ).activate() + let height = scrollView.contentView.height(equalTo: contentStackView) + + if configuration ~= .settings { + height.priority = .dragThatCanResizeWindow + scrollView.contentView.heightAnchor.constraint(lessThanOrEqualToConstant: 600).activate() + } contentStackView.alignment = .left } private func setUpBindings() { - if let popoverSettings = viewModel.popoverSettings { - - let popoverView = view.rx.observe(\.superview) - .compactMap { $0 as? NSVisualEffectView } - .take(1) - - Observable.combineLatest( - popoverView, popoverSettings.popoverMaterial - ) - .bind { $0.material = $1 } - .disposed(by: disposeBag) - } - viewModel.calendars .observe(on: MainScheduler.instance) .compactMap { [weak self] calendars -> [NSView]? in @@ -131,31 +122,30 @@ class CalendarPickerViewController: NSViewController, NSPopoverDelegate { return checkbox } - func popoverDidShow(_ notification: Notification) { - // 🔨 Allow dismiss with the escape key - view.window?.makeKey() - view.window?.makeFirstResponder(self) + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") } +} - private var popover: NSPopover? +// MARK: - Extensions - func popoverWillClose(_ notification: Notification) { - // 🔨 Prevent retain cycle - view.window?.makeFirstResponder(nil) - } +extension CalendarPickerConfiguration { - func popoverShouldClose(_ popover: NSPopover) -> Bool { - self.popover = popover - return true - } - - func popoverDidClose(_ notification: Notification) { - // 🔨 Prevent retain cycle - popover?.contentViewController = nil - popover = nil + var accessibilityIdentifier: String { + switch self { + case .settings: + return Accessibility.Settings.Calendars.view + case .picker: + return Accessibility.CalendarPicker.view + } } - required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") + var insets: NSEdgeInsets { + switch self { + case .settings: + return .init() + case .picker: + return .init(top: 16, left: 16, bottom: 16, right: 20) + } } } diff --git a/Calendr/Settings/CalendarPickerViewModel.swift b/Calendr/Settings/CalendarPickerViewModel.swift index 2f4fb555..8d52783b 100644 --- a/Calendr/Settings/CalendarPickerViewModel.swift +++ b/Calendr/Settings/CalendarPickerViewModel.swift @@ -17,9 +17,6 @@ class CalendarPickerViewModel { // Observables let calendars: Observable<[CalendarModel]> let enabledCalendars: Observable<[String]> - let popoverSettings: PopoverSettings? - - var isPopover: Bool { popoverSettings != nil } private let userDefaults: UserDefaults private let toggleCalendarSubject = PublishRelay() @@ -27,8 +24,7 @@ class CalendarPickerViewModel { init( calendarService: CalendarServiceProviding, - userDefaults: UserDefaults, - popoverSettings: PopoverSettings? + userDefaults: UserDefaults ) { self.calendars = calendarService.changeObservable @@ -48,7 +44,6 @@ class CalendarPickerViewModel { .share(replay: 1) self.userDefaults = userDefaults - self.popoverSettings = popoverSettings setUpBindings() } diff --git a/Calendr/Settings/SettingsViewController.swift b/Calendr/Settings/SettingsViewController.swift index 9725a120..b1f160ce 100644 --- a/Calendr/Settings/SettingsViewController.swift +++ b/Calendr/Settings/SettingsViewController.swift @@ -28,7 +28,9 @@ class SettingsViewController: NSTabViewController { tabStyle = .toolbar let general = NSTabViewItem(viewController: GeneralSettingsViewController(viewModel: settingsViewModel)) - let calendars = NSTabViewItem(viewController: CalendarPickerViewController(viewModel: calendarsViewModel)) + let calendars = NSTabViewItem( + viewController: CalendarPickerViewController(viewModel: calendarsViewModel, configuration: .settings) + ) let about = NSTabViewItem(viewController: AboutViewController()) general.label = Strings.Settings.Tab.general diff --git a/CalendrTests/CalendarPickerViewModelTests.swift b/CalendrTests/CalendarPickerViewModelTests.swift index 54295924..212a292a 100644 --- a/CalendrTests/CalendarPickerViewModelTests.swift +++ b/CalendrTests/CalendarPickerViewModelTests.swift @@ -18,8 +18,7 @@ class CalendarPickerViewModelTests: XCTestCase { lazy var viewModel = CalendarPickerViewModel( calendarService: calendarService, - userDefaults: userDefaults, - popoverSettings: nil + userDefaults: userDefaults ) override func setUp() { diff --git a/CalendrUITests/MainViewTests.swift b/CalendrUITests/MainViewTests.swift index d3452650..a76aa212 100644 --- a/CalendrUITests/MainViewTests.swift +++ b/CalendrUITests/MainViewTests.swift @@ -85,10 +85,11 @@ class MainViewTests: UITestCase { calendar.terminate() } - func testPickerButtonClicked_shouldOpenCalendarPicker() { + func testSettingsMenu_withPickerButtonHovered_shouldOpenCalendarPicker() { MenuBar.main.click() - Main.pickerBtn.click() + Main.settingsBtn.click() + Main.settingsBtn.menuItems.element(matching: NSPredicate(format: "title = %@", "Calendars")).hover() XCTAssertTrue(CalendarPicker.view.didAppear) @@ -97,10 +98,11 @@ class MainViewTests: UITestCase { XCTAssertFalse(CalendarPicker.view.exists) } - func testSettingsButtonClicked_shouldOpenSettings() { + func testSettingsMenu_withPreferencesButtonClicked_shouldOpenSettings() { MenuBar.main.click() Main.settingsBtn.click() + Main.settingsBtn.menuItems.element(matching: NSPredicate(format: "title = %@", "Preferences")).click() XCTAssertTrue(Settings.window.didAppear) XCTAssertTrue(Settings.view.didAppear) @@ -109,4 +111,13 @@ class MainViewTests: UITestCase { XCTAssertFalse(Settings.window.exists) } + + func testSettingsMenu_withQuitButtonClicked_shouldCloseApp() { + + MenuBar.main.click() + Main.settingsBtn.click() + Main.settingsBtn.menuItems.element(matching: NSPredicate(format: "title = %@", "Quit")).click() + + XCTAssert(app.wait(for: .notRunning, timeout: 1)) + } } diff --git a/CalendrUITests/UITestCase+Queries.swift b/CalendrUITests/UITestCase+Queries.swift index 4288f419..c0d895e4 100644 --- a/CalendrUITests/UITestCase+Queries.swift +++ b/CalendrUITests/UITestCase+Queries.swift @@ -12,6 +12,11 @@ extension UITestCase { static let app = XCUIApplication() var app: XCUIApplication { Self.app } + enum MenuBar { + static var main: XCUIElement { app.statusItems[Accessibility.MenuBar.main].wait(.shortTimeout) } + static var event: XCUIElement { app.statusItems[Accessibility.MenuBar.event] } + } + enum Main { static var view: XCUIElement { app.otherElements[Accessibility.Main.view] } @@ -22,15 +27,9 @@ extension UITestCase { static var pinBtn: XCUIElement { view.checkBoxes[Accessibility.Main.pinBtn] } static var remindersBtn: XCUIElement { view.buttons[Accessibility.Main.remindersBtn] } static var calendarBtn: XCUIElement { view.buttons[Accessibility.Main.calendarBtn] } - static var pickerBtn: XCUIElement { view.buttons[Accessibility.Main.pickerBtn] } static var settingsBtn: XCUIElement { view.buttons[Accessibility.Main.settingsBtn] } } - enum MenuBar { - static var main: XCUIElement { app.statusItems[Accessibility.MenuBar.main].wait(.shortTimeout) } - static var event: XCUIElement { app.statusItems[Accessibility.MenuBar.event] } - } - enum Calendar { static var view: XCUIElement { Main.view.otherElements[Accessibility.Calendar.view] }