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

Update ScannerViewControllerDelegate.swift #30

Closed
wants to merge 1 commit into from
Closed

Update ScannerViewControllerDelegate.swift #30

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 17, 2019

I add the @objc to the ScannerViewController Delegate Method to use the delegate method in a objective-c iOS app.

I add the @objc to the ScannerViewController Delegate Method to use the delegate method in a objective-c iOS app.
Copy link
Contributor

@xavierLowmiller xavierLowmiller left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the PR!

We didn't really have Objective-C interop in mind when designing this library, but if this is the only change needed for it to work, I'm all for it.

This change has the minor downside that implementers of the delegate protocol must be an NSObject subclass, but I'd wager that most (if not all) implementations slap this onto a UIViewController, so this should be fine.

@fbernutz I'd love to hear your thoughts.

@@ -1,5 +1,5 @@
import UIKit

public protocol ScannerViewControllerDelegate: AnyObject {
@objc public protocol ScannerViewControllerDelegate: AnyObject {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the : AnyObject?

Since @objc protocols must be implemented by NSObject subclasses, we don't need this annotation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@objc public protocol ScannerViewControllerDelegate: AnyObject {
@objc public protocol ScannerViewControllerDelegate {

@fbernutz
Copy link
Contributor

I think this should be fine. This can be merged, when you've resolved the conversation with @xavierLowmiller 👍
Thanks for your contribution, @rwarnecke 👏

@fbernutz fbernutz self-requested a review March 19, 2019 13:17
fbernutz added a commit that referenced this pull request Oct 18, 2019
(Original idea, see PR #30 by RWarnecke)
@fbernutz fbernutz closed this Oct 18, 2019
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.

2 participants