Skip to content
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

@W-17291595: [iOS] [Instrumentation] Add QR Code feature logging to track adoption #3790

Conversation

JohnsonEricAtSalesforce
Copy link
Contributor

🎸 _Ready For Review_ 🥁

This is a counterpart for forcedotcom/SalesforceMobileSDK-Android#2647

@@ -422,6 +424,7 @@ - (BOOL)loginWithCompletion:(SFUserAccountManagerSuccessCallbackBlock)completion
frontDoorBridgeUrl:(nullable NSURL * )frontDoorBridgeUrl
codeVerifier:(nullable NSString *)codeVerifier
{
[SFSDKAppFeatureMarkers registerAppFeature:kSFAppFeatureQrCodeLogin];
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to my comment on your Android PR is there anywhere QR Code specific we could put this instead? I did a quick search and didn't find anything (and also noticed that the isQrCodeLoginEnabled property in SalesforceSDKManager is never set/used anywhere 👀).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a little tricky to find since the iOS side has more layers of abstraction even before QR Code Login was introduced. It's there, though. Take a look at ac87637

…rack adoption (Move QR Code Login feature tracking to a more specifically named location.)
@JohnsonEricAtSalesforce JohnsonEricAtSalesforce merged commit 75de1c6 into forcedotcom:dev Jan 3, 2025
3 of 6 checks passed
@JohnsonEricAtSalesforce JohnsonEricAtSalesforce deleted the feature/w-17291595_ios-instrumentation-add-qr-code-feature-logging-to-track-adoption branch January 3, 2025 22:53
Copy link
Contributor

@wmathurin wmathurin left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants