-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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 Site Switcher screen not dismissing after adding self-hosted site #24273
base: trunk
Are you sure you want to change the base?
Fix Site Switcher screen not dismissing after adding self-hosted site #24273
Conversation
…swift - Add `signinObserver` to listen for sign-in completion notifications. - Implement observer removal in `deinit` to prevent memory leaks.
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr24273-b3594ec | |
Version | 25.8 | |
Bundle ID | org.wordpress.alpha | |
Commit | b3594ec | |
App Center Build | WPiOS - One-Offs #11801 |
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr24273-b3594ec | |
Version | 25.8 | |
Bundle ID | com.jetpack.alpha | |
Commit | b3594ec | |
App Center Build | jetpack-installable-builds #10825 |
…dding-a-self-hosted-site
…dding-a-self-hosted-site
…dding-a-self-hosted-site
…dding-a-self-hosted-site
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but left one nit
} | ||
|
||
deinit { | ||
if let observer = signinObserver { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) it's redundant
If your app targets iOS 9.0 and later or macOS 10.11 and later, and you used addObserver(_:selector:name:object:), you do not need to unregister the observer. If you forget or are unable to remove the observer, the system cleans up the next time it would have posted to it.
Fixes #24091
The Site Switcher screen wasn't being dismissed automatically after successfully adding a self-hosted site. This PR adds proper handling of the
WPSigninDidFinishNotification
notification to ensure the screen is dismissed when authentication completes.Testing Instructions
Screen recording
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-03-20.at.14.53.52.mp4
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: