Enter seed phrase component refactored according to new design #19004
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
micieslak
requested review from
a team,
alexjba,
caybro and
noeliaSD
as code owners
Jenkins BuildsClick to see older builds (139)
|
micieslak
force-pushed
the
feat/issue-18834
branch
2 times, most recently
from
fed3537 to
5e95d3e
Compare
caybro
reviewed
Oct 15, 2025
caybro
reviewed
Oct 15, 2025
micieslak
changed the title
micieslak
force-pushed
the
feat/issue-18834
branch
from
5e95d3e to
2b2263e
Compare
|
i will work on tests to fix them ,just ping me whenever the latest commit is please |
|
Test is failing because the continue button is not getting enabled when last word is entered, is that expected? I can reproduce manually too, type in last word in the grid ->> the suggestion is not going away + continue button is not enabled I have to hit Enter one more time to get it enabled. It was not like that before (in master now entering last word enables the button) Screen.Recording.2025-10-15.at.18.49.21.mov
|
micieslak
force-pushed
the
feat/issue-18834
branch
6 times, most recently
from
0383937 to
c618d05
Compare
alexjba
approved these changes
Oct 17, 2025
There was a problem hiding this comment.
Awesome work! Just nitpicks from my side. Didn't test it though. I see Nastya has it covered
The only thing that might be worth updating from my point of view is the way we're using callbacks in a signal.
But since it's a very specific component we can live with it for sure! Feel free to ignore.
micieslak
force-pushed
the
feat/issue-18834
branch
from
c618d05 to
c1e334d
Compare
micieslak
force-pushed
the
feat/issue-18834
branch
3 times, most recently
from
13e4003 to
0ee1ddd
Compare
micieslak
force-pushed
the
feat/issue-18834
branch
2 times, most recently
from
0ec927d to
64c225b
Compare
micieslak
force-pushed
the
feat/issue-18834
branch
from
64c225b to
3384a45
Compare
noeliaSD
approved these changes
Oct 22, 2025
There was a problem hiding this comment.
Really nice component cleanup! Also the new design looks good!!
I just left minor comments and to add some improvement.. When switching from 12 to 18 or to 24, the focus always changes to the first text field, losing the current text field you were using. It can be of course a separate task and nice to have one :)
Good job!
| readonly property var lengths: [12, 18, 24] | ||
| readonly property int selectedLength: { | ||
| if (v12.checked) | ||
| return 12 |
There was a problem hiding this comment.
Minor: Maybe you could use constants instead everywhere here for 12, 18 and 14
There was a problem hiding this comment.
As it's local thing I would just keep as it is. Those constants would have those number in its name as well...
| property string filteringPrefix | ||
|
|
||
| readonly property int twoColsThreshold: 450 | ||
| readonly property int oneColThreshold: 400 |
There was a problem hiding this comment.
Should we use here Theme.portraitBreakpoint?
There was a problem hiding this comment.
It's not applicable there. Ideally design team should provide some more curated values for that later.
| onWordSelected: word => { | ||
| inputModel.setEntry(d.currentIndex, word, true) | ||
|
|
||
| // don't loos focust on the list entry, close suggestions |
| } | ||
|
|
||
| onSeedphraseUpdated: function(valid, seedphrase) { | ||
| onSeedphraseProvided: seedPhrase => { |
There was a problem hiding this comment.
Would it make sense to have this validation inside the component?? Why the instantiator has the ability to change that?? 🤔
There was a problem hiding this comment.
Basically the EnterSeedPhrase provides basic validation by checking if provided words are in a dictionary. On top user can do arbitrary validation and set arbitrary error message, specific to given use case. It makes the component more generic and OCP principle compliant.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does the PR do
BIP39_endictionary component removed from storeEnterSeedPhrasecomponent with a new versionFigma: https://www.figma.com/design/SGyfSjxs5EbzimHDXTlj8B/Qt-Responsive---v?node-id=2423-40310&t=mAqENLAZjVmyQM
Closes: #18834
Affected areas
OnboardingArchitecture compliance
My PR is consistent with this document: QML Architecture Guidelines
Screencapture of the functionality
Screencast.from.15.10.2025.00.52.30.webm
Record_2025-10-15-10-37-23.mp4
Impact on end user
High
How to test
Run onboarding flows involving seed phrase
Risk