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

Add Cardinal UI Type and Render Types #1134

Merged
merged 13 commits into from
Nov 15, 2023
Merged

Conversation

jaxdesmarais
Copy link
Contributor

Summary of changes

Checklist

  • Added a changelog entry

Authors

@jaxdesmarais jaxdesmarais requested a review from a team as a code owner November 13, 2023 21:15
///
/// - Note: When using `BTThreeDSecureUIType.both` or `BTThreeDSecureUIType.html`, all `BTThreeDSecureRenderType` options must be set.
/// When using `BTThreeDSecureUIType.native`, all `BTThreeDSecureRenderType` options except `BTThreeDSecureRenderType.html` must be set.
public var renderTypes: [BTThreeDSecureRenderType.StringValue]?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arrays of Int enums do not exist in Obj-C so we cannot use the cleaner version (array of enums) here. 😢 This seems to be the consensus on the best practice in these cases. Open to other ideas if folks find something that works better as this is certainly not the most Swift-y approach. But working in the confines of Obj-C it seems to be our best bet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have any better ideas for this but curious if we are planning to remove obj-c support in the next major version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently there isn't a plan to remove Obj-C support in the next major version but we could consider discussing it further as an option! Or if Apple announces an official deprecation policy at some point it'd be a much easier choice.

public static let multiSelect: StringValue = "CardinalSessionRenderTypeMultiSelect"

/// OOB
public static let oob: StringValue = "CardinalSessionRenderTypeOOB"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the @objc annotation work for us (stack post)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That works for the enum but that's not the issue, the issue is that arrays of Int enums don't exist in Obj-C and enums can only be bridged to Int types to be interoperable with Obj-C. A Swift array is bridged to Obj-C's NSArray, and an Obj-C array should contain only pointers, but an enum of Int doesn't contain pointers so cannot be bridged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah got it yeah. That makes sense. Swift does have OptionSet as an NS_OPTIONS equivalent, but I can't tell if option sets are accessible from Obj-C.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 we may be able to make that work - it'll need to be a class instead of a struct since structs don't exist in Obj-c, but I think that may be okay. I'll poke around at it a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done: 5911c55. Great suggestion, that's way cleaner 🔥

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also if anyone is interested this is how the implementation will look for Obj-C merchants:

BTThreeDSecureRequest *request = [[BTThreeDSecureRequest alloc] init];
request.renderTypes = @[BTThreeDSecureRenderType.oob, BTThreeDSecureRenderType.html, BTThreeDSecureRenderType.singleSelect, BTThreeDSecureRenderType.multiSelect, BTThreeDSecureRenderType.otp];

@jaxdesmarais jaxdesmarais merged commit 93d3321 into main Nov 15, 2023
6 checks passed
@jaxdesmarais jaxdesmarais deleted the cardinal-ui-and-render-type branch November 15, 2023 17:09
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