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

feat(suite): update passphrase flow #16937

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Conversation

jvaclavik
Copy link
Contributor

@jvaclavik jvaclavik commented Feb 11, 2025

The problem

There are two entry points for showing modal:

  • PassphraseModal (called by Trezor hardware device)
  • SwitchDevice (shown by us in Suite)

Unfortunately part of the flow is handled in PassphraseModal and part of it in SwitchDevice.

Diagram

image

What happened in this PR

  • update flow, before there was no switch between creating new or existing one
  • polish Passphrase modal - divide to multiple external components
  • disable PassphraseWalletConfirmationSteps (1/2/3) for passing through the flows and introducing PassphraseModalContext for keeping the current state. It allows custom order without specific context
  • there are similar states for existing and non-existing flow, but they have different handling of back/next/cancel events

Related Issue

Resolve #15794

@jvaclavik jvaclavik changed the title WIP feat(suite): update passphrase flow Feb 11, 2025
@jvaclavik jvaclavik force-pushed the passphrase-flow-update branch 2 times, most recently from 4376924 to afe72d5 Compare February 13, 2025 16:43
@Vere-Grey Vere-Grey self-requested a review February 14, 2025 12:12
@Vere-Grey
Copy link
Contributor

I know the PR is not published. I have was analyzing test fails and notices these. You have added a new button to the wallet switcher but duplicated a testId. The playwright tests are now failing becuase they detect two elements with same testId instead of one.

@jvaclavik jvaclavik force-pushed the passphrase-flow-update branch 5 times, most recently from 2a6ab88 to 952e6e7 Compare February 20, 2025 14:39
@jvaclavik jvaclavik force-pushed the passphrase-flow-update branch 2 times, most recently from 9dd4594 to 3c677d7 Compare February 25, 2025 09:34
Copy link

github-actions bot commented Feb 25, 2025

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 26
  • More info

Learn more about 𝝠 Expo Github Action

const selectedDevice = useSelector(selectSelectedDevice);
const dispatch = useDispatch();
const { setPassphraseState, setIsExisting } = usePassphraseModalContext();
Copy link
Contributor Author

@jvaclavik jvaclavik Feb 25, 2025

Choose a reason for hiding this comment

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

This context keeps the passphrase state and identification if we went through isExisting branch in the state machine. Theoretically this info can be extracted also from passphraseState.

E.g.: not-exist-enter-passphrase state should have isExisting = false

Should I do it? WDYT?

}
};

export const PassphraseFlow = () => <ModalSwitcher />;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not happy about this component, but honestly, I don't know how to do it better. It's not proper refactoring, just moving related components to one specific place. It should be refactored in the future.


<ModalSwitcher />
<PassphraseFlow />
<AppShortcuts />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to be inside of PassphraseModalProvider context

firmware: FirmwareUpdate,
'firmware-type': FirmwareType,
'firmware-custom': FirmwareCustom,
version: Version,
bridge: BridgeUnavailable,
'bridge-requested': BridgeRequested,
udev: UdevRules,
'switch-device': SwitchDevice,
'switch-device': null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is handled in PassphraseFlow

Comment on lines -46 to -59
// Passphrase on host
case UI.REQUEST_PASSPHRASE:
return <PassphraseModal device={device} />;

case 'WordRequestType_Plain':
return <WordModal renderer={renderer} />;
case 'WordRequestType_Matrix6':
return <WordAdvancedModal count={6} renderer={renderer} />;
case 'WordRequestType_Matrix9':
return <WordAdvancedModal count={9} renderer={renderer} />;
// T2T1 firmware
case UI.REQUEST_PASSPHRASE_ON_DEVICE:
case 'ButtonRequest_PassphraseEntry':
return <PassphraseOnDeviceModal device={device} />;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to PassphraseFlow

}

// first step of adding passphrase wallet from switch device modal
return <PassphraseDefaultForm />;
throw new Error('Unexpected passphrase state');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should never happen

}}
data-testid="@passphrase-confirmation/step2-button"
>
<Button isFullWidth onClick={onNext} data-testid="@passphrase-confirmation/step2-button">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naming of some buttons is not valid anymore. We should rename it

@@ -27,7 +27,6 @@ import {
ImportTransactionModal,
MetadataProviderModal,
MoreRoundsNeededModal,
PassphraseDuplicateModal,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to PassphraseFlow

@jvaclavik jvaclavik force-pushed the passphrase-flow-update branch 6 times, most recently from 4e2c45d to b35f285 Compare March 3, 2025 13:25
@jvaclavik jvaclavik force-pushed the passphrase-flow-update branch 2 times, most recently from deebe4c to 4cb3e67 Compare March 5, 2025 14:01
@jvaclavik jvaclavik force-pushed the passphrase-flow-update branch from 4cb3e67 to e763869 Compare March 13, 2025 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏃‍♀️ In progress
Development

Successfully merging this pull request may close these issues.

Passphrase Understandability 1
2 participants