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

fix for iOS >= 15.4 (cam preview not visible) #1

Merged
merged 2 commits into from
Mar 24, 2022
Merged

Conversation

fabioz23
Copy link

fix for iOS >= 15.4 (cam preview not visible)

fix for iOS >= 15.4 (cam preview not visible)
@phausero
Copy link

Looking good to me. You could go the safe route by also setting the isOpaque property of the scrollView. Also, makeOpaque function is doing the wrong thing. You could easily fix this in here so hide() work too.

@v1934
Copy link
Owner

v1934 commented Mar 24, 2022

I wonder what the original author of cordova-plugin-qrscanner had on mind, that the opacity wasn't actually being reset after hiding for both Android and iOS.

@objc func makeOpaque(){
        self.webView?.isOpaque = false
        self.webView?.backgroundColor = UIColor.clear
    }
@objc func show(_ command: CDVInvokedUrlCommand) {
        self.webView?.isOpaque = false
        self.webView?.backgroundColor = UIColor.clear
        self.getStatus(command)
    }

Why even bother making 2 different methods that are the same? I'm thinking of fixing this along with this PR, but maybe there was some reasoning for that?

Yeah I think we should also set isOpaque like @phausero mentioned, just to be on the safe side

@phausero
Copy link

I think this is a mistake and there already is a PR to fix this because makeOpaque() is doing the opposite of what it should do. https://github.com/bitpay/cordova-plugin-qrscanner/pull/351/files

fix opaque and device selection
@fabioz23
Copy link
Author

@phausero i've inserted on this PR some changes from PR 351 from the abandoned repo and another fix related to camera selection (was using a deprecated method)

@fabioz23
Copy link
Author

@v1934 can you check if the same mistake is present on Android side?

@v1934
Copy link
Owner

v1934 commented Mar 24, 2022

It was, but I have fixed it in aa0f9dd#diff-52338800f13fca00cf795e162fb193f92026ea81ba418b56546af376f8cb904e

Looks good to me, I'll merge this PR and push a new release to npm later

@v1934 v1934 merged commit 0916e7e into v1934:master Mar 24, 2022
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