New approach to implement new wallet wizard #7986
Conversation
| self.logger.debug(repr(wizard)) | ||
| views = { | ||
| 'trustedcoin_start': { | ||
| 'gui': '../../../../plugins/trustedcoin/qml/Disclaimer', |
There was a problem hiding this comment.
what is this relative to -- what is the "current directory" here?
There was a problem hiding this comment.
Current directory is the directory where the QML file exists that is referencing it.
so:
/path/1.qml: Qt.resolvedUrl('path/2.qml')
/path/path/2.qml: Qt.resolvedUrl('../1.qml')
I know, this is ugly as f :( I hope Qt6 improves on that.
I've tried to add more import paths to the QML engine, to have more 'roots', but that doesn't work :(
There was a problem hiding this comment.
But on the bright side, if we factor out plugins into the main tree, this disappears mostly.
There was a problem hiding this comment.
Note: I also tried to define a QQmlAbstractUrlInterceptor, but that didn't really work in python/PyQt.
There was a problem hiding this comment.
Relative paths can also be resolved by packaging all qml files in a resource bundle.
This however requires one to regenerate the bundle before testing each change.
accumulator
force-pushed
the
wizard
branch
14 times, most recently
from
beba877
to
e514cef
Compare
|
If there are no big objections to this PR, I suggest we merge it, as it's getting harder to maintain separate of master. It's fully working on QML (including complete 2FA wizard implementation and regular 2FA use while still keeping it 'isolated' as a plugin), and merging doesn't impact the existing Qt and Kivy clients, as it is kept separate from the existing (base)wizard. the |
|
@accumulator I will have a look tomorrow |
|
Would it be possible to move |
Yep, no problem. |
'last' property on WizardComponents is now queried from UI, not from the wizard. This allows the content of the WizardComponent itself to be taken into account.
|
rebased on master, small conflict resolved |
electrum/wizard.py
Outdated
| return self._current | ||
|
|
||
| def last_if_single_password(self, view, wizard_data): | ||
| return False # TODO: self._daemon.config.get('single_password') |
There was a problem hiding this comment.
Please raise a NotImplementedError exception here, instead of returning False.
it took me a while to understand that this method must be overloaded.
There was a problem hiding this comment.
The idea was to provide a common implementation that optionally can be overridden, but as we need to combine the password check, config setting and check whether the other wallets use the same password, I guess this will always need a gui specific implementation.. Will modify to NotimplementedError.
…d page also refactor seed verification, split off seed_variant from seed_type (partial disambiguation), fix bip39 wallet creation
… always available to wizard navigation evaluation
This PR separates the wizard navigation from the logic to validate and create new wallets.
The current implementation is mostly imperative.
BaseWizard(inbase_wizard.py) implements a number of functions/handlers, which decide (depending on earlier choices, or passed parameters) which view to instantiate. The instantiated view gets passed a run_next parameter that determines which handler function follows the to-be shown view. Often some validator function is passed as well, which the view can use to determine if the user is allowed to proceed to the next view.Specific GUI implementations then subclass
BaseWizardand implement supporting functions that create views imperatively and sometimes blocking execution until a view is completed.This imperative approach is quite incompatible with QML, which has a much stronger separation between UI and backend logic.
This PR adds a new wizard approach, that uses simply view names (strings), linked together in a 'navmap'. The wizard maintains a dict of user entered or wizard added data, which is used at the conclusion of the wizard to create the wallet. The UI specific implementation simply associates the view names with concrete views, and provides a small wrapper to expose the navigation actions.
Also, with this approach the UI is in control, using the wizard as a context and helper, instead of the other way around (wizard (sub)class in control, controlling the UI).
The separation of the navigation also makes it much easier to extend/modify the flow through the views. The old wizard requires subclassing to override behavior (also requiring you to copy potential validation in the subclass), in this approach you simply add views and add/override navigation decisions. For example, the QML implementation for 2fa/trustedcoin defines new views and hooks them into the base NewWalletWizard. (Note: I know we want to get rid of plugins, but for now it'll have to do).
Note 1: validation is currently mostly done in the QML views and in create_storage, this could be moved to the wizard as well.
Note 2: as the wizard state is serializable, this approach should also be able to 'resume' the wizard on a different computer (for example the 2fa offline wallet creation use case). This is not implemented yet.
Note 3: this new wizard can also be used for conversational flows other than new wallet wizards, e.g. collecting the signers in a MuSig scenario