From b10d534c9457ce9762a97d86a9e31de6eef92c55 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Fri, 3 Nov 2023 07:31:01 +0000 Subject: [PATCH] Debounce PIN entry to show the last digit before submitting. (#2017) --- .../AppLockScreen/AppLockScreenModels.swift | 2 -- .../AppLockScreenViewModel.swift | 16 +++++++--- .../AppLockScreen/View/AppLockScreen.swift | 4 --- .../AppLockSetupPINScreenModels.swift | 2 -- .../AppLockSetupPINScreenViewModel.swift | 14 ++++++-- .../View/AppLockSetupPINScreen.swift | 4 --- .../AppLock/AppLockScreenViewModelTests.swift | 20 ++++++++---- .../AppLockSetupPINScreenViewModelTests.swift | 32 ++++++++++++------- changelog.d/2010.bugfix | 1 + 9 files changed, 58 insertions(+), 37 deletions(-) create mode 100644 changelog.d/2010.bugfix diff --git a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift index 6953ac0de..947f177a3 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift @@ -60,8 +60,6 @@ enum AppLockScreenAlertType { } enum AppLockScreenViewAction { - /// Attempt to unlock the app with the supplied PIN code. - case submitPINCode /// Clears the PIN code after a failure animation. case clearPINCode /// The user didn't heed the warnings and can't remember their PIN. diff --git a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift index af9ac99df..de25584b5 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift @@ -36,6 +36,16 @@ class AppLockScreenViewModel: AppLockScreenViewModelType, AppLockScreenViewModel .weakAssign(to: \.state.numberOfPINAttempts, on: self) .store(in: &cancellables) + context.$viewState + .map(\.bindings.pinCode) + .removeDuplicates() + .debounce(for: 0.05, scheduler: DispatchQueue.main) // Allow the last digit to be briefly shown + .sink { [weak self] pinCode in + guard pinCode.count == 4 else { return } + self?.submit(pinCode) + } + .store(in: &cancellables) + showForceLogoutAlertIfNeeded() } @@ -45,8 +55,6 @@ class AppLockScreenViewModel: AppLockScreenViewModelType, AppLockScreenViewModel MXLog.info("View model: received view action: \(viewAction)") switch viewAction { - case .submitPINCode: - submitPINCode() case .clearPINCode: state.bindings.pinCode = "" case .forgotPIN: @@ -56,8 +64,8 @@ class AppLockScreenViewModel: AppLockScreenViewModelType, AppLockScreenViewModel // MARK: - Private - private func submitPINCode() { - guard appLockService.unlock(with: state.bindings.pinCode) else { + private func submit(_ pinCode: String) { + guard appLockService.unlock(with: pinCode) else { handleInvalidPIN() return } diff --git a/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift b/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift index 8afbaec20..145380f99 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift @@ -49,10 +49,6 @@ struct AppLockScreen: View { .accessibilityValue(L10n.a11yDigitsEntered(context.viewState.numberOfDigitsEntered)) AppLockScreenPINKeypad(pinCode: $context.pinCode) - .onChange(of: context.pinCode) { newValue in - guard newValue.count == 4 else { return } - context.send(viewAction: .submitPINCode) - } } } bottomContent: { Button(L10n.screenAppLockForgotPin) { diff --git a/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenModels.swift b/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenModels.swift index 08ae27ad6..72a404479 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenModels.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenModels.swift @@ -88,8 +88,6 @@ enum AppLockSetupPINScreenAlertType { } enum AppLockSetupPINScreenViewAction { - /// Confirm the entered PIN. - case submitPINCode /// Stop entering a PIN. case cancel } diff --git a/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenViewModel.swift b/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenViewModel.swift index d26ffff48..55d68ce42 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenViewModel.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/AppLockSetupPINScreenViewModel.swift @@ -38,6 +38,16 @@ class AppLockSetupPINScreenViewModel: AppLockSetupPINScreenViewModelType, AppLoc appLockService.numberOfPINAttempts .weakAssign(to: \.state.numberOfUnlockAttempts, on: self) .store(in: &cancellables) + + context.$viewState + .map(\.bindings.pinCode) + .removeDuplicates() + .debounce(for: 0.1, scheduler: DispatchQueue.main) // Show the last digit for long enough to be read. + .sink { [weak self] pinCode in + guard pinCode.count == 4 else { return } + self?.submit(pinCode) + } + .store(in: &cancellables) } // MARK: - Public @@ -46,8 +56,6 @@ class AppLockSetupPINScreenViewModel: AppLockSetupPINScreenViewModelType, AppLoc MXLog.info("View model: received view action: \(viewAction)") switch viewAction { - case .submitPINCode: - submitPINCode() case .cancel: actionsSubject.send(.cancel) } @@ -56,7 +64,7 @@ class AppLockSetupPINScreenViewModel: AppLockSetupPINScreenViewModelType, AppLoc // MARK: - Private /// Handle the entered PIN code. - private func submitPINCode() { + private func submit(_ pinCode: String) { switch state.mode { case .create: createPIN() diff --git a/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/View/AppLockSetupPINScreen.swift b/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/View/AppLockSetupPINScreen.swift index 83b21e400..552995727 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/View/AppLockSetupPINScreen.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockSetupPINScreen/View/AppLockSetupPINScreen.swift @@ -43,10 +43,6 @@ struct AppLockSetupPINScreen: View { PINTextField(pinCode: $context.pinCode, isSecure: context.viewState.mode == .unlock) .focused($textFieldFocus) - .onChange(of: context.pinCode) { newValue in - guard newValue.count == 4 else { return } - context.send(viewAction: .submitPINCode) - } } .padding(.horizontal, 16) .padding(.top, UIConstants.iconTopPaddingToNavigationBar) diff --git a/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift b/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift index 46a69c01a..207655ff8 100644 --- a/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift +++ b/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift @@ -48,7 +48,6 @@ class AppLockScreenViewModelTests: XCTestCase { // When entering it on the lock screen. let deferred = deferFulfillment(viewModel.actions) { $0 == .appUnlocked } viewModel.context.pinCode = pinCode - context.send(viewAction: .submitPINCode) let result = try await deferred.fulfill() // The app should become unlocked. @@ -76,8 +75,10 @@ class AppLockScreenViewModelTests: XCTestCase { XCTAssertNil(context.alertInfo, "No alert should be shown yet.") // When entering it on the lock screen. + var deferred = deferFulfillment(context.$viewState) { $0.numberOfPINAttempts == 1 } viewModel.context.pinCode = pinCode - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() + context.send(viewAction: .clearPINCode) // Simulate the animation completion // Then a failed attempt should be shown. XCTAssertEqual(context.viewState.numberOfPINAttempts, 1, "A failed attempt should have been recorded.") @@ -85,8 +86,14 @@ class AppLockScreenViewModelTests: XCTestCase { XCTAssertNil(context.alertInfo, "No alert should be shown yet.") // When entering twice more - context.send(viewAction: .submitPINCode) - context.send(viewAction: .submitPINCode) + deferred = deferFulfillment(context.$viewState) { $0.numberOfPINAttempts == 2 } + viewModel.context.pinCode = pinCode + try await deferred.fulfill() + context.send(viewAction: .clearPINCode) // Simulate the animation completion + deferred = deferFulfillment(context.$viewState) { $0.numberOfPINAttempts == 3 } + viewModel.context.pinCode = pinCode + try await deferred.fulfill() + context.send(viewAction: .clearPINCode) // Simulate the animation completion // Then an alert should be shown XCTAssertEqual(context.viewState.numberOfPINAttempts, 3, "All the attempts should have been recorded.") @@ -94,14 +101,15 @@ class AppLockScreenViewModelTests: XCTestCase { XCTAssertEqual(context.alertInfo?.id, .forcedLogout, "An alert should now be shown.") } - func testForceQuitRequiresLogout() { + func testForceQuitRequiresLogout() async throws { // Given an app with a PIN set where the user attempted to unlock 3 times. keychainController.pinCodeReturnValue = "2023" keychainController.containsPINCodeBiometricStateReturnValue = false appSettings.appLockNumberOfPINAttempts = 2 XCTAssertNil(context.alertInfo) + let deferred = deferFulfillment(context.$viewState) { $0.numberOfPINAttempts == 3 } viewModel.context.pinCode = "0000" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() XCTAssertEqual(appSettings.appLockNumberOfPINAttempts, 3, "The app should have 3 failed attempts before the force quit.") XCTAssertEqual(context.alertInfo?.id, .forcedLogout, "The app should be showing the alert before the force quit.") diff --git a/UnitTests/Sources/AppLock/AppLockSetupPINScreenViewModelTests.swift b/UnitTests/Sources/AppLock/AppLockSetupPINScreenViewModelTests.swift index aa77904d2..c0c7b2f04 100644 --- a/UnitTests/Sources/AppLock/AppLockSetupPINScreenViewModelTests.swift +++ b/UnitTests/Sources/AppLock/AppLockSetupPINScreenViewModelTests.swift @@ -44,7 +44,6 @@ class AppLockSetupPINScreenViewModelTests: XCTestCase { // When entering an new PIN. let createDeferred = deferFulfillment(context.$viewState, message: "A valid PIN needs confirming.") { $0.mode == .confirm } context.pinCode = "2023" - context.send(viewAction: .submitPINCode) try await createDeferred.fulfill() // Then the screen should transition to the confirm mode. @@ -53,7 +52,6 @@ class AppLockSetupPINScreenViewModelTests: XCTestCase { // When re-entering that PIN. let confirmDeferred = deferFulfillment(viewModel.actions, message: "The screen should be finished.") { $0 == .complete } context.pinCode = "2023" - context.send(viewAction: .submitPINCode) // Then the screen should signal it is complete. try await confirmDeferred.fulfill() @@ -66,8 +64,9 @@ class AppLockSetupPINScreenViewModelTests: XCTestCase { XCTAssertNil(context.alertInfo, "There shouldn't be an alert to begin with.") // When entering a weak PIN on the blocklist. + let deferred = deferFulfillment(context.$viewState) { $0.bindings.alertInfo != nil } context.pinCode = "0000" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() // Then the PIN should be rejected and the user alerted. XCTAssertEqual(context.alertInfo?.id, .weakPIN, "The weak PIN should be rejected.") @@ -82,25 +81,29 @@ class AppLockSetupPINScreenViewModelTests: XCTestCase { let createDeferred = deferFulfillment(context.$viewState, message: "A valid PIN needs confirming.") { $0.mode == .confirm } context.pinCode = "2023" - context.send(viewAction: .submitPINCode) try await createDeferred.fulfill() XCTAssertEqual(context.viewState.mode, .confirm, "The mode should transition to confirmation.") XCTAssertEqual(context.viewState.numberOfConfirmAttempts, 0, "The mode should start with zero attempts.") XCTAssertNil(context.alertInfo, "There shouldn't be an alert after a valid initial PIN.") // When entering the new PIN incorrectly + var deferred = deferFulfillment(context.$viewState) { $0.numberOfConfirmAttempts == 1 } context.pinCode = "2024" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() // Then the user should be alerted. XCTAssertEqual(context.viewState.numberOfConfirmAttempts, 1, "The mismatch should be counted.") XCTAssertEqual(context.alertInfo?.id, .pinMismatch, "A PIN mismatch should be rejected.") - // When repeating this twice more. + // When dismissing the alert and repeating twice more. + context.alertInfo?.primaryButton.action?() + deferred = deferFulfillment(context.$viewState) { $0.numberOfConfirmAttempts == 2 } context.pinCode = "2024" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() + context.alertInfo?.primaryButton.action?() + deferred = deferFulfillment(context.$viewState) { $0.numberOfConfirmAttempts == 3 } context.pinCode = "2024" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() XCTAssertEqual(context.viewState.numberOfConfirmAttempts, 3, "All the mismatches should be counted.") XCTAssertEqual(context.alertInfo?.id, .pinMismatch, "A PIN mismatch should be rejected.") @@ -120,7 +123,6 @@ class AppLockSetupPINScreenViewModelTests: XCTestCase { // When entering the configured PIN. let deferred = deferFulfillment(viewModel.actions, message: "The screen should be finished.") { $0 == .complete } context.pinCode = pinCode - context.send(viewAction: .submitPINCode) // Then the screen should signal it is complete. try await deferred.fulfill() @@ -137,8 +139,10 @@ class AppLockSetupPINScreenViewModelTests: XCTestCase { XCTAssertFalse(context.viewState.isLoggingOut, "The view should not start disabled.") // When entering a different PIN. + var deferred = deferFulfillment(context.$viewState, keyPath: \.bindings.pinCode, transitionValues: ["", "2024", ""], + message: "The PIN should be entered and then cleared by the view model.") context.pinCode = "2024" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() // Then the PIN should be rejected and the user notified. XCTAssertEqual(context.viewState.numberOfUnlockAttempts, 1, "An invalid attempt should be counted.") @@ -146,10 +150,14 @@ class AppLockSetupPINScreenViewModelTests: XCTestCase { XCTAssertFalse(context.viewState.isLoggingOut, "The view should still work.") // When entering the same incorrect PIN twice more + deferred = deferFulfillment(context.$viewState, keyPath: \.bindings.pinCode, transitionValues: ["", "2024", ""], + message: "The PIN should be entered and then cleared by the view model.") context.pinCode = "2024" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() + deferred = deferFulfillment(context.$viewState, keyPath: \.bindings.pinCode, transitionValues: ["", "2024", ""], + message: "The PIN should be entered and then cleared by the view model.") context.pinCode = "2024" - context.send(viewAction: .submitPINCode) + try await deferred.fulfill() // Then the user should be alerted that they're being signed out. XCTAssertEqual(context.viewState.numberOfUnlockAttempts, 3, "All invalid attempts should be counted.") diff --git a/changelog.d/2010.bugfix b/changelog.d/2010.bugfix new file mode 100644 index 000000000..8e2111249 --- /dev/null +++ b/changelog.d/2010.bugfix @@ -0,0 +1 @@ +Make sure the last digit is visible when entering a PIN. \ No newline at end of file