From 0ef547a645522a39606b357c8cdce6d9d5c9af10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Tinkl?= Date: Thu, 13 Feb 2025 15:37:23 +0100 Subject: [PATCH] fix(onboarding): Incorrect Back Navigation and Button Visibility on PIN Screens For the PIN pages: - add a `pinSettingInProgress` bool hint to `KeycardCreatePinPage` when setting/authorizing the PIN is in progress to be able to correctly display the Back button - don't display the "success" image yet when in progress - use the hint in related flows - extract the default attempts numbers to `Constants` For the backup seed phrase sequence: - the mnemonic is a string and gets submitted when exiting the BackupSeedphraseReveal, not right after its (re)creation - when starting the flow (going from `KeycardCreatePinDelayedPage`), replace instead of push, so that Back skips the PIN page - fixup the related SB pages Fixes #17218 --- storybook/pages/KeycardEnterPinPagePage.qml | 6 ++++-- storybook/pages/KeycardEnterPukPagePage.qml | 6 ++++-- storybook/pages/LoginScreenPage.qml | 4 ++-- storybook/pages/OnboardingLayoutPage.qml | 8 ++++---- storybook/pages/UnblockWithPukFlowPage.qml | 16 +++++++-------- .../qmlTests/tests/tst_OnboardingLayout.qml | 4 ++-- .../Onboarding2/KeycardCreateProfileFlow.qml | 20 +++++++++---------- .../KeycardCreateReplacementFlow.qml | 2 ++ .../pages/KeycardCreatePinPage.qml | 6 ++---- ui/imports/utils/Constants.qml | 3 +++ 10 files changed, 40 insertions(+), 35 deletions(-) diff --git a/storybook/pages/KeycardEnterPinPagePage.qml b/storybook/pages/KeycardEnterPinPagePage.qml index 643f0b4dbd..422705422c 100644 --- a/storybook/pages/KeycardEnterPinPagePage.qml +++ b/storybook/pages/KeycardEnterPinPagePage.qml @@ -5,6 +5,8 @@ import QtQuick.Layouts 1.15 import AppLayouts.Onboarding2.pages 1.0 import AppLayouts.Onboarding.enums 1.0 +import utils 1.0 + Item { id: root @@ -54,8 +56,8 @@ Item { id: remainingAttemptsSpinBox from: 0 - to: 3 - value: 3 + to: Constants.onboarding.defaultPinAttempts + value: Constants.onboarding.defaultPinAttempts } } diff --git a/storybook/pages/KeycardEnterPukPagePage.qml b/storybook/pages/KeycardEnterPukPagePage.qml index a9d76ba782..f43dbccf28 100644 --- a/storybook/pages/KeycardEnterPukPagePage.qml +++ b/storybook/pages/KeycardEnterPukPagePage.qml @@ -3,6 +3,8 @@ import QtQuick.Controls 2.15 import AppLayouts.Onboarding2.pages 1.0 +import utils 1.0 + Item { id: root @@ -11,7 +13,7 @@ Item { KeycardEnterPukPage { id: page anchors.fill: parent - remainingAttempts: 3 + remainingAttempts: Constants.onboarding.defaultPukAttempts tryToSetPukFunction: (puk) => { console.warn("!!! ATTEMPTED PUK:", puk) const valid = puk === root.existingPuk @@ -28,7 +30,7 @@ Item { console.warn("onKeycardFactoryResetRequested") console.warn("!!! RESETTING FLOW") state = "entering" - remainingAttempts = 3 + remainingAttempts = Constants.onboarding.defaultPukAttempts } } diff --git a/storybook/pages/LoginScreenPage.qml b/storybook/pages/LoginScreenPage.qml index a2a356f1cc..5addd5d726 100644 --- a/storybook/pages/LoginScreenPage.qml +++ b/storybook/pages/LoginScreenPage.qml @@ -22,8 +22,8 @@ SplitView { // keycard property int keycardState: Onboarding.KeycardState.NoPCSCService - property int keycardRemainingPinAttempts: 3 - property int keycardRemainingPukAttempts: 5 + property int keycardRemainingPinAttempts: Constants.onboarding.defaultPinAttempts + property int keycardRemainingPukAttempts: Constants.onboarding.defaultPukAttempts } LoginScreen { diff --git a/storybook/pages/OnboardingLayoutPage.qml b/storybook/pages/OnboardingLayoutPage.qml index c4c5b8fbf1..8e0550237d 100644 --- a/storybook/pages/OnboardingLayoutPage.qml +++ b/storybook/pages/OnboardingLayoutPage.qml @@ -43,8 +43,8 @@ SplitView { store.authorizationState = Onboarding.ProgressState.Idle store.restoreKeysExportState = Onboarding.ProgressState.Idle store.syncState = Onboarding.ProgressState.Idle - store.keycardRemainingPinAttempts = 3 - store.keycardRemainingPukAttempts = 5 + store.keycardRemainingPinAttempts = Constants.onboarding.defaultPinAttempts + store.keycardRemainingPukAttempts = Constants.onboarding.defaultPukAttempts onboarding.restartFlow() } @@ -81,8 +81,8 @@ SplitView { property int syncState: Onboarding.ProgressState.Idle property var loginAccountsModel: ctrlLoginScreen.checked ? loginAccountsModel : emptyModel - property int keycardRemainingPinAttempts: 3 - property int keycardRemainingPukAttempts: 5 + property int keycardRemainingPinAttempts: Constants.onboarding.defaultPinAttempts + property int keycardRemainingPukAttempts: Constants.onboarding.defaultPukAttempts function setPin(pin: string) { logs.logEvent("OnboardingStore.setPin", ["pin"], arguments) diff --git a/storybook/pages/UnblockWithPukFlowPage.qml b/storybook/pages/UnblockWithPukFlowPage.qml index 559c73e013..d5db1fc6af 100644 --- a/storybook/pages/UnblockWithPukFlowPage.qml +++ b/storybook/pages/UnblockWithPukFlowPage.qml @@ -83,16 +83,16 @@ SplitView { tryToSetPukFunction: mockDriver.setPuk remainingAttempts: mockDriver.keycardRemainingPukAttempts onSetPinRequested: (pin) => { - logs.logEvent("keycardPinCreated", ["pin"], arguments) - console.warn("!!! PIN CREATED:", pin) - } + logs.logEvent("setPinRequested", ["pin"], arguments) + console.warn("!!! SET PIN REQUESTED:", pin) + } onKeycardFactoryResetRequested: { logs.logEvent("keycardFactoryResetRequested", ["pin"], arguments) console.warn("!!! FACTORY RESET REQUESTED") } - onFinished: { - console.warn("!!! UNLOCK WITH PUK FINISHED") - logs.logEvent("finished") + onFinished: (success) => { + console.warn("!!! UNLOCK WITH PUK FINISHED:", success) + logs.logEvent("finished", ["success"], arguments) console.warn("!!! RESTARTING FLOW") stackView.clear() @@ -106,11 +106,11 @@ SplitView { function reset() { keycardState = Onboarding.KeycardState.NoPCSCService - keycardRemainingPukAttempts = 3 + keycardRemainingPukAttempts = Constants.onboarding.defaultPukAttempts } property int keycardState: Onboarding.KeycardState.NoPCSCService - property int keycardRemainingPukAttempts: 3 + property int keycardRemainingPukAttempts: Constants.onboarding.defaultPukAttempts function setPuk(puk) { // -> bool logs.logEvent("setPuk", ["puk"], arguments) diff --git a/storybook/qmlTests/tests/tst_OnboardingLayout.qml b/storybook/qmlTests/tests/tst_OnboardingLayout.qml index 364c1812c3..3066583c06 100644 --- a/storybook/qmlTests/tests/tst_OnboardingLayout.qml +++ b/storybook/qmlTests/tests/tst_OnboardingLayout.qml @@ -59,8 +59,8 @@ Item { readonly property int pinSettingState: mockDriver.pinSettingState // enum Onboarding.ProgressState readonly property int authorizationState: mockDriver.authorizationState // enum Onboarding.ProgressState readonly property int restoreKeysExportState: mockDriver.restoreKeysExportState // enum Onboarding.ProgressState - property int keycardRemainingPinAttempts: 3 - property int keycardRemainingPukAttempts: 5 + property int keycardRemainingPinAttempts: Constants.onboarding.defaultPinAttempts + property int keycardRemainingPukAttempts: Constants.onboarding.defaultPukAttempts property var loginAccountsModel: emptyModel function setPin(pin: string) { diff --git a/ui/app/AppLayouts/Onboarding2/KeycardCreateProfileFlow.qml b/ui/app/AppLayouts/Onboarding2/KeycardCreateProfileFlow.qml index fb18c36933..ac6be51df7 100644 --- a/ui/app/AppLayouts/Onboarding2/KeycardCreateProfileFlow.qml +++ b/ui/app/AppLayouts/Onboarding2/KeycardCreateProfileFlow.qml @@ -43,7 +43,7 @@ SQUtils.QObject { id: d property bool withNewSeedphrase - property var mnemonic + property string mnemonic function initialComponent() { if (root.keycardState === Onboarding.KeycardState.Empty) @@ -113,17 +113,13 @@ SQUtils.QObject { Component { id: backupSeedRevealPage BackupSeedphraseReveal { - Component.onCompleted: { - try { - d.mnemonic = root.generateMnemonic() - root.seedphraseSubmitted(d.mnemonic) - } catch (e) { - console.error('failed to generate mnemonic', e) - } - } + Component.onCompleted: d.mnemonic = root.generateMnemonic() mnemonic: d.mnemonic - onBackupSeedphraseConfirmed: root.stackView.push(backupSeedVerifyPage) + onBackupSeedphraseConfirmed: { + root.seedphraseSubmitted(d.mnemonic) + root.stackView.push(backupSeedVerifyPage) + } } } @@ -165,6 +161,8 @@ SQUtils.QObject { id: keycardCreatePinPage KeycardCreatePinDelayedPage { + readonly property bool backAvailableHint: !success && !pinSettingInProgress + pinSettingState: root.pinSettingState authorizationState: root.authorizationState @@ -173,7 +171,7 @@ SQUtils.QObject { onFinished: { if (d.withNewSeedphrase) { - root.stackView.push(backupSeedIntroPage) + root.stackView.replace(backupSeedIntroPage) } else { root.loadMnemonicRequested() root.stackView.push(addKeypairPage) diff --git a/ui/app/AppLayouts/Onboarding2/KeycardCreateReplacementFlow.qml b/ui/app/AppLayouts/Onboarding2/KeycardCreateReplacementFlow.qml index f25f666e7e..054f14397d 100644 --- a/ui/app/AppLayouts/Onboarding2/KeycardCreateReplacementFlow.qml +++ b/ui/app/AppLayouts/Onboarding2/KeycardCreateReplacementFlow.qml @@ -91,6 +91,8 @@ SQUtils.QObject { id: keycardCreatePinPage KeycardCreatePinDelayedPage { + readonly property bool backAvailableHint: !success && !pinSettingInProgress + authorizationState: root.authorizationState pinSettingState: root.pinSettingState diff --git a/ui/app/AppLayouts/Onboarding2/pages/KeycardCreatePinPage.qml b/ui/app/AppLayouts/Onboarding2/pages/KeycardCreatePinPage.qml index b245b88398..0ffc5498d7 100644 --- a/ui/app/AppLayouts/Onboarding2/pages/KeycardCreatePinPage.qml +++ b/ui/app/AppLayouts/Onboarding2/pages/KeycardCreatePinPage.qml @@ -13,6 +13,8 @@ KeycardBasePage { property bool success + readonly property bool pinSettingInProgress: d.state === "settingInProgress" + signal setPinRequested(string pin) image.source: Theme.png("onboarding/keycard/reading") @@ -130,10 +132,6 @@ KeycardBasePage { target: loadingIndicator visible: true } - PropertyChanges { - target: root - image.source: Theme.png("onboarding/keycard/success") - } StateChangeScript { script: { pinInput.setPin(d.pin) diff --git a/ui/imports/utils/Constants.qml b/ui/imports/utils/Constants.qml index f0b8620073..d69b5485d7 100644 --- a/ui/imports/utils/Constants.qml +++ b/ui/imports/utils/Constants.qml @@ -418,6 +418,9 @@ QtObject { readonly property string watchOnlyAccounts: "watchOnlyAccounts" } } + + readonly property int defaultPinAttempts: 3 + readonly property int defaultPukAttempts: 5 } readonly property QtObject onlineStatus: QtObject{