Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix discarded quick start notification observers #20997

Merged
merged 9 commits into from
Jul 5, 2023
Merged

Fix discarded quick start notification observers #20997

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
254f61a
Fix discarded quick start notification observers
crazytonyli Jun 30, 2023
97bbe96
Add a release note
crazytonyli Jul 3, 2023
2ea3f35
Change DashboardQuickActionsCardCell to use self as the quick start o…
crazytonyli Jul 5, 2023
61724d8
Change NewQuickStartChecklistView to use self as the quick start obse…
crazytonyli Jul 5, 2023
52fdf44
Change QuickStartChecklistView to use self as the quick start observer
crazytonyli Jul 5, 2023
4d92db8
Change MySiteViewController to use self as the quick start observer
crazytonyli Jul 5, 2023
8fd6993
Change SitePickerViewController to use self as the quick start observer
crazytonyli Jul 5, 2023
3ee06ad
Update release note
crazytonyli Jul 5, 2023
accb98d
Merge branch 'trunk' into fix-memory-leak-in-notification-observers-3
crazytonyli Jul 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* [*] Blogging Reminders: Disabled prompt for self-hosted sites not connected to Jetpack. [#20970]
* [**] [internal] Do not save synced blogs if the app has signed out. [#20959]
* [**] [internal] Make sure synced posts are saved before calling completion block. [#20960]
* [**] [internal] Fix observing Quick Start notifications. [#20997]


22.7
Expand Down
43 changes: 20 additions & 23 deletions WordPress/Classes/ViewRelated/Blog/Blog Dashboard/BlogDashboardViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,30 +159,27 @@ final class BlogDashboardViewController: UIViewController {
}

private func addQuickStartObserver() {
NotificationCenter.default.addObserver(forName: .QuickStartTourElementChangedNotification, object: nil, queue: nil) { [weak self] notification in

guard let self = self else {
return
}

if let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {

switch element {
case .setupQuickStart:
self.loadCardsFromCache()
self.displayQuickStart()
case .updateQuickStart:
self.loadCardsFromCache()
case .stats, .mediaScreen:
if self.embeddedInScrollView {
self.mySiteScrollView?.scrollToTop(animated: true)
} else {
self.collectionView.scrollToTop(animated: true)
}
default:
break
NotificationCenter.default.addObserver(self, selector: #selector(handleQuickStartTourElementChangedNotification(_:)), name: .QuickStartTourElementChangedNotification, object: nil)
}

@objc private func handleQuickStartTourElementChangedNotification(_ notification: Foundation.Notification) {
if let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {

switch element {
case .setupQuickStart:
self.loadCardsFromCache()
self.displayQuickStart()
case .updateQuickStart:
self.loadCardsFromCache()
case .stats, .mediaScreen:
if self.embeddedInScrollView {
self.mySiteScrollView?.scrollToTop(animated: true)
} else {
self.collectionView.scrollToTop(animated: true)
}
default:
break
}
}
}
Expand Down
57 changes: 26 additions & 31 deletions ...s/ViewRelated/Blog/Blog Dashboard/Cards/Quick Actions/DashboardQuickActionsCardCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ final class DashboardQuickActionsCardCell: UICollectionViewCell, Reusable {
required init?(coder: NSCoder) {
fatalError("Not implemented")
}

deinit {
stopObservingQuickStart()
}
}

// MARK: - Button Actions
Expand Down Expand Up @@ -142,35 +138,34 @@ extension DashboardQuickActionsCardCell {
}

private func startObservingQuickStart() {
NotificationCenter.default.addObserver(forName: .QuickStartTourElementChangedNotification, object: nil, queue: nil) { [weak self] notification in

if let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {

switch element {
case .stats:
guard QuickStartTourGuide.shared.entryPointForCurrentTour == .blogDashboard else {
return
}

self?.autoScrollToStatsButton()
case .mediaScreen:
guard QuickStartTourGuide.shared.entryPointForCurrentTour == .blogDashboard else {
return
}

self?.autoScrollToMediaButton()
default:
break
}
self?.statsButton.shouldShowSpotlight = element == .stats
self?.mediaButton.shouldShowSpotlight = element == .mediaScreen
}
}
NotificationCenter.default.addObserver(self, selector: #selector(handleQuickStartTourElementChangedNotification(_:)), name: .QuickStartTourElementChangedNotification, object: nil)
}

private func stopObservingQuickStart() {
NotificationCenter.default.removeObserver(self)
@objc private func handleQuickStartTourElementChangedNotification(_ notification: Foundation.Notification) {
guard let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement
else {
return
}

switch element {
case .stats:
guard QuickStartTourGuide.shared.entryPointForCurrentTour == .blogDashboard else {
return
}

autoScrollToStatsButton()
case .mediaScreen:
guard QuickStartTourGuide.shared.entryPointForCurrentTour == .blogDashboard else {
return
}

autoScrollToMediaButton()
default:
break
}
statsButton.shouldShowSpotlight = element == .stats
mediaButton.shouldShowSpotlight = element == .mediaScreen
}

private func autoScrollToStatsButton() {
Expand Down
24 changes: 9 additions & 15 deletions ...lasses/ViewRelated/Blog/Blog Dashboard/Cards/Quick Start/NewQuickStartChecklistView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,6 @@ final class NewQuickStartChecklistView: UIView, QuickStartChecklistConfigurable
fatalError("Not implemented")
}

deinit {
stopObservingQuickStart()
}

// MARK: - Trait Collection

override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
Expand Down Expand Up @@ -197,20 +193,18 @@ extension NewQuickStartChecklistView {
}

private func startObservingQuickStart() {
NotificationCenter.default.addObserver(forName: .QuickStartTourElementChangedNotification, object: nil, queue: nil) { [weak self] notification in

guard let userInfo = notification.userInfo,
let element = userInfo[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement,
element == .tourCompleted else {
return
}
NotificationCenter.default.addObserver(self, selector: #selector(handleQuickStartTourElementChangedNotification(_:)), name: .QuickStartTourElementChangedNotification, object: nil)
}

self?.updateViews()
@objc private func handleQuickStartTourElementChangedNotification(_ notification: Foundation.Notification) {
guard let userInfo = notification.userInfo,
let element = userInfo[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement,
element == .tourCompleted
else {
return
}
}

private func stopObservingQuickStart() {
NotificationCenter.default.removeObserver(self)
updateViews()
}

@objc private func didTap() {
Expand Down
24 changes: 9 additions & 15 deletions ...s/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Quick Start/QuickStartChecklistView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,6 @@ final class QuickStartChecklistView: UIView, QuickStartChecklistConfigurable {
fatalError("Not implemented")
}

deinit {
stopObservingQuickStart()
}

func configure(collection: QuickStartToursCollection, blog: Blog) {
self.tours = collection.tours
self.blog = blog
Expand Down Expand Up @@ -134,20 +130,18 @@ extension QuickStartChecklistView {
}

private func startObservingQuickStart() {
NotificationCenter.default.addObserver(forName: .QuickStartTourElementChangedNotification, object: nil, queue: nil) { [weak self] notification in

guard let userInfo = notification.userInfo,
let element = userInfo[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement,
element == .tourCompleted else {
return
}
NotificationCenter.default.addObserver(self, selector: #selector(handleQuickStartTourElementChangedNotification(_:)), name: .QuickStartTourElementChangedNotification, object: nil)
}

self?.updateViews()
@objc private func handleQuickStartTourElementChangedNotification(_ notification: Foundation.Notification) {
guard let userInfo = notification.userInfo,
let element = userInfo[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement,
element == .tourCompleted
else {
return
}
}

private func stopObservingQuickStart() {
NotificationCenter.default.removeObserver(self)
updateViews()
}

@objc private func didTap() {
Expand Down
35 changes: 19 additions & 16 deletions WordPress/Classes/ViewRelated/Blog/My Site/MySiteViewController+QuickStart.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,31 @@ import UIKit
extension MySiteViewController {

func startObservingQuickStart() {
NotificationCenter.default.addObserver(forName: .QuickStartTourElementChangedNotification, object: nil, queue: nil) { [weak self] (notification) in
NotificationCenter.default.addObserver(self, selector: #selector(handleQuickStartTourElementChangedNotification(_:)), name: .QuickStartTourElementChangedNotification, object: nil)
}

if let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {
@objc private func handleQuickStartTourElementChangedNotification(_ notification: Foundation.Notification) {
guard let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement
else {
return
}

self?.siteMenuSpotlightIsShown = element == .siteMenu
siteMenuSpotlightIsShown = element == .siteMenu

switch element {
case .noSuchElement, .newpost:
self?.additionalSafeAreaInsets = .zero
switch element {
case .noSuchElement, .newpost:
additionalSafeAreaInsets = .zero

case .siteIcon, .siteTitle, .viewSite:
self?.scrollView.scrollToTop(animated: true)
fallthrough
case .siteIcon, .siteTitle, .viewSite:
scrollView.scrollToTop(animated: true)
fallthrough

case .siteMenu, .pages, .sharing, .stats, .readerTab, .notifications, .mediaScreen:
self?.additionalSafeAreaInsets = Constants.quickStartNoticeInsets
case .siteMenu, .pages, .sharing, .stats, .readerTab, .notifications, .mediaScreen:
additionalSafeAreaInsets = Constants.quickStartNoticeInsets

default:
break
}
}
default:
break
}
}

Expand Down
20 changes: 9 additions & 11 deletions WordPress/Classes/ViewRelated/Blog/Site Picker/SitePickerViewController+QuickStart.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,17 @@ extension SitePickerViewController {
}

func startObservingQuickStart() {
NotificationCenter.default.addObserver(forName: .QuickStartTourElementChangedNotification, object: nil, queue: nil) { [weak self] (notification) in
guard self?.blog.managedObjectContext != nil else {
return
}

self?.blogDetailHeaderView.toggleSpotlightOnSiteTitle()
self?.blogDetailHeaderView.toggleSpotlightOnSiteUrl()
self?.blogDetailHeaderView.refreshIconImage()
}
NotificationCenter.default.addObserver(self, selector: #selector(handleQuickStartTourElementChangedNotification(_:)), name: .QuickStartTourElementChangedNotification, object: nil)
}

func stopObservingQuickStart() {
NotificationCenter.default.removeObserver(self)
@objc private func handleQuickStartTourElementChangedNotification(_ notification: Foundation.Notification) {
guard blog.managedObjectContext != nil else {
return
}

blogDetailHeaderView.toggleSpotlightOnSiteTitle()
blogDetailHeaderView.toggleSpotlightOnSiteUrl()
blogDetailHeaderView.refreshIconImage()
}

func startAlertTimer() {
Expand Down
4 changes: 0 additions & 4 deletions WordPress/Classes/ViewRelated/Blog/Site Picker/SitePickerViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ final class SitePickerViewController: UIViewController {
startObservingTitleChanges()
}

deinit {
stopObservingQuickStart()
}

private func setupHeaderView() {
blogDetailHeaderView.blog = blog
blogDetailHeaderView.delegate = self
Expand Down
12 changes: 7 additions & 5 deletions WordPress/Classes/ViewRelated/Reader/Tab Navigation/ReaderTabViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,13 @@ extension ReaderTabViewController {
// MARK: Observing Quick Start
extension ReaderTabViewController {
private func startObservingQuickStart() {
NotificationCenter.default.addObserver(forName: .QuickStartTourElementChangedNotification, object: nil, queue: nil) { [weak self] notification in
if let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {
self?.settingsButton.shouldShowSpotlight = element == .readerDiscoverSettings
}
NotificationCenter.default.addObserver(self, selector: #selector(handleQuickStartTourElementChangedNotification(_:)), name: .QuickStartTourElementChangedNotification, object: nil)
}

@objc private func handleQuickStartTourElementChangedNotification(_ notification: Foundation.Notification) {
if let info = notification.userInfo,
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {
settingsButton.shouldShowSpotlight = element == .readerDiscoverSettings
}
}
}
Expand Down