Skip to content

Commit

Permalink
Merge pull request #20997 from wordpress-mobile/fix-memory-leak-in-no…
Browse files Browse the repository at this point in the history 
…tification-observers-3

Fix discarded quick start notification observers
  • Loading branch information
@crazytonyli
crazytonyli authored Jul 5, 2023
2 parents 6c06381 + accb98d commit 7685808
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 120 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
View file
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]
* [**] [internal] Fixed an issue that was causing a memory leak in the domain selection flow. [#20813]


Expand Down
43 changes: 20 additions & 23 deletions WordPress/Classes/ViewRelated/Blog/Blog Dashboard/BlogDashboardViewController.swift
View file
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
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
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
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
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
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
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
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

0 comments on commit 7685808

Please sign in to comment.