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.

Sign up for GitHub

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

Jump to bottom

Fix custom derivation path #4658

Merged
merged 19 commits into from
Dec 12, 2022
Merged

Fix custom derivation path #4658

merged 19 commits into from
Dec 12, 2022

Conversation

oskarleonard
Copy link
Contributor

What was the problem?

This PR resolves #4591 #4616

How was it solved?

  • Remove custom derivation path from redux
  • Add validation to derivation path, typing an incorrect derivation path shall prevent a user to go to SetPasswordForm
  • Fix alignment bug of error icon when input size = m

How was it tested?

Manually

@oskarleonard oskarleonard self-assigned this Dec 8, 2022
@oskarleonard oskarleonard marked this pull request as draft December 8, 2022 05:52
@oskarleonard
Remove test because customDerivationPath is not in redux store anymore
a8e8156
@oskarleonard
Use the defaultDerivationPath when enableCustomDerivationPath is true
8ad4302
@oskarleonard
Remove test for login component which we will deprecate
195ab6e
@oskarleonard
remove unused
5b624eb
@oskarleonard
Add tests for CustomDerivationPath
7bbb2ed
@oskarleonard
Use getByLabelText from render instead of importing it, also change t…
f323611 
…he test name to correspond to what is does
@oskarleonard
Add test to make sure that onAddAccount is called with both a passphr…
c485146 
…ase and a derivationPath
@oskarleonard
Add test for when passphrase is correct but derivation path is incorr…
6a88ea3 
…ect.
@oskarleonard
Fix typo
9dc7e99
@oskarleonard
Remove this second failsafe check since the button is already disable…
7052f0a 
…d this function cannot be called if isSubmitDisabled, add test for checking if button is disabled instead
@oskarleonard
Make sure disabled buttons dont have cursor pointer
cf31cbf
@oskarleonard
Merge branch 'development' into 4591-fix-custom-derivation-path
3b3aafa
@oskarleonard
remove unused css rule
2761e8d
@oskarleonard
Merge branch 'development' into 4591-fix-custom-derivation-path
709ea87
@oskarleonard oskarleonard marked this pull request as ready for review December 9, 2022 15:39
clemente-xyz
clemente-xyz requested changes Dec 12, 2022
View reviewed changes
Copy link
Member

@clemente-xyz clemente-xyz left a comment

Choose a reason for hiding this comment

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

small obs

src/theme/buttons/css/base.css Show resolved Hide resolved
@ManuGowda ManuGowda removed the request for review from ikem-legend December 12, 2022 12:34
clemente-xyz
clemente-xyz approved these changes Dec 12, 2022
View reviewed changes
Copy link
Member

@clemente-xyz clemente-xyz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Approving considering that it will be addressed on #4664

@ManuGowda ManuGowda merged commit 06f6084 into development Dec 12, 2022
@ManuGowda ManuGowda deleted the 4591-fix-custom-derivation-path branch December 12, 2022 13:13
@soroushm soroushm mentioned this pull request Feb 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clemente-xyz clemente-xyz clemente-xyz approved these changes

@ManuGowda ManuGowda ManuGowda approved these changes

Assignees

@oskarleonard oskarleonard

Labels
None yet
Projects
No open projects
Lisk Desktop Version 3.0.0
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Default custom derivation path setting is overridden
3 participants
@oskarleonard @ManuGowda @clemente-xyz