[WCiOS17] Address warnings from WordPressAuthenticator deprecations #16391
Conversation
Generated by 🚫 Danger |
iamgabrielma
added
type: task
|
|
RafaelKayumov
left a comment
RafaelKayumov
left a comment
There was a problem hiding this comment.
The change and cleanup look good and reasonable to me.
The WPNUXSecondaryButton is only used in LoginUsernamePasswordViewController and LoginSelfHostedViewController. But looks like those VC's are only presented in the legacy flow and seem to be unreachable in current flows.
I think we should investigate if those view controllers can be removed including the WPNUXSecondaryButton I wish I was more fluent with authorization flows. I'll approve the change and create a backlog issue for the investigation.
GPT 5.1 Codex analysis
The only code that still instantiates LoginUsernamePasswordViewController or LoginSelfHostedViewController lives in the legacy login stack (LoginSiteAddressViewController, LoginEmailViewController). In the shipping WooCommerce app we now drive everything through Unified Auth:
Entry screen is SiteAddressViewController (Unified Auth). When the delegate asks for password entry, it either shows SiteCredentialsViewController (self-hosted) or GetStartedViewController/email login for WP.com sites.
The delegate (AuthenticationManager) never requests presentPasswordController(isSelfHosted: false), so showWPUsernamePassword() is effectively dead code. Likewise, Unified Auth replaces LoginSelfHostedViewController with SiteCredentialsViewController.
So, in the current app build there is no live navigation path to either LoginUsernamePasswordViewController or LoginSelfHostedViewController; they’re leftovers from the pre–Unified Auth flow.
|
Thanks for the review!
Agree, I think there's a bunch of stuff that can be removed or cleaned up from the auth flows, but I haven't worked on those to know better and wanted to be careful. Thanks for logging a followup! |
iamgabrielma
deleted the
task/wcios17-warnings-clean-up-wpauth-buttons
branch
Closes WOOMOB-1796
Description
This PR addresses the
titleEdgeInsetsandsetTitleEdgeInsetsdeprecation warnings coming from WordPressAuthenticator, by makingWPNUXSecondaryButtonuse the newUIButtonConfigurationAPI instead.We also remove other bits that throw warnings but are unused in the WCiOS codebase:
WPNUXSecondaryButton, andWPWalkthroughOverlayView. Why? Since these are no longer maintained as part of the WordPressAuthenticator library, are now part of the WCiOS codebase and represent a maintenance burden for us, so let's get rid of them if unused.Test Steps
Tagging kiwi for review as you know this flow better. AFAIK
WPNUXSecondaryButtonTitleEdgeInsetsis only used for the "forgot password" button inLoginSelfHostedViewControllerandLoginUsernamePasswordViewController, so these should look the same as before.I've checked the connection flow through Menu > Select Store as well, but I think is not used there.
Screenshots