Skip to content

My Site Dashboard: fix crash on iOS 13/14 #18531

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

Closed
wants to merge 2 commits into from
Closed

My Site Dashboard: fix crash on iOS 13/14 #18531

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd1c283
add: disable animations for non-iOS 15 devices
leandroalonso May 6, 2022
503c063
refactor: typo
leandroalonso May 6, 2022
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
12 changes: 9 additions & 3 deletions WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Posts/PostsCardViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class PostsCardViewModel: NSObject {
didSet {
if oldValue != currentState {
forceReloadSnapshot()
trackCardDisaplyedIfNeeded()
trackCardDisplayedIfNeeded()
}
}
}
Expand Down Expand Up @@ -259,7 +259,7 @@ private extension PostsCardViewModel {
return false
}

func trackCardDisaplyedIfNeeded() {
func trackCardDisplayedIfNeeded() {
switch currentState {
case .posts:
BlogDashboardAnalytics.shared.track(.dashboardCardShown, properties: ["type": "post", "sub_type": status.rawValue])
Expand Down Expand Up @@ -370,7 +370,13 @@ extension PostsCardViewModel: NSFetchedResultsControllerDelegate {
}

private func applySnapshot(_ snapshot: Snapshot, to dataSource: DataSource) {
let shouldAnimate = view?.tableView.numberOfSections != 0 && view?.tableView.numberOfRows(inSection: 0) != 0
var shouldAnimate: Bool
if #available(iOS 15.0, *) {
shouldAnimate = view?.tableView.numberOfSections != 0 && view?.tableView.numberOfRows(inSection: 0) != 0
Copy link
Contributor Author

@leandroalonso leandroalonso May 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hassaanelgarem I was unable to come up with a good solution for that.

As for now, animations are disabled for iOS 13 and 14.

Before the refactor we identified this issue and we fixed using the numberOfRows verification. However, due to the refactor this returns true because the ghost cells are in the same table view.

I tried adding an additional check to ignore ghost cells but it didn't work. From the ghost cells to rendering the list of posts, the app crashes. Overall, this is the crash I've been seeing:

Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of sections. The number of sections contained in the table view after the update (1) must be equal to the number of sections contained in the table view before the update (1), plus or minus the number of sections inserted or deleted (1 inserted, 0 deleted).'

@wargcm reported that he had this one:

Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'UITableView dataSource returned a nil cell for row at index path: <NSIndexPath: 0xb268d3b7ae136b70> {length = 2, path = 0 - 0}. Table view:

Adding all this context here because maybe you might have an idea of a better solution that would allow us to keep the animation.

Copy link
Contributor

@hassaanelgarem hassaanelgarem May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leandroalonso Thanks for all the context!

I've dived into this a bit and was able to come up with an alternate solution. That solution tackles the core of the problem and lets us keep the animation in all cases. I've created a PR with what I came up with here #18537

} else {
shouldAnimate = false
}

dataSource.defaultRowAnimation = .fade
dataSource.apply(snapshot, animatingDifferences: shouldAnimate, completion: nil)
view?.tableView.allowsSelection = currentState == .posts
Expand Down