Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import BitwardenResources
import SwiftUI
import UIKit
import VisionKit

// MARK: - CardScannerWrapperView
Expand All @@ -21,6 +22,12 @@ struct CardScannerWrapperView: View {
/// Drives `startScanning()`/`stopScanning()` via the SwiftUI view lifecycle.
@SwiftUI.State private var isScanning = false

/// Counts how many stop-then-restart cycles have been attempted this session in response to
/// genuine scanner failures (`onScannerUnavailable`). Capped at 2 to avoid infinite loops;
/// resets on each `.onAppear`. Foreground-triggered restarts do not count against this budget.
@SwiftUI.State private var scannerRetryCount = 0

/// Dismisses the sheet when the scanner gives up after exhausting retries.
@Environment(\.dismiss) private var dismiss

var body: some View {
Expand All @@ -35,11 +42,15 @@ struct CardScannerWrapperView: View {
scanner: scanner,
onLinesUpdated: onLinesUpdated,
isScanning: $isScanning,
onScannerUnavailable: restartScanning,
)
.padding(.horizontal, 12)
.padding(.top, 12)
.padding(.bottom, 35)
.onAppear { isScanning = true }
.onAppear {
scannerRetryCount = 0
isScanning = true
}
.onDisappear { isScanning = false }
}
.navigationTitle(Localizations.scanCard)
Expand All @@ -51,6 +62,43 @@ struct CardScannerWrapperView: View {
}
}
.navigationViewStyle(.stack)
.onReceive(NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification)) { _ in

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@matt-livefront I wasn't sure whether this would be a good idea to put it directly here. It seems more like SwiftUI but less testable so if needed I can move it to the processor and do the logic somehow there.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it's ok given that we would have used scene phase here and it's for a minor and self-contained use.

// `scenePhase` is driven by SwiftUI's own `Scene`/`App` lifecycle and is unreliable
// for views hosted in a UIKit app via `UIHostingController`, so foreground transitions
// are observed directly via `UIApplication` notifications instead.
if isScanning {
Comment on lines +65 to +69
resumeScanningAfterForeground()
}
}
}

// MARK: Private

/// Stops scanning, waits 300 ms for the camera to fully release, then restarts.
/// After two retries the sheet is dismissed so the user is never left with a blank screen.
private func restartScanning() {
guard scannerRetryCount < 2 else {
dismiss()
return
}
scannerRetryCount += 1
isScanning = false
Comment on lines +77 to +85
Task { @MainActor in
try? await Task.sleep(for: .milliseconds(300))
isScanning = true
}
Comment on lines +84 to +89
}

/// Stops scanning, waits 300 ms for the camera to fully release, then restarts, in response to
/// the app returning to the foreground. Unlike `restartScanning()`, this does not consume the
/// failure retry budget, since a foreground transition is a routine occurrence rather than a
/// scanner failure and would otherwise exhaust the budget after a couple of normal app switches.
private func resumeScanningAfterForeground() {
isScanning = false
Task { @MainActor in
try? await Task.sleep(for: .milliseconds(300))
isScanning = true
}
}
}

Expand All @@ -75,6 +123,10 @@ struct CardScannerView: UIViewControllerRepresentable {
/// When `true`, scanning is active; when `false`, scanning is stopped.
@Binding var isScanning: Bool

/// Called when `startScanning()` throws or the scanner becomes unavailable at runtime
/// (e.g. camera interrupted). The wrapper uses this to schedule a stop-then-restart cycle.
var onScannerUnavailable: (() -> Void)?

// MARK: Static methods for UIViewControllerRepresentable

/// Stops scanning and clears the delegate when SwiftUI removes this representable from the hierarchy,
Expand Down Expand Up @@ -103,7 +155,7 @@ struct CardScannerView: UIViewControllerRepresentable {
// MARK: UIViewControllerRepresentable

func makeCoordinator() -> Coordinator {
Coordinator(onLinesUpdated: onLinesUpdated)
Coordinator(onLinesUpdated: onLinesUpdated, onScannerUnavailable: onScannerUnavailable)
}

func makeUIViewController(context: Context) -> DataScannerViewController {
Expand All @@ -114,7 +166,11 @@ struct CardScannerView: UIViewControllerRepresentable {

func updateUIViewController(_ uiViewController: DataScannerViewController, context: Context) {
if isScanning {
try? uiViewController.startScanning()
do {
try uiViewController.startScanning()
} catch {
DispatchQueue.main.async { context.coordinator.onScannerUnavailable?() }
Comment thread
matt-livefront marked this conversation as resolved.
}
} else {
uiViewController.stopScanning()
}
Expand All @@ -139,10 +195,15 @@ extension CardScannerView {

let onLinesUpdated: ([String]) -> Void

/// Forwarded from `CardScannerView.onScannerUnavailable`; called when `startScanning()`
/// fails or the camera is interrupted at runtime.
var onScannerUnavailable: (() -> Void)?

// MARK: Initialization

init(onLinesUpdated: @escaping ([String]) -> Void) {
init(onLinesUpdated: @escaping ([String]) -> Void, onScannerUnavailable: (() -> Void)?) {
self.onLinesUpdated = onLinesUpdated
self.onScannerUnavailable = onScannerUnavailable
}

// MARK: DataScannerViewControllerDelegate
Expand Down Expand Up @@ -173,6 +234,13 @@ extension CardScannerView {
updateLines(from: allItems)
}

func dataScanner(
_ dataScanner: DataScannerViewController,
becameUnavailableWithError error: DataScannerViewController.ScanningUnavailable,
) {
onScannerUnavailable?()
}

// MARK: Private Helpers

private func updateLines(from items: [RecognizedItem]) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import VisionKit
import XCTest

@testable import BitwardenShared

// MARK: - CardScannerViewTests

/// Tests for `CardScannerView.Coordinator`.
///
/// `CardScannerWrapperView`'s retry/foreground-resume logic is driven by SwiftUI `@State` and cannot
/// be meaningfully exercised without a live view host, so it is not covered here.
///
@available(iOS 16.0, *)
class CardScannerViewTests: BitwardenTestCase {
// MARK: Tests

/// `dataScanner(_:becameUnavailableWithError:)` calls `onScannerUnavailable` when the scanner
/// reports an `unsupported` error.
func test_becameUnavailableWithError_callsOnScannerUnavailable_unsupported() {
var callbackInvoked = false
let subject = CardScannerView.Coordinator(
onLinesUpdated: { _ in },
onScannerUnavailable: { callbackInvoked = true },
)

subject.dataScanner(CardScannerView.makeScanner(), becameUnavailableWithError: .unsupported)

XCTAssertTrue(callbackInvoked)
}

/// `dataScanner(_:becameUnavailableWithError:)` calls `onScannerUnavailable` when the scanner
/// reports a `cameraRestricted` error.
func test_becameUnavailableWithError_callsOnScannerUnavailable_cameraRestricted() {
var callbackInvoked = false
let subject = CardScannerView.Coordinator(
onLinesUpdated: { _ in },
onScannerUnavailable: { callbackInvoked = true },
)

subject.dataScanner(CardScannerView.makeScanner(), becameUnavailableWithError: .cameraRestricted)

XCTAssertTrue(callbackInvoked)
}

/// `dataScanner(_:becameUnavailableWithError:)` does not crash when `onScannerUnavailable` is nil.
func test_becameUnavailableWithError_nilCallback_doesNotCrash() {
let subject = CardScannerView.Coordinator(
onLinesUpdated: { _ in },
onScannerUnavailable: nil,
)

subject.dataScanner(CardScannerView.makeScanner(), becameUnavailableWithError: .unsupported)
}
}
Loading