-
Notifications
You must be signed in to change notification settings - Fork 3
Pp 2310 qr code unsupported warning flag #1180
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
base: release/qr-code-improvements
Are you sure you want to change the base?
Changes from all commits
6168241
861f895
fdda0a3
e34dc25
6c33e25
b4d8e16
bceb1de
2970d07
f8f360a
aa4f9f6
754b5b5
4fdb092
1fe374b
9295a09
25249e1
8f2d97f
bc2f62c
d39e992
bebc167
156de27
46a171e
cf9f57e
17f68bc
5f41f49
fa65534
996b596
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -46,6 +46,10 @@ final class CameraViewController: UIViewController { | |||||||||||||||||
| private var isPresentedOnScreen = false | ||||||||||||||||||
|
|
||||||||||||||||||
| private var isValidIBANDetected: Bool = false | ||||||||||||||||||
| // Snapshot of the backend flag taken on the first invalid QR scan. | ||||||||||||||||||
| // Stays fixed for the session so all repeated scans show the same feedback type. | ||||||||||||||||||
| private var sessionUnsupportedQRCodeWarningEnabled: Bool? | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion]: Looks like one boolean should be enough like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Second bool is for manage the session to have 1 type of alert within same session. `Given the user already triggered an invalid QR-code warning earlier in the same SDK session, when the user scans a second (or subsequent) invalid QR-code, the SDK shows the same warning type as the first time. This holds even if the configuration endpoint response resolves in between and changes the flag value (e.g. unsupportedQRCodeWarningEnabled becomes true). Example: On first install, no ClientConfiguration is stored, so the default applies. The user scans an invalid QR-code before the endpoint responds and sees the old yellow warning. While they go back, the endpoint resolves with unsupportedQRCodeWarningEnabled = true. The user scans a second invalid QR-code and still sees the old yellow warning, not the new popup. The new popup only appears from the next SDK session onward.` |
||||||||||||||||||
| private var isWarningFlagSnapshotted = false | ||||||||||||||||||
| // Analytics | ||||||||||||||||||
| private var invalidQRCodeOverlayFirstAppearance: Bool = true | ||||||||||||||||||
| private var ibanOverlayFirstAppearance: Bool = true | ||||||||||||||||||
|
|
@@ -589,24 +593,39 @@ final class CameraViewController: UIViewController { | |||||||||||||||||
| resetQRCodeTask?.cancel() | ||||||||||||||||||
| detectedQRCodeDocument = document | ||||||||||||||||||
|
|
||||||||||||||||||
| hideQRCodeTask = DispatchWorkItem(block: { | ||||||||||||||||||
| self.resetQRCodeScanning(isValid: isValid) | ||||||||||||||||||
|
|
||||||||||||||||||
| if let QRDocument = self.detectedQRCodeDocument { | ||||||||||||||||||
| if isValid { | ||||||||||||||||||
| if isValid { | ||||||||||||||||||
| let task = DispatchWorkItem(block: { | ||||||||||||||||||
| self.resetQRCodeScanning(isValid: true) | ||||||||||||||||||
| if let QRDocument = self.detectedQRCodeDocument { | ||||||||||||||||||
| self.didPick(QRDocument) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| }) | ||||||||||||||||||
|
|
||||||||||||||||||
| if isValid { | ||||||||||||||||||
| }) | ||||||||||||||||||
| hideQRCodeTask = task | ||||||||||||||||||
| showValidQRCodeFeedback() | ||||||||||||||||||
| DispatchQueue.main.asyncAfter(deadline: .now() + Constants.hideQRCodeDelay, execute: task) | ||||||||||||||||||
| } else { | ||||||||||||||||||
| if !isValidIBANDetected { | ||||||||||||||||||
| showInvalidQRCodeFeedback() | ||||||||||||||||||
| // Snapshot the flag once so the feedback type stays consistent across | ||||||||||||||||||
| // repeated scans in the same session. A separate boolean guards the snapshot | ||||||||||||||||||
| // because assigning nil to a Bool? still leaves it nil, making a nil-check | ||||||||||||||||||
| // unreliable as a "has snapshotted" gate. | ||||||||||||||||||
|
qnaveed87 marked this conversation as resolved.
|
||||||||||||||||||
| if !isWarningFlagSnapshotted { | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we keep 1 boolean
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented as per the AC - 7 |
||||||||||||||||||
| sessionUnsupportedQRCodeWarningEnabled = GiniCaptureUserDefaultsStorage.unsupportedQRCodeWarningEnabled | ||||||||||||||||||
| isWarningFlagSnapshotted = true | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if sessionUnsupportedQRCodeWarningEnabled == true { | ||||||||||||||||||
| showUnsupportedQRCodeAlert() | ||||||||||||||||||
| } else { | ||||||||||||||||||
| showInvalidQRCodeFeedback() | ||||||||||||||||||
| let task = DispatchWorkItem(block: { | ||||||||||||||||||
| self.resetQRCodeScanning(isValid: false) | ||||||||||||||||||
| }) | ||||||||||||||||||
| hideQRCodeTask = task | ||||||||||||||||||
| DispatchQueue.main.asyncAfter(deadline: .now() + Constants.hideQRCodeDelay, execute: task) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| DispatchQueue.main.asyncAfter(deadline: .now() + 1.5, execute: hideQRCodeTask!) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private func showValidQRCodeFeedback() { | ||||||||||||||||||
|
|
@@ -651,6 +670,43 @@ final class CameraViewController: UIViewController { | |||||||||||||||||
| qrCodeOverLay.configureQrCodeOverlay(withCorrectQrCode: false) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private func showUnsupportedQRCodeAlert() { | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Clarification]: pauseQRDetection() schedules its work asynchronously on sessionQueue, but the QR delegate fires per-frame and fans out via DispatchQueue.main.async for each frame. By the time the pause actually takes effect, several showUnsupportedQRCodeAlert() calls are already queued on the main thread-each one firing sendGiniAnalyticsEventForInvalidQRCode() and playVoiceOverMessage(). UIKit silently swallows the duplicate present() calls so there's no crash, but analytics and voice-over fire N times. Introducing `guard in suggestion below can be a fix.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not able to re produce issue with voice over but added guard so that it wont happen fro any edge case |
||||||||||||||||||
| // Skip duplicate side-effects when the alert is already on screen and more frames | ||||||||||||||||||
| // are still queued before pauseQRDetection takes effect on the session queue. | ||||||||||||||||||
| guard presentedViewController == nil else { return } | ||||||||||||||||||
|
|
||||||||||||||||||
| let generator = UINotificationFeedbackGenerator() | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented as per the AC - 7 |
||||||||||||||||||
| generator.notificationOccurred(.warning) | ||||||||||||||||||
|
|
||||||||||||||||||
| cameraPreviewViewController.camera.pauseQRDetection() | ||||||||||||||||||
|
|
||||||||||||||||||
| sendGiniAnalyticsEventForInvalidQRCode() | ||||||||||||||||||
| playVoiceOverMessage(success: false) | ||||||||||||||||||
|
|
||||||||||||||||||
| let alert = UIAlertController(title: Strings.unsupportedQRAlertTitle, | ||||||||||||||||||
| message: nil, | ||||||||||||||||||
| preferredStyle: .alert) | ||||||||||||||||||
| alert.addAction(UIAlertAction(title: Strings.scanAnotherQRCode, style: .default) { [weak self] _ in | ||||||||||||||||||
| self?.handleScanAnotherQRCode() | ||||||||||||||||||
| }) | ||||||||||||||||||
| alert.addAction(UIAlertAction(title: Strings.takePhotoOfDocument, style: .default) { [weak self] _ in | ||||||||||||||||||
| self?.handleTakePhotoOfDocument() | ||||||||||||||||||
| }) | ||||||||||||||||||
|
|
||||||||||||||||||
| present(alert, animated: true) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private func handleScanAnotherQRCode() { | ||||||||||||||||||
| cameraPreviewViewController.camera.resumeQRDetection() | ||||||||||||||||||
| detectedQRCodeDocument = nil | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private func handleTakePhotoOfDocument() { | ||||||||||||||||||
| // QR detection stays paused — resuming here would immediately re-trigger the alert | ||||||||||||||||||
| cameraPreviewViewController.cameraFrameView.isHidden = false | ||||||||||||||||||
| detectedQRCodeDocument = nil | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private func isAccessibilityLargeTextEnabled() -> Bool { | ||||||||||||||||||
| let contentSizeCategory = UIApplication.shared.preferredContentSizeCategory | ||||||||||||||||||
| return contentSizeCategory.isAccessibilityCategory | ||||||||||||||||||
|
|
@@ -767,6 +823,7 @@ private extension CameraViewController { | |||||||||||||||||
| static let switcherPadding: CGFloat = 8 | ||||||||||||||||||
| static let phoneSwitcherSize: CGSize = CGSize(width: 124, height: 40) | ||||||||||||||||||
| static let tableSwitcherSize: CGSize = CGSize(width: 40, height: 124) | ||||||||||||||||||
| static let hideQRCodeDelay: TimeInterval = 1.5 | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private struct Strings { | ||||||||||||||||||
|
|
@@ -778,6 +835,21 @@ private extension CameraViewController { | |||||||||||||||||
| comment: "Info label") | ||||||||||||||||||
| static let cameraTitle = NSLocalizedStringPreferredFormat("ginicapture.navigationbar.camera.title", | ||||||||||||||||||
| comment: "Camera title") | ||||||||||||||||||
|
|
||||||||||||||||||
| static let unsupportedQRAlertTitleKey = "ginicapture.QRscanning.alert.title" | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion]: Wrapping all keys in the constants looks like overhead... was is need for lint?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. If we keep everything in a single line, SwiftLint raises a warning. We also follow the same pattern in |
||||||||||||||||||
| static let unsupportedQRAlertTitleComment = "Unsupported QR code alert title" | ||||||||||||||||||
| static let unsupportedQRAlertTitle = NSLocalizedStringPreferredFormat(unsupportedQRAlertTitleKey, | ||||||||||||||||||
| comment: unsupportedQRAlertTitleComment) | ||||||||||||||||||
|
|
||||||||||||||||||
| static let scanAnotherQRCodeKey = "ginicapture.QRscanning.alert.scanAnother" | ||||||||||||||||||
| static let scanAnotherQRCodeComment = "Scan another QR code button" | ||||||||||||||||||
| static let scanAnotherQRCode = NSLocalizedStringPreferredFormat(scanAnotherQRCodeKey, | ||||||||||||||||||
| comment: scanAnotherQRCodeComment) | ||||||||||||||||||
|
|
||||||||||||||||||
| static let takePhotoOfDocumentKey = "ginicapture.QRscanning.alert.takePhoto" | ||||||||||||||||||
| static let takePhotoOfDocumentComment = "Take photo of document button" | ||||||||||||||||||
| static let takePhotoOfDocument = NSLocalizedStringPreferredFormat(takePhotoOfDocumentKey, | ||||||||||||||||||
| comment: takePhotoOfDocumentComment) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| // swiftlint:enable type_body_length | ||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.