Scan QR code from static image

[ericholguin]

This PR allows users to open the link in a QR Code if it exists. Detects if there is a qr code and extracts that info, if it is an lnurl it'll append the lightning prefix so it can be opened by a lightning wallet. If it is a regular link it'll open a dialog to ask the user if they want to open the link, with the link displayed. If there is no link it show a dialog that says no qr code found.

See these notes for video examples:

https://damus.io/note1mpc6y8ft80wpavmedsy7h08s7n2l95flm0d3tlqd6hfdd27ta38s4rusrj

https://damus.io/note1dw9y7lc09f3694avg2z2a82kc68yh479vkkur336g78w9svh3tfsp0yy5g

[jb55]

Hey! Comments inline:

On Tue, Sep 19, 2023 at 06:15:50PM -0600, ericholguin wrote: Closes: #1550 --- .../Images/ImageContextMenuModifier.swift | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/damus/Views/Images/ImageContextMenuModifier.swift b/damus/Views/Images/ImageContextMenuModifier.swift index a3090bb5f..f75b4c3da 100644 --- a/damus/Views/Images/ImageContextMenuModifier.swift +++ b/damus/Views/Images/ImageContextMenuModifier.swift @@ -12,8 +12,15 @@ import UIKit struct ImageContextMenuModifier: ViewModifier { let url: URL? let image: UIImage? + + @State var qrCodeLink: String = "" + @State var open_link_confirm: Bool = false + @State var no_link_found: Bool = false + @binding var showShareSheet: Bool + @Environment(\.openURL) var openURL + func body(content: Content) -> some View { return content.contextMenu { Button { @@ -32,6 +39,32 @@ struct ImageContextMenuModifier: ViewModifier { } label: { Label(NSLocalizedString("Save Image", comment: "Context menu option to save an image."), image: "download") } + Button { + qrCodeLink = "" + let detector:CIDetector = CIDetector(ofType: CIDetectorTypeQRCode, context: nil, options: [CIDetectorAccuracy:CIDetectorAccuracyHigh])! + let ciImage: CIImage = CIImage(image:someImage)! + let features = detector.features(in: ciImage) + for feature in features as! [CIQRCodeFeature] { + qrCodeLink += feature.messageString! + }

Let's avoid using exclamation marks which may crash. This can just be: for feature in features as? [CIQRCodeFeature] { if let feature, let mstr = feature.messageString { qrCodeLink += mstr } }

+ + if qrCodeLink == "" { + no_link_found.toggle() + } else { + if qrCodeLink.contains("lnurl") { + // TO DO: + // Ideally this would open the user's default wallet from their settings but that would mean modifying way too many files

you can just use UserSettingsStore.shared if you don't want to pass it down. We probably need to change the shared property to static var shared: UserSettingsStore!

…

+ qrCodeLink = "lightning:\(qrCodeLink)" + if let url = URL(string: qrCodeLink) { + openURL(url) + } + } else if let _ = URL(string: qrCodeLink) { + open_link_confirm.toggle() + } + } + } label: { + Label(NSLocalizedString("Scan for QR Code", comment: "Context menu option to scan image for a QR Code."), image: "qr-code.fill") + } } Button { showShareSheet = true @@ -39,5 +72,16 @@ struct ImageContextMenuModifier: ViewModifier { Label(NSLocalizedString("Share", comment: "Button to share an image."), image: "upload") } } + .alert(NSLocalizedString("Found \(qrCodeLink).\nOpen link?", comment: "Alert message asking if the user wants to open the link."), isPresented: $open_link_confirm) { + Button(NSLocalizedString("Open", comment: "Button to proceed with opening link."), role: .none) { + if let url = URL(string: qrCodeLink) { + openURL(url) + } + } + Button(NSLocalizedString("Cancel", comment: "Button to cancel the upload."), role: .cancel) {} + } + .alert(NSLocalizedString("Unable to find a QR Code", comment: "Alert message letting user know a link was not found."), isPresented: $no_link_found) { + Button(NSLocalizedString("Dismiss", comment: "Button to dismiss alert"), role: .cancel) {} + } } }

[jb55]

On Wed, Sep 20, 2023 at 08:54:17AM -0400, William Casarin wrote: >+ // TO DO: >+ // Ideally this would open the user's default wallet from their settings but that would mean modifying way too many files you can just use UserSettingsStore.shared if you don't want to pass it down. We probably need to change the shared property to static var shared: UserSettingsStore!

Now that I think about it you may to be careful when doing this, we only set this during connect, but if there are image views that use this modifier before the main ContentView then it may crash. Ideally we just update the modifier to take in the UserSettingsStore, and update all the callsites, otherwise we open us to more crashing and non-testability. This is why I'd rather be explicit about view dependencies rather than implicit environment stuff or global variables. No need to be afraid about touching many files, sometimes it just makes the most sense.

[ericholguin]

comments addressed

[jb55]

On Sun, Sep 24, 2023 at 10:42:27AM -0700, Eric Holguin wrote: comments addressed

thanks! will take a look today

[jb55]

On Wed, Sep 20, 2023 at 07:14:53PM -0600, ericholguin wrote: @@ -44,20 +45,17 @@ struct ImageContextMenuModifier: ViewModifier { let detector:CIDetector = CIDetector(ofType: CIDetectorTypeQRCode, context: nil, options: [CIDetectorAccuracy:CIDetectorAccuracyHigh])! let ciImage: CIImage = CIImage(image:someImage)! let features = detector.features(in: ciImage) - for feature in features as! [CIQRCodeFeature] { - qrCodeLink += feature.messageString! + if let qrfeatures = features as? [CIQRCodeFeature] { + for feature in qrfeatures { + qrCodeLink += feature.messageString!

looks like this is still using ! but I can fix that up

[jb55]

This PR needs to be rebased. Merging master into your branch is not the right way to do it unfortunately: 1. Individual patches are invalidated by the merge. It's impossible to know if individual commits are doing the right thing if some merge commit changes them after the fact. 2. It makes review 1000x more difficult when the same code is touched multiple times across multiple commits. 3. It completely breaks git bisect as a tool for debugging which commit introduced which bug. Since changes are no longer localized to the commit that made the change in the first place. Always rebase and never merge master into your branch. Thanks :)

[ericholguin]

Redone in #1566