Skip to content

Conversation

@iamgabrielma
Copy link
Contributor

Closes WOOMOB-1796

Description

This PR addresses the titleEdgeInsets and setTitleEdgeInsets deprecation warnings coming from WordPressAuthenticator, by making WPNUXSecondaryButton use the new UIButtonConfiguration API instead.

We also remove other bits that throw warnings but are unused in the WCiOS codebase: WPNUXSecondaryButton, and WPWalkthroughOverlayView. 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 WPNUXSecondaryButtonTitleEdgeInsets is only used for the "forgot password" button in LoginSelfHostedViewController and LoginUsernamePasswordViewController, 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

  • Before and after:
Simulator Screenshot - iPhone 16 Pro Max - 2025-11-25 at 15 38 52

@dangermattic
Copy link
Collaborator

dangermattic commented Nov 25, 2025

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@iamgabrielma iamgabrielma added type: task An internally driven task. type: technical debt Represents or solves tech debt of the project. labels Nov 25, 2025
@iamgabrielma iamgabrielma added this to the 23.8 milestone Nov 25, 2025
@iamgabrielma iamgabrielma marked this pull request as ready for review November 25, 2025 09:01
@wpmobilebot
Copy link
Collaborator

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16391-d70d12c
Version23.7
Bundle IDcom.automattic.alpha.woocommerce
Commitd70d12c
Installation URL6t1pgskrv4js8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@iamgabrielma iamgabrielma modified the milestones: 23.8, 23.9 Nov 28, 2025
Copy link
Contributor

@RafaelKayumov RafaelKayumov left a comment

Choose a reason for hiding this comment

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

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.

@iamgabrielma
Copy link
Contributor Author

Thanks for the review!

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.

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 iamgabrielma merged commit 1713c0e into trunk Dec 3, 2025
26 of 27 checks passed
@iamgabrielma iamgabrielma deleted the task/wcios17-warnings-clean-up-wpauth-buttons branch December 3, 2025 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: task An internally driven task. type: technical debt Represents or solves tech debt of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants