Skip to content

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Jan 4, 2023

Closes: #8539

Description

Note: I'm tagging reviewers based on the author/reviewer of the related changes while the author isn't on the team anymore.

The crash

This PR aims to fix a recent crash in release 11.6 that I got to reproduce. The crash happens when the dashboard is showing the Add Product header, then the header is hidden like in landscape mode (the navigation bar is collapsed). Then, the dashboard content view gets replaced (e.g. when analytics is re-enabled) while the Auto Layout constraint that connects the header and the previous content view is being activated separately (DashboardViewController.observeNavigationBarHeightForHeaderVisibility).

The fix

To prevent the Auto Layout constraint from being activated when it includes a view that is being removed from the view hierarchy, the constraint is reset to nil before the content view is removed next. The constraint will be re-established in onDashboardUIUpdate's call to addViewBelowHeaderStackView for the new content view.

Testing instructions

Prerequisite: the Add Product banner is shown for the store

  • Launch the app and log in to a store where the Add Product banner is shown --> the banner should be shown on the My store tab
  • On web, go to the store's /wp-admin/admin.php?page=wc-settings&tab=advanced&section=features to disable analytics (uncheck Enables WooCommerce Analytics)
  • In the app, pull to refresh the My store tab
  • Rotate the screen to landscape mode and back to portrait mode --> the Add Product banner is not shown anymore
  • On web, go to the store's /wp-admin/admin.php?page=wc-settings&tab=advanced&section=features to re-enable analytics (chcek Enables WooCommerce Analytics)
  • In the app, pull to refresh the My store tab --> the app should not crash, and the Add Product banner is shown again above the analytics

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@jaclync jaclync added the type: crash The worst kind of bug. label Jan 4, 2023
@jaclync jaclync added this to the 11.8 milestone Jan 4, 2023
@jaclync jaclync requested a review from toupper January 4, 2023 02:44
@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8540-be35f31 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@toupper toupper self-assigned this Jan 4, 2023
@toupper
Copy link
Contributor

toupper commented Jan 4, 2023

Great finding @jaclync 👏 Thanks for fixing this, LGTM :shipit:

@jaclync jaclync merged commit cdac7d8 into trunk Jan 5, 2023
@jaclync jaclync deleted the crash/8539-fix-dashboard-header-crash branch January 5, 2023 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: crash The worst kind of bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard: NSGenericException: Unable to activate constraint with anchors <NSLayoutYAxisAnchor:0x280c2f3c0 "UIView:0x10620e8d0.t...

4 participants