Conversation
92c63e8 to
e5f70b3
Compare
- ConnectButton: activate the orphaned lessThanOrEqualTo bottom constraint so its return value is not ignored. - AuthenticationError: add .notInteractive, .matchedExcludedCredential, .credentialImport, .credentialExport to mirror the new ASAuthorizationError.Code cases Apple added in iOS 15.4 / 18 / 18.2. - SignInWithAppleAuthentication: handle those new codes behind availability checks and map each 1:1 to the corresponding AuthenticationError so callers see the actual Apple-side reason rather than a generic .failed / .unknown bucket.
e5f70b3 to
965726b
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses Swift compiler warnings and improves error fidelity by aligning the SDK’s Sign in with Apple error handling with newer ASAuthorizationError.Code cases while fixing an unused Auto Layout constraint result in ConnectButton.
Changes:
- Activate a previously-unused “bottom ≤ container bottom” footer label constraint in
ConnectButton. - Extend
AuthenticationErrorwith newer Apple authorization error cases. - Map newer
ASAuthorizationError.Codevalues to dedicatedAuthenticationErrorcases behind availability checks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
IFTTT SDK/WebServiceAuthentication.swift |
Adds new AuthenticationError cases and documentation for newer Apple authorization errors. |
IFTTT SDK/SignInWithAppleAuthentication.swift |
Adds availability-gated mappings from newer ASAuthorizationError.Code cases to AuthenticationError. |
IFTTT SDK/ConnectButton.swift |
Activates a previously unactivated layout constraint to remove a compiler warning and enforce intended layout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// non-interactive context. Mirrors `ASAuthorizationError.notInteractive` | ||
| /// (iOS 15.4+). | ||
| /// - matchedExcludedCredential: A matched credential was excluded from use. | ||
| /// Mirrors `ASAuthorizationError.matchedExcludedCredential` (iOS 18+). | ||
| /// - credentialImport: An error occurred importing a credential. Mirrors | ||
| /// `ASAuthorizationError.credentialImport` (iOS 18.2+). | ||
| /// - credentialExport: An error occurred exporting a credential. Mirrors | ||
| /// `ASAuthorizationError.credentialExport` (iOS 18.2+). |
There was a problem hiding this comment.
The doc comments reference ASAuthorizationError.notInteractive / .matchedExcludedCredential / .credentialImport / .credentialExport, but these cases live on ASAuthorizationError.Code (e.g., ASAuthorizationError.Code.notInteractive). Updating the symbol names in the documentation will avoid misleading API docs for maintainers/callers.
| /// non-interactive context. Mirrors `ASAuthorizationError.notInteractive` | |
| /// (iOS 15.4+). | |
| /// - matchedExcludedCredential: A matched credential was excluded from use. | |
| /// Mirrors `ASAuthorizationError.matchedExcludedCredential` (iOS 18+). | |
| /// - credentialImport: An error occurred importing a credential. Mirrors | |
| /// `ASAuthorizationError.credentialImport` (iOS 18.2+). | |
| /// - credentialExport: An error occurred exporting a credential. Mirrors | |
| /// `ASAuthorizationError.credentialExport` (iOS 18.2+). | |
| /// non-interactive context. Mirrors `ASAuthorizationError.Code.notInteractive` | |
| /// (iOS 15.4+). | |
| /// - matchedExcludedCredential: A matched credential was excluded from use. | |
| /// Mirrors `ASAuthorizationError.Code.matchedExcludedCredential` (iOS 18+). | |
| /// - credentialImport: An error occurred importing a credential. Mirrors | |
| /// `ASAuthorizationError.Code.credentialImport` (iOS 18.2+). | |
| /// - credentialExport: An error occurred exporting a credential. Mirrors | |
| /// `ASAuthorizationError.Code.credentialExport` (iOS 18.2+). |
| // Cases added after the SDK's iOS 14.2 deployment target. | ||
| // Each is referenced behind an availability check and mapped | ||
| // to the matching AuthenticationError case so callers can | ||
| // distinguish it from the generic .failed / .unknown bucket. | ||
| if #available(iOS 15.4, *), authorizationError.code == .notInteractive { |
There was a problem hiding this comment.
AuthenticationError includes .presentationContextInvalid, but didCompleteWithError never maps ASAuthorizationError.Code.presentationContextInvalid to it (it currently falls through to .unknown). Add an explicit mapping so callers can distinguish this error from .unknown/.failed, consistent with the 1:1 mapping intent described in the comment.
| footerLabel.bottomAnchor.constraint(lessThanOrEqualTo: footerLabelContainer.bottomAnchor) | ||
| footerLabel.bottomAnchor.constraint(lessThanOrEqualTo: footerLabelContainer.bottomAnchor).isActive = true | ||
| let breakableBottomConstraint = footerLabel.bottomAnchor.constraint(equalTo: footerLabelContainer.bottomAnchor) | ||
| breakableBottomConstraint.priority = .defaultHigh |
There was a problem hiding this comment.
breakableBottomConstraint is created and its priority is set, but it’s never activated. If the intent is to keep the footer label bottom-aligned when possible (and let it break when the container is taller), the constraint should be activated; otherwise this constraint has no effect and the label may not pin to the container bottom even when it could.
| breakableBottomConstraint.priority = .defaultHigh | |
| breakableBottomConstraint.priority = .defaultHigh | |
| breakableBottomConstraint.isActive = true |
|
Bundling these changes into #312 instead — the PR-311 commit (with the three Copilot review fixes applied: |
Summary
ConnectButton.swiftThe
constraint(lessThanOrEqualTo:)call on line 348 was never activated, leaving the "label bottom must not exceed container bottom" relationship dangling. The compiler flagged the unused result. Activate it explicitly — matches the clear intent of the comment above it.AuthenticationError+SignInWithAppleAuthentication.swiftApple has added new
ASAuthorizationError.Codecases since the SDK was written. Add matching cases toAuthenticationErrorand map 1:1 so callers see the real Apple-side reason instead of being funneled into a generic.failed/.unknownbucket.ASAuthorizationError.CodeAuthenticationErrorcase.notInteractive.notInteractive.matchedExcludedCredential.matchedExcludedCredential.credentialImport.credentialImport.credentialExport.credentialExportEach Apple case is referenced behind an
if #availablecheck so the SDK still builds at its iOS 14.2 deployment target. The original switch (with@unknown default) is kept as a safety net for future Apple additions. This also clears the "switch must be exhaustive" warning that newer compilers raised against the previous iOS-13-only case set.Test plan
ConnectButton.swift:348orSignInWithAppleAuthentication.swift:44AuthenticationErrorcases